* [PATCH] travis-ci: fix running P4 and Git LFS tests in Linux build jobs @ 2017-11-01 11:55 SZEDER Gábor 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-11-01 11:55 UTC (permalink / raw) To: Lars Schneider, Junio C Hamano; +Cc: git, SZEDER Gábor Linux build jobs on Travis CI skip the P4 and Git LFS tests since commit 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10), claiming there are no P4 or Git LFS installed. The reason is that P4 and Git LFS binaries are not installed to a directory in the default $PATH, but their directories are prepended to $PATH. This worked just fine before said commit, because $PATH was set in a scriptlet embedded in our '.travis.yml', thus its new value was visible during the rest of the build job. However, after these embedded scriptlets were moved into dedicated scripts executed in separate shell processes, any variable set in one of those scripts is only visible in that single script but not in any of the others. In this case, 'ci/install-dependencies.sh' downloads P4 and Git LFS and modifies $PATH, but to no effect, because 'ci/run-tests.sh' only sees Travis CI's default $PATH. Move adjusting $PATH to 'ci/lib-travisci.sh', which is sourced in all other 'ci/' scripts, so all those scripts will see the updated $PATH value. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/install-dependencies.sh | 10 ++++------ ci/lib-travisci.sh | 8 ++++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index a29246af3..5bd06fe90 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -12,20 +12,18 @@ case "${TRAVIS_OS_NAME:-linux}" in linux) export GIT_TEST_HTTPD=YesPlease - mkdir --parents custom/p4 - pushd custom/p4 + mkdir --parents "$P4_PATH" + pushd "$P4_PATH" wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" wget --quiet "$P4WHENCE/bin.linux26x86_64/p4" chmod u+x p4d chmod u+x p4 - export PATH="$(pwd):$PATH" popd - mkdir --parents custom/git-lfs - pushd custom/git-lfs + mkdir --parents "$GIT_LFS_PATH" + pushd "$GIT_LFS_PATH" wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs . - export PATH="$(pwd):$PATH" popd ;; osx) diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index b3ed0a0dd..ac05f1f46 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -26,3 +26,11 @@ skip_branch_tip_with_tag () { set -e skip_branch_tip_with_tag + +case "${TRAVIS_OS_NAME:-linux}" in +linux) + P4_PATH="$(pwd)/custom/p4" + GIT_LFS_PATH="$(pwd)/custom/git-lfs" + export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" + ;; +esac -- 2.15.0.67.gb67a46776 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH 0/4] travis-ci: clean up setting environment variables 2017-11-01 11:55 [PATCH] travis-ci: fix running P4 and Git LFS tests in Linux build jobs SZEDER Gábor @ 2017-12-11 23:34 ` SZEDER Gábor 2017-12-11 23:34 ` [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output SZEDER Gábor ` (4 more replies) 0 siblings, 5 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-11 23:34 UTC (permalink / raw) To: git; +Cc: Lars Schneider, Thomas Gummerer, SZEDER Gábor (Was: [PATCH] travis-ci: fix running P4 and Git LFS tests in Linux build jobs) On Wed, Nov 1, 2017 at 12:55 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote: > However, after these > embedded scriptlets were moved into dedicated scripts executed in > separate shell processes, any variable set in one of those scripts is > only visible in that single script but not in any of the others. > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh > index a29246af3..5bd06fe90 100755 > --- a/ci/install-dependencies.sh > +++ b/ci/install-dependencies.sh > @@ -12,20 +12,18 @@ case "${TRAVIS_OS_NAME:-linux}" in > linux) > export GIT_TEST_HTTPD=YesPlease Astute readers ;) might have been wondering what's the deal with this environment variable in the patch context, since this won't have any effect outside of this script, either. Alas, it's not as easy as moving setting GIT_TEST_HTTPD to 'ci/lib-travisci.sh', like the quoted patch did with paths to P4 and Git LFS, because then it would be set for the 32 bit Linux build job which runs everything as root, thus can't run https tests. This patch series deals with this by adding means that 'ci/*' scripts can use to identify which build job they are taking part in (patch 3), and then setting environment variables in 'ci/lib-travisci.sh', where we have now more freedom to set a specific variable only for specific build jobs (patches 3 and 4). This patch series conflicts with the last patch in Thomas' split index fix series. https://public-inbox.org/git/20171210212202.28231-4-t.gummerer@gmail.com/ The conflict is not overly difficult, but to resolve it we should first come up with a reasonable job name for that build job, e.g. something like "misc-knobs" or whatever, because the combination "GETTEXT_POISON-SPLIT_INDEX" is just too long to exist and won't scale if we enable further knobs in the future. SZEDER Gábor (4): travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output travis-ci: introduce a $jobname variable for 'ci/*' scripts travis-ci: move setting environment variables to 'ci/lib-travisci.sh' travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' .travis.yml | 26 +++++--------------------- ci/install-dependencies.sh | 8 +++----- ci/lib-travisci.sh | 34 +++++++++++++++++++++++++++++++--- 3 files changed, 39 insertions(+), 29 deletions(-) -- 2.15.1.421.gc469ca1de ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor @ 2017-12-11 23:34 ` SZEDER Gábor 2017-12-12 18:00 ` Lars Schneider 2017-12-11 23:34 ` [PATCH 2/4] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor ` (3 subsequent siblings) 4 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-11 23:34 UTC (permalink / raw) To: git; +Cc: Lars Schneider, Thomas Gummerer, SZEDER Gábor While the build logic was embedded in our '.travis.yml', Travis CI used to produce a nice trace log including all commands executed in those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10), however, we only see the name of the dedicated scripts, but not what those scripts are actually doing, resulting in a less useful trace log. A patch later in this series will move setting environment variables from '.travis.yml' to the 'ci/*' scripts, so not even those will be included in the trace log. Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other 'ci/*' scripts, so we get trace log about the commands executed in all of those scripts. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/lib-travisci.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index ac05f1f46..a0c8ae03f 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () { # Set 'exit on error' for all CI scripts to let the caller know that # something went wrong -set -e +set -ex skip_branch_tip_with_tag -- 2.15.1.421.gc469ca1de ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-11 23:34 ` [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output SZEDER Gábor @ 2017-12-12 18:00 ` Lars Schneider 2017-12-12 18:43 ` SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: Lars Schneider @ 2017-12-12 18:00 UTC (permalink / raw) To: SZEDER Gábor; +Cc: git, Thomas Gummerer > On 12 Dec 2017, at 00:34, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > While the build logic was embedded in our '.travis.yml', Travis CI > used to produce a nice trace log including all commands executed in > those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI > code into dedicated scripts, 2017-09-10), however, we only see the > name of the dedicated scripts, but not what those scripts are actually > doing, resulting in a less useful trace log. A patch later in this > series will move setting environment variables from '.travis.yml' to > the 'ci/*' scripts, so not even those will be included in the trace > log. > > Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other > 'ci/*' scripts, so we get trace log about the commands executed in all > of those scripts. I kind of did that intentionally to avoid clutter in the logs. However, I also agree with your reasoning. Therefore, the change looks good to me! Thanks, Lars > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > ci/lib-travisci.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh > index ac05f1f46..a0c8ae03f 100755 > --- a/ci/lib-travisci.sh > +++ b/ci/lib-travisci.sh > @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () { > > # Set 'exit on error' for all CI scripts to let the caller know that > # something went wrong > -set -e > +set -ex > > skip_branch_tip_with_tag > > -- > 2.15.1.421.gc469ca1de > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-12 18:00 ` Lars Schneider @ 2017-12-12 18:43 ` SZEDER Gábor 2017-12-13 23:10 ` Lars Schneider 0 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-12 18:43 UTC (permalink / raw) To: Lars Schneider; +Cc: Git mailing list, Thomas Gummerer On Tue, Dec 12, 2017 at 7:00 PM, Lars Schneider <larsxschneider@gmail.com> wrote: > >> On 12 Dec 2017, at 00:34, SZEDER Gábor <szeder.dev@gmail.com> wrote: >> >> While the build logic was embedded in our '.travis.yml', Travis CI >> used to produce a nice trace log including all commands executed in >> those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI >> code into dedicated scripts, 2017-09-10), however, we only see the >> name of the dedicated scripts, but not what those scripts are actually >> doing, resulting in a less useful trace log. A patch later in this >> series will move setting environment variables from '.travis.yml' to >> the 'ci/*' scripts, so not even those will be included in the trace >> log. >> >> Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other >> 'ci/*' scripts, so we get trace log about the commands executed in all >> of those scripts. > > I kind of did that intentionally to avoid clutter in the logs. > However, I also agree with your reasoning. Therefore, the change > looks good to me! Great, 'cause I'm starting to have second thoughts about this change :) It sure helped a lot while I worked on this patch series and a couple of other Travis CI related patches (will submit them later)... OTOH it definitely creates clutter in the trace log. The worst offender might be 'ci/print-test-failures.sh', which iterates over all 't/test-results/*.exit' files to find which tests failed and to show their output, and 'set -x' makes every iteration visible. And we have about 800 tests, which means 800 iterations. Yuck. Perhaps we should use other means to show what's going on instead, e.g. use more 'echo's and '--verbose' options, or just avoid using '--quiet'. And if some brave souls really want to tweak '.travis.yml' or the 'ci/*' scripts, then they can set 'set -x' for themselves during development... as the patch below shows it's easy enough, just a single character :) Gábor >> >> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> >> --- >> ci/lib-travisci.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh >> index ac05f1f46..a0c8ae03f 100755 >> --- a/ci/lib-travisci.sh >> +++ b/ci/lib-travisci.sh >> @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () { >> >> # Set 'exit on error' for all CI scripts to let the caller know that >> # something went wrong >> -set -e >> +set -ex >> >> skip_branch_tip_with_tag >> >> -- >> 2.15.1.421.gc469ca1de >> > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-12 18:43 ` SZEDER Gábor @ 2017-12-13 23:10 ` Lars Schneider 2017-12-14 23:51 ` SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: Lars Schneider @ 2017-12-13 23:10 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Git mailing list, Thomas Gummerer > On 12 Dec 2017, at 19:43, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > On Tue, Dec 12, 2017 at 7:00 PM, Lars Schneider > <larsxschneider@gmail.com> wrote: >> >>> On 12 Dec 2017, at 00:34, SZEDER Gábor <szeder.dev@gmail.com> wrote: >>> >>> While the build logic was embedded in our '.travis.yml', Travis CI >>> used to produce a nice trace log including all commands executed in >>> those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI >>> code into dedicated scripts, 2017-09-10), however, we only see the >>> name of the dedicated scripts, but not what those scripts are actually >>> doing, resulting in a less useful trace log. A patch later in this >>> series will move setting environment variables from '.travis.yml' to >>> the 'ci/*' scripts, so not even those will be included in the trace >>> log. >>> >>> Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other >>> 'ci/*' scripts, so we get trace log about the commands executed in all >>> of those scripts. >> >> I kind of did that intentionally to avoid clutter in the logs. >> However, I also agree with your reasoning. Therefore, the change >> looks good to me! > > Great, 'cause I'm starting to have second thoughts about this change :) > > It sure helped a lot while I worked on this patch series and a couple of > other Travis CI related patches (will submit them later)... OTOH it > definitely creates clutter in the trace log. The worst offender might > be 'ci/print-test-failures.sh', which iterates over all > 't/test-results/*.exit' files to find which tests failed and to show > their output, and 'set -x' makes every iteration visible. And we have > about 800 tests, which means 800 iterations. Yuck. > > Perhaps we should use other means to show what's going on instead, e.g. > use more 'echo's and '--verbose' options, or just avoid using '--quiet'. > And if some brave souls really want to tweak '.travis.yml' or the 'ci/*' > scripts, then they can set 'set -x' for themselves during development... > as the patch below shows it's easy enough, just a single character :) Hm... in that case. Would it be an option to "set -x" only in the header of "install-dependencies.sh"? In "lib-travisci.sh" we could keep your "set -x" and execute "set +x" at the end of the file. Wouldn't that give us the interesting traces without much clutter (e.g. what is $PATH etc)? - Lars > > > Gábor > > >>> >>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> >>> --- >>> ci/lib-travisci.sh | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh >>> index ac05f1f46..a0c8ae03f 100755 >>> --- a/ci/lib-travisci.sh >>> +++ b/ci/lib-travisci.sh >>> @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () { >>> >>> # Set 'exit on error' for all CI scripts to let the caller know that >>> # something went wrong >>> -set -e >>> +set -ex >>> >>> skip_branch_tip_with_tag >>> >>> -- >>> 2.15.1.421.gc469ca1de >>> >> ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-13 23:10 ` Lars Schneider @ 2017-12-14 23:51 ` SZEDER Gábor 2017-12-15 12:10 ` Johannes Schindelin 0 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-14 23:51 UTC (permalink / raw) To: Lars Schneider; +Cc: Git mailing list, Thomas Gummerer, Johannes Schindelin On Thu, Dec 14, 2017 at 12:10 AM, Lars Schneider <larsxschneider@gmail.com> wrote: > >> On 12 Dec 2017, at 19:43, SZEDER Gábor <szeder.dev@gmail.com> wrote: >> >> On Tue, Dec 12, 2017 at 7:00 PM, Lars Schneider >> <larsxschneider@gmail.com> wrote: >>> >>>> On 12 Dec 2017, at 00:34, SZEDER Gábor <szeder.dev@gmail.com> wrote: >>>> >>>> While the build logic was embedded in our '.travis.yml', Travis CI >>>> used to produce a nice trace log including all commands executed in >>>> those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI >>>> code into dedicated scripts, 2017-09-10), however, we only see the >>>> name of the dedicated scripts, but not what those scripts are actually >>>> doing, resulting in a less useful trace log. A patch later in this >>>> series will move setting environment variables from '.travis.yml' to >>>> the 'ci/*' scripts, so not even those will be included in the trace >>>> log. >>>> >>>> Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other >>>> 'ci/*' scripts, so we get trace log about the commands executed in all >>>> of those scripts. >>> >>> I kind of did that intentionally to avoid clutter in the logs. >>> However, I also agree with your reasoning. Therefore, the change >>> looks good to me! >> >> Great, 'cause I'm starting to have second thoughts about this change :) >> >> It sure helped a lot while I worked on this patch series and a couple of >> other Travis CI related patches (will submit them later)... OTOH it >> definitely creates clutter in the trace log. The worst offender might >> be 'ci/print-test-failures.sh', which iterates over all >> 't/test-results/*.exit' files to find which tests failed and to show >> their output, and 'set -x' makes every iteration visible. And we have >> about 800 tests, which means 800 iterations. Yuck. >> >> Perhaps we should use other means to show what's going on instead, e.g. >> use more 'echo's and '--verbose' options, or just avoid using '--quiet'. >> And if some brave souls really want to tweak '.travis.yml' or the 'ci/*' >> scripts, then they can set 'set -x' for themselves during development... >> as the patch below shows it's easy enough, just a single character :) > > Hm... in that case. Would it be an option to "set -x" only in the header > of "install-dependencies.sh"? > > In "lib-travisci.sh" we could keep your "set -x" and execute > "set +x" at the end of the file. Wouldn't that give us the > interesting traces without much clutter (e.g. what is $PATH etc)? Hmm, that's an idea worth considering. Scripts like 'run-build.sh', 'run-tests.sh' and 'run-static-analysis.sh' do basically nothing more than run make with different targets, so on one hand 'set -x' doesn't cause any clutter in the trace log, on the other hand there is no benefit from it either. 'run-linux32-docker.sh' runs docker (the command) twice, so it's basically in the same boat. I think both 'lib-travisci.sh' and 'install-dependencies.sh' benefit from 'set -x'. So does 'test-documentation.sh': it executes about 15 commands, among them a bunch of 'test -s <file>' which fail quietly. With 'set -x' we would see the last executed command and know that that's the one that failed. As mentioned above, 'print-test-failures.sh' definitely suffers from 'set -x'. There is a lot going on in 'run-windows-build.sh', so the output of 'set -x' might be useful or might be considered too much clutter, I don't know. I put Dscho on Cc, I think it's mainly his call. So it seems that there are more scripts that would benefit from tracing executed command using 'set -x' than scripts that would suffer because of it, and it doesn't matter for the rest. This means we could issue a 'set -x' in 'lib-travisci.sh' and disable it only in 'print-test-failures.sh'. There is one thing that triggers my OCD: whenever we echo something, it ends up being duplicated in the trace log, e.g.: + echo foo bar baz foo bar baz We could workaround it by writing 'echo "<msg>" >/dev/null', but it feels hackish and I'm not sure it's worth it. Gábor >>>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> >>>> --- >>>> ci/lib-travisci.sh | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh >>>> index ac05f1f46..a0c8ae03f 100755 >>>> --- a/ci/lib-travisci.sh >>>> +++ b/ci/lib-travisci.sh >>>> @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () { >>>> >>>> # Set 'exit on error' for all CI scripts to let the caller know that >>>> # something went wrong >>>> -set -e >>>> +set -ex >>>> >>>> skip_branch_tip_with_tag >>>> >>>> -- >>>> 2.15.1.421.gc469ca1de >>>> >>> > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-14 23:51 ` SZEDER Gábor @ 2017-12-15 12:10 ` Johannes Schindelin 2017-12-15 13:06 ` SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: Johannes Schindelin @ 2017-12-15 12:10 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Lars Schneider, Git mailing list, Thomas Gummerer Hi, > There is a lot going on in 'run-windows-build.sh', so the output of 'set > -x' might be useful or might be considered too much clutter, I don't > know. I put Dscho on Cc, I think it's mainly his call. Certainly it might be useful. However, please make sure that the secret token is not leaked that way. Like, *really* sure. Due to the failure of Git to use a portable and performant test suite, it does take about 90 minutes to build and test a revision, therefore it would be very easy to DOS my build system, and I really, really need it not to be DOSed because I use it in my day job, too. Ciao, Dscho ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-15 12:10 ` Johannes Schindelin @ 2017-12-15 13:06 ` SZEDER Gábor 2017-12-15 15:32 ` Johannes Schindelin 0 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-15 13:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Lars Schneider, Git mailing list, Thomas Gummerer On Fri, Dec 15, 2017 at 1:10 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > Hi, > >> There is a lot going on in 'run-windows-build.sh', so the output of 'set >> -x' might be useful or might be considered too much clutter, I don't >> know. I put Dscho on Cc, I think it's mainly his call. > > Certainly it might be useful. > > However, please make sure that the secret token is not leaked that way. > Like, *really* sure. Due to the failure of Git to use a portable and > performant test suite, it does take about 90 minutes to build and test a > revision, therefore it would be very easy to DOS my build system, and I > really, really need it not to be DOSed because I use it in my day job, too. Ugh, I was completely unaware of this issue, and the first version of this patch is already in 'pu'... **runs to check the trace logs** Great, it seems we are in luck, as the secret token was specified as an encrypted environment variable in git/git repository settings on Travis CI. It's redacted in the trace log and I only see lines like these: Setting environment variables from repository settings $ export GFW_CI_TOKEN=[secure] +test -z [secure] +++curl -H 'Authentication: Bearer [secure]' --silent --retry 5 --write-out '%{HTTP_CODE}' --output /dev/fd/63 'https://git-for-windows-ci.azurewebsites.net/api/TestNow?action=trigger&branch=pu&commit=1229713f78cd2883798e95f33c19c81b523413fd&skipTests=false' https://travis-ci.org/git/git/jobs/316791071 Phew. However, I don't feel competent enough with Travis CI's encrypted environment variables to say that this qualifies as "*really* sure" "that the secret token is not leaked". Anyway, note, that that '$ export GFW_CI_TOKEN=[secure]' line is already present in all 'git/git' trace logs independently of this 'set -x' change in question. Gábor ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output 2017-12-15 13:06 ` SZEDER Gábor @ 2017-12-15 15:32 ` Johannes Schindelin 0 siblings, 0 replies; 44+ messages in thread From: Johannes Schindelin @ 2017-12-15 15:32 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Lars Schneider, Git mailing list, Thomas Gummerer [-- Attachment #1: Type: text/plain, Size: 1214 bytes --] Hi Gábor, On Fri, 15 Dec 2017, SZEDER Gábor wrote: > On Fri, Dec 15, 2017 at 1:10 PM, Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > > >> There is a lot going on in 'run-windows-build.sh', so the output of > >> 'set -x' might be useful or might be considered too much clutter, I > >> don't know. I put Dscho on Cc, I think it's mainly his call. > > > > Certainly it might be useful. > > > > However, please make sure that the secret token is not leaked that > > way. Like, *really* sure. Due to the failure of Git to use a portable > > and performant test suite, it does take about 90 minutes to build and > > test a revision, therefore it would be very easy to DOS my build > > system, and I really, really need it not to be DOSed because I use it > > in my day job, too. > > Ugh, I was completely unaware of this issue, and the first version of > this patch is already in 'pu'... **runs to check the trace logs** > > Great, it seems we are in luck, as the secret token was specified as an > encrypted environment variable in git/git repository settings on Travis > CI. It's redacted in the trace log [...] That's good enough. Thanks for making sure, Dscho ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 2/4] travis-ci: introduce a $jobname variable for 'ci/*' scripts 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor 2017-12-11 23:34 ` [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output SZEDER Gábor @ 2017-12-11 23:34 ` SZEDER Gábor 2017-12-11 23:34 ` [PATCH 3/4] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' SZEDER Gábor ` (2 subsequent siblings) 4 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-11 23:34 UTC (permalink / raw) To: git; +Cc: Lars Schneider, Thomas Gummerer, SZEDER Gábor A couple of 'ci/*' scripts are shared between different build jobs: 'ci/lib-travisci.sh', being a common library, is sourced from almost every script, while 'ci/install-dependencies.sh', 'ci/run-build.sh' and 'ci/run-tests.sh' are shared between the "regular" GCC and Clang Linux and OSX build jobs, and the latter two scripts are used in the GETTEXT_POISON Linux build job as well. Our builds could benefit from these shared scripts being able to easily tell which build job they are taking part in. Now, it's already quite easy to tell apart Linux vs OSX and GCC vs Clang build jobs, but it gets trickier with all the additional Linux-based build jobs included explicitly in the build matrix. Unfortunately, Travis CI doesn't provide much help in this regard. The closest we've got is the $TRAVIS_JOB_NUMBER variable, the value of which is two dot-separated integers, where the second integer indicates a particular build job. While it would be possible to use that second number to identify the build job in our shared scripts, it doesn't seem like a good idea to rely on that: - Though the build job numbering sequence seems to be stable so far, Travis CI's documentation doesn't explicitly states that it is indeed stable and will remain so in the future. And even if it were stable, - if we were to remove or insert a build job in the middle, then the job numbers of all subsequent build jobs would change accordingly. So roll our own means of simple build job identification and introduce the $jobname environment variable in our builds, setting it in the environments of the explicitly included jobs in '.travis.yml', while constructing one in 'ci/lib-travisci.sh' as the combination of the OS and compiler name for the GCC and Clang Linux and OSX build jobs. Use $jobname instead of $TRAVIS_OS_NAME in scripts taking different actions based on the OS and build job (when installing P4 and Git LFS dependencies and including them in $PATH). The following two patches will also rely on $jobname. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 10 +++++----- ci/install-dependencies.sh | 6 +++--- ci/lib-travisci.sh | 9 +++++++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 281f101f3..88435e11c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,12 +39,12 @@ env: matrix: include: - - env: GETTEXT_POISON=YesPlease + - env: jobname=GETTEXT_POISON GETTEXT_POSION=YesPlease os: linux compiler: addons: before_install: - - env: Windows + - env: jobname=Windows os: linux compiler: addons: @@ -55,7 +55,7 @@ matrix: test "$TRAVIS_REPO_SLUG" != "git/git" || ci/run-windows-build.sh $TRAVIS_BRANCH $(git rev-parse HEAD) after_failure: - - env: Linux32 + - env: jobname=Linux32 os: linux compiler: services: @@ -63,7 +63,7 @@ matrix: before_install: before_script: script: ci/run-linux32-docker.sh - - env: Static Analysis + - env: jobname=StaticAnalysis os: linux compiler: addons: @@ -74,7 +74,7 @@ matrix: before_script: script: ci/run-static-analysis.sh after_failure: - - env: Documentation + - env: jobname=Documentation os: linux compiler: addons: diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 5bd06fe90..468788566 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -8,8 +8,8 @@ P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION -case "${TRAVIS_OS_NAME:-linux}" in -linux) +case "$jobname" in +linux-clang|linux-gcc) export GIT_TEST_HTTPD=YesPlease mkdir --parents "$P4_PATH" @@ -26,7 +26,7 @@ linux) cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs . popd ;; -osx) +osx-clang|osx-gcc) brew update --quiet # Uncomment this if you want to run perf tests: # brew install gnu-time diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index a0c8ae03f..b60e93797 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -27,8 +27,13 @@ set -ex skip_branch_tip_with_tag -case "${TRAVIS_OS_NAME:-linux}" in -linux) +if test -z "$jobname" +then + jobname="$TRAVIS_OS_NAME-$CC" +fi + +case "$jobname" in +linux-clang|linux-gcc) P4_PATH="$(pwd)/custom/p4" GIT_LFS_PATH="$(pwd)/custom/git-lfs" export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" -- 2.15.1.421.gc469ca1de ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH 3/4] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor 2017-12-11 23:34 ` [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output SZEDER Gábor 2017-12-11 23:34 ` [PATCH 2/4] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor @ 2017-12-11 23:34 ` SZEDER Gábor 2017-12-11 23:34 ` [PATCH 4/4] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 0/8] Travis CI cleanups SZEDER Gábor 4 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-11 23:34 UTC (permalink / raw) To: git; +Cc: Lars Schneider, Thomas Gummerer, SZEDER Gábor Our '.travis.yml's 'env.global' section sets a bunch of environment variables for all build jobs, though none of them actually affects all build jobs. It's convenient for us, and in most cases it works just fine, because irrelevant environment variables are simply ignored. However, $GIT_SKIP_TESTS is an exception: it tells the test harness to skip the two test scripts that are prone to occasional failures on OSX, but as it's set for all build jobs those tests are not run in any of the build jobs that are capable to run them reliably, either. Therefore $GIT_SKIP_TESTS should only be set in the OSX build jobs, but those build jobs are included in the build matrix implicitly (i.e. by combining the matrix keys 'os' and 'compiler'), and there is no way to set an environment variable only for a subset of those implicit build jobs. (Unless we were to add new scriptlets to '.travis.yml', which is exactly the opposite direction that we took with commit 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10)). So move setting $GIT_SKIP_TESTS to 'ci/lib-travisci.sh', where it can trivially be set only for the OSX build jobs. Furthermore, move setting all other environment variables from '.travis.yml' to 'ci/lib-travisci.sh', too, because a couple of environment variables are already set there, and this way all environment variables will be set in the same place. All the logic controlling our builds is already in the 'ci/*' scripts anyway, so there is really no good reason to keep the environment variables separately. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 18 +----------------- ci/lib-travisci.sh | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index 88435e11c..7c9aa0557 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,25 +21,9 @@ addons: - git-svn - apache2 -env: - global: - - DEVELOPER=1 - # The Linux build installs the defined dependency versions below. - # The OS X build installs the latest available versions. Keep that - # in mind when you encounter a broken OS X build! - - LINUX_P4_VERSION="16.2" - - LINUX_GIT_LFS_VERSION="1.5.2" - - DEFAULT_TEST_TARGET=prove - - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save" - - GIT_TEST_OPTS="--verbose-log" - - GIT_TEST_CLONE_2GB=YesPlease - # t9810 occasionally fails on Travis CI OS X - # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI OS X - - GIT_SKIP_TESTS="t9810 t9816" - matrix: include: - - env: jobname=GETTEXT_POISON GETTEXT_POSION=YesPlease + - env: jobname=GETTEXT_POISON os: linux compiler: addons: diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index b60e93797..e85571298 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -32,10 +32,31 @@ then jobname="$TRAVIS_OS_NAME-$CC" fi +export DEVELOPER=1 +export DEFAULT_TEST_TARGET=prove +export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save" +export GIT_TEST_OPTS="--verbose-log" +export GIT_TEST_CLONE_2GB=YesPlease + case "$jobname" in linux-clang|linux-gcc) + # The Linux build installs the defined dependency versions below. + # The OS X build installs the latest available versions. Keep that + # in mind when you encounter a broken OS X build! + export LINUX_P4_VERSION="16.2" + export LINUX_GIT_LFS_VERSION="1.5.2" + P4_PATH="$(pwd)/custom/p4" GIT_LFS_PATH="$(pwd)/custom/git-lfs" export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" ;; +osx-clang|osx-gcc) + # t9810 occasionally fails on Travis CI OS X + # t9816 occasionally fails with "TAP out of sequence errors" on + # Travis CI OS X + export GIT_SKIP_TESTS="t9810 t9816" + ;; +GETTEXT_POISON) + export GETTEXT_POISON=YesPlease + ;; esac -- 2.15.1.421.gc469ca1de ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH 4/4] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor ` (2 preceding siblings ...) 2017-12-11 23:34 ` [PATCH 3/4] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' SZEDER Gábor @ 2017-12-11 23:34 ` SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 0/8] Travis CI cleanups SZEDER Gábor 4 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-11 23:34 UTC (permalink / raw) To: git; +Cc: Lars Schneider, Thomas Gummerer, SZEDER Gábor Commit 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10) converted '.travis.yml's default 'before_install' scriptlet to the 'ci/install-dependencies.sh' script, and while doing so moved setting GIT_TEST_HTTPD=YesPlease for the 64-bit GCC and Clang Linux build jobs to that script. This is wrong for two reasons: - The purpose of that script is, as its name suggests, to install dependencies, not to set any environment variables influencing which tests should be run (though, arguably, this was already an issue with the original 'before_install' scriptlet). - Setting the variable has no effect anymore, because that script is run in a separate shell process, and the variable won't be visible in any of the other scripts, notably in 'ci/run-tests.sh' responsible for, well, running the tests. Luckily, this didn't have a negative effect on our Travis CI build jobs, because GIT_TEST_HTTPD is a tri-state variable defaulting to "auto" and a functioning web server was installed in those Linux build jobs, so the httpd tests were run anyway. Apparently the httpd tests run just fine without GIT_TEST_HTTPD being set, therefore we could simply remove this environment variable. However, if a bug were to creep in to change the Travis CI build environment to run the tests as root or to not install Apache, then the httpd tests would be skipped and the build job would still succeed. We would only notice if someone actually were to look through the build job's trace log; but who would look at the trace log of a successful build job?! Since httpd tests are important, we do want to run them and we want to be loudly reminded if they can't be run. Therefore, move setting GIT_TEST_HTTPD=YesPlease for the 64-bit GCC and Clang Linux build jobs to 'ci/lib-travisci.sh' to ensure that the build job fails when the httpd tests can't be run. (We could set it in 'ci/run-tests.sh' just as well, but it's better to keep all environment variables in one place in 'ci/lib-travisci.sh'.) Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/install-dependencies.sh | 2 -- ci/lib-travisci.sh | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 468788566..75a9fd247 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -10,8 +10,6 @@ LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VE case "$jobname" in linux-clang|linux-gcc) - export GIT_TEST_HTTPD=YesPlease - mkdir --parents "$P4_PATH" pushd "$P4_PATH" wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index e85571298..331d3eb3a 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -40,6 +40,8 @@ export GIT_TEST_CLONE_2GB=YesPlease case "$jobname" in linux-clang|linux-gcc) + export GIT_TEST_HTTPD=YesPlease + # The Linux build installs the defined dependency versions below. # The OS X build installs the latest available versions. Keep that # in mind when you encounter a broken OS X build! -- 2.15.1.421.gc469ca1de ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 0/8] Travis CI cleanups 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor ` (3 preceding siblings ...) 2017-12-11 23:34 ` [PATCH 4/4] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' SZEDER Gábor @ 2017-12-16 12:54 ` SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (2 more replies) 4 siblings, 3 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:54 UTC (permalink / raw) To: Junio C Hamano Cc: Lars Schneider, Johannes Schindelin, git, SZEDER Gábor This is a reroll of 'sg/travis-fixes'. Changes since the previous round: - Patch 1 got updated following the discussion: - I went with enabling tracing executed commands everywhere, including the Windows build job, except where we know it causes way too much clutter, which is currently only 'ci/print-test-failures.sh'. - Also enable this tracing in 'ci/run-linux32-build.sh', which doesn't source 'ci/lib-travisci.sh' as it's run inside a Docker container. - The commit message got updated accordingly, including a note about the Windows build job's secret token. I would like to get an Acked-by: from Dscho on this patch before it gets merged. - Patches 5-8 are new. They are various fixes/cleanups unrelated to the Travis CI scriptification, but I don't think it's worth to have them in separate patch series. SZEDER Gábor (8): travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing travis-ci: introduce a $jobname variable for 'ci/*' scripts travis-ci: move setting environment variables to 'ci/lib-travisci.sh' travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' travis-ci: don't install default addon packages for the 32 bit Linux build travis-ci: don't install 'language-pack-is' package travis-ci: save prove state for the 32 bit Linux build travis-ci: only print test failures if there are test results available .travis.yml | 28 ++++++---------------------- ci/install-dependencies.sh | 8 +++----- ci/lib-travisci.sh | 38 ++++++++++++++++++++++++++++++++++---- ci/print-test-failures.sh | 9 +++++++++ ci/run-linux32-build.sh | 3 +++ ci/run-linux32-docker.sh | 1 + 6 files changed, 56 insertions(+), 31 deletions(-) -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing 2017-12-16 12:54 ` [PATCH v2 0/8] Travis CI cleanups SZEDER Gábor @ 2017-12-16 12:54 ` SZEDER Gábor 2017-12-16 12:55 ` [PATCH v2 2/8] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor ` (8 more replies) 2017-12-18 21:46 ` [PATCH v2 0/8] Travis CI cleanups Lars Schneider 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor 2 siblings, 9 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:54 UTC (permalink / raw) To: Junio C Hamano Cc: Lars Schneider, Johannes Schindelin, git, SZEDER Gábor While the build logic was embedded in our '.travis.yml', Travis CI used to produce a nice trace log including all commands executed in those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10), however, we only see the name of the dedicated scripts, but not what those scripts are actually doing, resulting in a less useful trace log about e.g. installing dependencies. A patch later in this series will move setting environment variables from '.travis.yml' to 'ci/lib-travisci.sh', so not even those will be included in the trace log. Unrelated to 657343a60, 'ci/test-documentation.sh' runs a bunch of 'test -s <file>' checks which would fail quietly if something were wrong, leaving no clue about which one of those checks triggered the failure. Use 'set -x' in 'ci/lib-travisci.sh' to get more detailed trace log about the commands executed in the 'ci/*' scripts. Use it in 'ci/run-linux32-build.sh' as well, which is run in a Docker container and therefore doesn't source 'ci/lib-travisci.sh'. The secret token used for the Windows builds is specified as an encrypted environment variable in git/git repository settings on Travis CI and it's redacted in the trace logs even with 'set -x'. However, disable this tracing in 'ci/print-test-failures.sh', as it produces far too much noise in the output of that script. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/lib-travisci.sh | 6 ++++-- ci/print-test-failures.sh | 3 +++ ci/run-linux32-build.sh | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index ac05f1f46..2d0d1d613 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -22,8 +22,10 @@ skip_branch_tip_with_tag () { } # Set 'exit on error' for all CI scripts to let the caller know that -# something went wrong -set -e +# something went wrong. +# Set tracing executed commands, primarily setting environment variables +# and installing dependencies. +set -ex skip_branch_tip_with_tag diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh index 8c8973cbf..f757e616c 100755 --- a/ci/print-test-failures.sh +++ b/ci/print-test-failures.sh @@ -5,6 +5,9 @@ . ${0%/*}/lib-travisci.sh +# Tracing executed commands would produce too much noise in this script. +set +x + for TEST_EXIT in t/test-results/*.exit do if [ "$(cat "$TEST_EXIT")" != "0" ] diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index e30fb2cdd..a8518eddf 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -6,6 +6,8 @@ # run-linux32-build.sh [host-user-id] # +set -x + # Update packages to the latest available versions linux32 --32bit i386 sh -c ' apt update >/dev/null && -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 2/8] travis-ci: introduce a $jobname variable for 'ci/*' scripts 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor @ 2017-12-16 12:55 ` SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 3/8] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' SZEDER Gábor ` (7 subsequent siblings) 8 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:55 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor A couple of 'ci/*' scripts are shared between different build jobs: 'ci/lib-travisci.sh', being a common library, is sourced from almost every script, while 'ci/install-dependencies.sh', 'ci/run-build.sh' and 'ci/run-tests.sh' are shared between the "regular" GCC and Clang Linux and OSX build jobs, and the latter two scripts are used in the GETTEXT_POISON Linux build job as well. Our builds could benefit from these shared scripts being able to easily tell which build job they are taking part in. Now, it's already quite easy to tell apart Linux vs OSX and GCC vs Clang build jobs, but it gets trickier with all the additional Linux-based build jobs included explicitly in the build matrix. Unfortunately, Travis CI doesn't provide much help in this regard. The closest we've got is the $TRAVIS_JOB_NUMBER variable, the value of which is two dot-separated integers, where the second integer indicates a particular build job. While it would be possible to use that second number to identify the build job in our shared scripts, it doesn't seem like a good idea to rely on that: - Though the build job numbering sequence seems to be stable so far, Travis CI's documentation doesn't explicitly states that it is indeed stable and will remain so in the future. And even if it were stable, - if we were to remove or insert a build job in the middle, then the job numbers of all subsequent build jobs would change accordingly. So roll our own means of simple build job identification and introduce the $jobname environment variable in our builds, setting it in the environments of the explicitly included jobs in '.travis.yml', while constructing one in 'ci/lib-travisci.sh' as the combination of the OS and compiler name for the GCC and Clang Linux and OSX build jobs. Use $jobname instead of $TRAVIS_OS_NAME in scripts taking different actions based on the OS and build job (when installing P4 and Git LFS dependencies and including them in $PATH). The following two patches will also rely on $jobname. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 10 +++++----- ci/install-dependencies.sh | 6 +++--- ci/lib-travisci.sh | 9 +++++++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 281f101f3..88435e11c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,12 +39,12 @@ env: matrix: include: - - env: GETTEXT_POISON=YesPlease + - env: jobname=GETTEXT_POISON GETTEXT_POSION=YesPlease os: linux compiler: addons: before_install: - - env: Windows + - env: jobname=Windows os: linux compiler: addons: @@ -55,7 +55,7 @@ matrix: test "$TRAVIS_REPO_SLUG" != "git/git" || ci/run-windows-build.sh $TRAVIS_BRANCH $(git rev-parse HEAD) after_failure: - - env: Linux32 + - env: jobname=Linux32 os: linux compiler: services: @@ -63,7 +63,7 @@ matrix: before_install: before_script: script: ci/run-linux32-docker.sh - - env: Static Analysis + - env: jobname=StaticAnalysis os: linux compiler: addons: @@ -74,7 +74,7 @@ matrix: before_script: script: ci/run-static-analysis.sh after_failure: - - env: Documentation + - env: jobname=Documentation os: linux compiler: addons: diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 5bd06fe90..468788566 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -8,8 +8,8 @@ P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION -case "${TRAVIS_OS_NAME:-linux}" in -linux) +case "$jobname" in +linux-clang|linux-gcc) export GIT_TEST_HTTPD=YesPlease mkdir --parents "$P4_PATH" @@ -26,7 +26,7 @@ linux) cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs . popd ;; -osx) +osx-clang|osx-gcc) brew update --quiet # Uncomment this if you want to run perf tests: # brew install gnu-time diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index 2d0d1d613..4b3c5fdd0 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -29,8 +29,13 @@ set -ex skip_branch_tip_with_tag -case "${TRAVIS_OS_NAME:-linux}" in -linux) +if test -z "$jobname" +then + jobname="$TRAVIS_OS_NAME-$CC" +fi + +case "$jobname" in +linux-clang|linux-gcc) P4_PATH="$(pwd)/custom/p4" GIT_LFS_PATH="$(pwd)/custom/git-lfs" export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 3/8] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor 2017-12-16 12:55 ` [PATCH v2 2/8] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor @ 2017-12-16 12:57 ` SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 4/8] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' SZEDER Gábor ` (6 subsequent siblings) 8 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor Our '.travis.yml's 'env.global' section sets a bunch of environment variables for all build jobs, though none of them actually affects all build jobs. It's convenient for us, and in most cases it works just fine, because irrelevant environment variables are simply ignored. However, $GIT_SKIP_TESTS is an exception: it tells the test harness to skip the two test scripts that are prone to occasional failures on OSX, but as it's set for all build jobs those tests are not run in any of the build jobs that are capable to run them reliably, either. Therefore $GIT_SKIP_TESTS should only be set in the OSX build jobs, but those build jobs are included in the build matrix implicitly (i.e. by combining the matrix keys 'os' and 'compiler'), and there is no way to set an environment variable only for a subset of those implicit build jobs. (Unless we were to add new scriptlets to '.travis.yml', which is exactly the opposite direction that we took with commit 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10)). So move setting $GIT_SKIP_TESTS to 'ci/lib-travisci.sh', where it can trivially be set only for the OSX build jobs. Furthermore, move setting all other environment variables from '.travis.yml' to 'ci/lib-travisci.sh', too, because a couple of environment variables are already set there, and this way all environment variables will be set in the same place. All the logic controlling our builds is already in the 'ci/*' scripts anyway, so there is really no good reason to keep the environment variables separately. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 18 +----------------- ci/lib-travisci.sh | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index 88435e11c..7c9aa0557 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,25 +21,9 @@ addons: - git-svn - apache2 -env: - global: - - DEVELOPER=1 - # The Linux build installs the defined dependency versions below. - # The OS X build installs the latest available versions. Keep that - # in mind when you encounter a broken OS X build! - - LINUX_P4_VERSION="16.2" - - LINUX_GIT_LFS_VERSION="1.5.2" - - DEFAULT_TEST_TARGET=prove - - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save" - - GIT_TEST_OPTS="--verbose-log" - - GIT_TEST_CLONE_2GB=YesPlease - # t9810 occasionally fails on Travis CI OS X - # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI OS X - - GIT_SKIP_TESTS="t9810 t9816" - matrix: include: - - env: jobname=GETTEXT_POISON GETTEXT_POSION=YesPlease + - env: jobname=GETTEXT_POISON os: linux compiler: addons: diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index 4b3c5fdd0..c5c5cb1bf 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -34,10 +34,31 @@ then jobname="$TRAVIS_OS_NAME-$CC" fi +export DEVELOPER=1 +export DEFAULT_TEST_TARGET=prove +export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save" +export GIT_TEST_OPTS="--verbose-log" +export GIT_TEST_CLONE_2GB=YesPlease + case "$jobname" in linux-clang|linux-gcc) + # The Linux build installs the defined dependency versions below. + # The OS X build installs the latest available versions. Keep that + # in mind when you encounter a broken OS X build! + export LINUX_P4_VERSION="16.2" + export LINUX_GIT_LFS_VERSION="1.5.2" + P4_PATH="$(pwd)/custom/p4" GIT_LFS_PATH="$(pwd)/custom/git-lfs" export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" ;; +osx-clang|osx-gcc) + # t9810 occasionally fails on Travis CI OS X + # t9816 occasionally fails with "TAP out of sequence errors" on + # Travis CI OS X + export GIT_SKIP_TESTS="t9810 t9816" + ;; +GETTEXT_POISON) + export GETTEXT_POISON=YesPlease + ;; esac -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 4/8] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor 2017-12-16 12:55 ` [PATCH v2 2/8] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 3/8] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' SZEDER Gábor @ 2017-12-16 12:57 ` SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 5/8] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor ` (5 subsequent siblings) 8 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor Commit 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10) converted '.travis.yml's default 'before_install' scriptlet to the 'ci/install-dependencies.sh' script, and while doing so moved setting GIT_TEST_HTTPD=YesPlease for the 64-bit GCC and Clang Linux build jobs to that script. This is wrong for two reasons: - The purpose of that script is, as its name suggests, to install dependencies, not to set any environment variables influencing which tests should be run (though, arguably, this was already an issue with the original 'before_install' scriptlet). - Setting the variable has no effect anymore, because that script is run in a separate shell process, and the variable won't be visible in any of the other scripts, notably in 'ci/run-tests.sh' responsible for, well, running the tests. Luckily, this didn't have a negative effect on our Travis CI build jobs, because GIT_TEST_HTTPD is a tri-state variable defaulting to "auto" and a functioning web server was installed in those Linux build jobs, so the httpd tests were run anyway. Apparently the httpd tests run just fine without GIT_TEST_HTTPD being set, therefore we could simply remove this environment variable. However, if a bug were to creep in to change the Travis CI build environment to run the tests as root or to not install Apache, then the httpd tests would be skipped and the build job would still succeed. We would only notice if someone actually were to look through the build job's trace log; but who would look at the trace log of a successful build job?! Since httpd tests are important, we do want to run them and we want to be loudly reminded if they can't be run. Therefore, move setting GIT_TEST_HTTPD=YesPlease for the 64-bit GCC and Clang Linux build jobs to 'ci/lib-travisci.sh' to ensure that the build job fails when the httpd tests can't be run. (We could set it in 'ci/run-tests.sh' just as well, but it's better to keep all environment variables in one place in 'ci/lib-travisci.sh'.) Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/install-dependencies.sh | 2 -- ci/lib-travisci.sh | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 468788566..75a9fd247 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -10,8 +10,6 @@ LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VE case "$jobname" in linux-clang|linux-gcc) - export GIT_TEST_HTTPD=YesPlease - mkdir --parents "$P4_PATH" pushd "$P4_PATH" wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index c5c5cb1bf..348fe3c3c 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -42,6 +42,8 @@ export GIT_TEST_CLONE_2GB=YesPlease case "$jobname" in linux-clang|linux-gcc) + export GIT_TEST_HTTPD=YesPlease + # The Linux build installs the defined dependency versions below. # The OS X build installs the latest available versions. Keep that # in mind when you encounter a broken OS X build! -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 5/8] travis-ci: don't install default addon packages for the 32 bit Linux build 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (2 preceding siblings ...) 2017-12-16 12:57 ` [PATCH v2 4/8] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' SZEDER Gábor @ 2017-12-16 12:57 ` SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package SZEDER Gábor ` (4 subsequent siblings) 8 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor The 32 bit Linux build job compiles Git and runs the test suite in a Docker container, while the additional packages (apache2, git-svn, language-pack-is) are installed on the host, therefore don't have any effect and are unnecessary. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 7c9aa0557..4684b3f4f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,6 +42,7 @@ matrix: - env: jobname=Linux32 os: linux compiler: + addons: services: - docker before_install: -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (3 preceding siblings ...) 2017-12-16 12:57 ` [PATCH v2 5/8] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor @ 2017-12-16 12:57 ` SZEDER Gábor 2017-12-18 21:33 ` Lars Schneider 2017-12-16 12:58 ` [PATCH v2 7/8] travis-ci: save prove state for the 32 bit Linux build SZEDER Gábor ` (3 subsequent siblings) 8 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor Ever since we have started to use Travis CI in 522354d70 (Add Travis CI support, 2015-11-27), our 64 bit Linux build jobs install the 'languate-pack-is' package. That commit doesn't discuss why it was deemed necessary back then, but Travis CI can build and test Git without that package just fine, even that commit introducing Travis CI support. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4684b3f4f..ea11b5af6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,6 @@ compiler: addons: apt: packages: - - language-pack-is - git-svn - apache2 -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package 2017-12-16 12:57 ` [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package SZEDER Gábor @ 2017-12-18 21:33 ` Lars Schneider 2017-12-18 22:04 ` SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: Lars Schneider @ 2017-12-18 21:33 UTC (permalink / raw) To: SZEDER Gábor, Ævar Arnfjörð Bjarmason Cc: Junio C Hamano, git > On 16 Dec 2017, at 13:57, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > Ever since we have started to use Travis CI in 522354d70 (Add Travis > CI support, 2015-11-27), our 64 bit Linux build jobs install the > 'languate-pack-is' package. That commit doesn't discuss why it was > deemed necessary back then, but Travis CI can build and test Git > without that package just fine, even that commit introducing Travis CI > support. If I remember correctly then we had to install the Icelandic language pack for the i18n tests (e.g. t0204): https://github.com/git/git/blob/master/t/lib-gettext.sh https://packages.ubuntu.com/trusty/language-pack-is However, I checked your Travis-CI and I cannot spot any errors: https://travis-ci.org/szeder/git/jobs/317494789 I am a bit puzzled. Maybe the Icelandic language pack was not part of the Ubuntu image that was used when we introduced Travis CI? The Ubuntu default image was 12.04 back then: https://travis-ci.org/git/git/jobs/100241871 Nowadays it is 14.04. @Avar: Do you know what kind of errors we should expect if the language pack is not installed? Thanks, Lars > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > .travis.yml | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/.travis.yml b/.travis.yml > index 4684b3f4f..ea11b5af6 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -17,7 +17,6 @@ compiler: > addons: > apt: > packages: > - - language-pack-is > - git-svn > - apache2 > > -- > 2.15.1.429.ga000dd9c7 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package 2017-12-18 21:33 ` Lars Schneider @ 2017-12-18 22:04 ` SZEDER Gábor 2017-12-18 22:17 ` Lars Schneider 2017-12-19 12:22 ` SZEDER Gábor 0 siblings, 2 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-18 22:04 UTC (permalink / raw) To: Lars Schneider Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Git mailing list On Mon, Dec 18, 2017 at 10:33 PM, Lars Schneider <larsxschneider@gmail.com> wrote: > >> On 16 Dec 2017, at 13:57, SZEDER Gábor <szeder.dev@gmail.com> wrote: >> >> Ever since we have started to use Travis CI in 522354d70 (Add Travis >> CI support, 2015-11-27), our 64 bit Linux build jobs install the >> 'languate-pack-is' package. That commit doesn't discuss why it was >> deemed necessary back then, but Travis CI can build and test Git >> without that package just fine, even that commit introducing Travis CI >> support. > > If I remember correctly then we had to install the Icelandic > language pack for the i18n tests (e.g. t0204): > > https://github.com/git/git/blob/master/t/lib-gettext.sh > https://packages.ubuntu.com/trusty/language-pack-is > > However, I checked your Travis-CI and I cannot spot any errors: > https://travis-ci.org/szeder/git/jobs/317494789 Ah, now I start to understand. I was only looking for errors, too, but there are none, because the tests requiring the language pack are protected by the appropriate prerequisites. On my box, without and with the language pack: $ ./t0204-gettext-reencode-sanity.sh # lib-gettext: No is_IS UTF-8 locale available # lib-gettext: No is_IS ISO-8859-1 locale available ok 1 # skip gettext: Emitting UTF-8 from our UTF-8 *.mo files / Icelandic (missing GETTEXT_LOCALE) ok 2 # skip gettext: Emitting UTF-8 from our UTF-8 *.mo files / Runes (missing GETTEXT_LOCALE) ok 3 # skip gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / Icelandic (missing GETTEXT_ISO_LOCALE) ok 4 # skip gettext: impossible ISO-8859-1 output (missing GETTEXT_ISO_LOCALE) ok 5 # skip gettext: Fetching a UTF-8 msgid -> UTF-8 (missing GETTEXT_LOCALE) ok 6 # skip gettext: Fetching a UTF-8 msgid -> ISO-8859-1 (missing GETTEXT_ISO_LOCALE) ok 7 # skip gettext.c: git init UTF-8 -> UTF-8 (missing GETTEXT_LOCALE) ok 8 # skip gettext.c: git init UTF-8 -> ISO-8859-1 (missing GETTEXT_ISO_LOCALE) # passed all 8 test(s) $ sudo apt-get install language-pack-is [...] $ ./t0204-gettext-reencode-sanity.sh # lib-gettext: Found 'is_IS.utf8' as an is_IS UTF-8 locale # lib-gettext: No is_IS ISO-8859-1 locale available ok 1 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Icelandic ok 2 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Runes ok 3 # skip gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / Icelandic (missing GETTEXT_ISO_LOCALE) ok 4 # skip gettext: impossible ISO-8859-1 output (missing GETTEXT_ISO_LOCALE) ok 5 - gettext: Fetching a UTF-8 msgid -> UTF-8 ok 6 # skip gettext: Fetching a UTF-8 msgid -> ISO-8859-1 (missing GETTEXT_ISO_LOCALE) ok 7 - gettext.c: git init UTF-8 -> UTF-8 ok 8 # skip gettext.c: git init UTF-8 -> ISO-8859-1 (missing GETTEXT_ISO_LOCALE) # passed all 8 test(s) 1..8 I'd expect something like this in the Travis CI build jobs as well, but prove hides the detailed output. It seems we would loose coverage with this patch, so it should be dropped. > I am a bit puzzled. Maybe the Icelandic language pack was not part > of the Ubuntu image that was used when we introduced Travis CI? > > The Ubuntu default image was 12.04 back then: > https://travis-ci.org/git/git/jobs/100241871 > > Nowadays it is 14.04. > > @Avar: > Do you know what kind of errors we should expect if the language > pack is not installed? > > Thanks, > Lars > >> >> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> >> --- >> .travis.yml | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/.travis.yml b/.travis.yml >> index 4684b3f4f..ea11b5af6 100644 >> --- a/.travis.yml >> +++ b/.travis.yml >> @@ -17,7 +17,6 @@ compiler: >> addons: >> apt: >> packages: >> - - language-pack-is >> - git-svn >> - apache2 >> >> -- >> 2.15.1.429.ga000dd9c7 >> > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package 2017-12-18 22:04 ` SZEDER Gábor @ 2017-12-18 22:17 ` Lars Schneider 2017-12-18 22:34 ` Junio C Hamano 2017-12-19 12:22 ` SZEDER Gábor 1 sibling, 1 reply; 44+ messages in thread From: Lars Schneider @ 2017-12-18 22:17 UTC (permalink / raw) To: SZEDER Gábor Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Git mailing list > On 18 Dec 2017, at 23:04, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > On Mon, Dec 18, 2017 at 10:33 PM, Lars Schneider > <larsxschneider@gmail.com> wrote: >> >>> On 16 Dec 2017, at 13:57, SZEDER Gábor <szeder.dev@gmail.com> wrote: >>> >>> Ever since we have started to use Travis CI in 522354d70 (Add Travis >>> CI support, 2015-11-27), our 64 bit Linux build jobs install the >>> 'languate-pack-is' package. That commit doesn't discuss why it was >>> deemed necessary back then, but Travis CI can build and test Git >>> without that package just fine, even that commit introducing Travis CI >>> support. >> >> If I remember correctly then we had to install the Icelandic >> language pack for the i18n tests (e.g. t0204): >> >> https://github.com/git/git/blob/master/t/lib-gettext.sh >> https://packages.ubuntu.com/trusty/language-pack-is >> >> However, I checked your Travis-CI and I cannot spot any errors: >> https://travis-ci.org/szeder/git/jobs/317494789 > > Ah, now I start to understand. I was only looking for errors, too, but > there are none, because the tests requiring the language pack are > protected by the appropriate prerequisites. On my box, without and > with the language pack: > > $ ./t0204-gettext-reencode-sanity.sh > # lib-gettext: No is_IS UTF-8 locale available > # lib-gettext: No is_IS ISO-8859-1 locale available > ok 1 # skip gettext: Emitting UTF-8 from our UTF-8 *.mo files / > Icelandic (missing GETTEXT_LOCALE) > ok 2 # skip gettext: Emitting UTF-8 from our UTF-8 *.mo files / > Runes (missing GETTEXT_LOCALE) > ok 3 # skip gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / > Icelandic (missing GETTEXT_ISO_LOCALE) > ok 4 # skip gettext: impossible ISO-8859-1 output (missing GETTEXT_ISO_LOCALE) > ok 5 # skip gettext: Fetching a UTF-8 msgid -> UTF-8 (missing GETTEXT_LOCALE) > ok 6 # skip gettext: Fetching a UTF-8 msgid -> ISO-8859-1 (missing > GETTEXT_ISO_LOCALE) > ok 7 # skip gettext.c: git init UTF-8 -> UTF-8 (missing GETTEXT_LOCALE) > ok 8 # skip gettext.c: git init UTF-8 -> ISO-8859-1 (missing > GETTEXT_ISO_LOCALE) > # passed all 8 test(s) > > $ sudo apt-get install language-pack-is > [...] > $ ./t0204-gettext-reencode-sanity.sh > # lib-gettext: Found 'is_IS.utf8' as an is_IS UTF-8 locale > # lib-gettext: No is_IS ISO-8859-1 locale available > ok 1 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Icelandic > ok 2 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Runes > ok 3 # skip gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / > Icelandic (missing GETTEXT_ISO_LOCALE) > ok 4 # skip gettext: impossible ISO-8859-1 output (missing GETTEXT_ISO_LOCALE) > ok 5 - gettext: Fetching a UTF-8 msgid -> UTF-8 > ok 6 # skip gettext: Fetching a UTF-8 msgid -> ISO-8859-1 (missing > GETTEXT_ISO_LOCALE) > ok 7 - gettext.c: git init UTF-8 -> UTF-8 > ok 8 # skip gettext.c: git init UTF-8 -> ISO-8859-1 (missing > GETTEXT_ISO_LOCALE) > # passed all 8 test(s) > 1..8 > > I'd expect something like this in the Travis CI build jobs as well, but > prove hides the detailed output. Ahh. That explains it! > It seems we would loose coverage with this patch, so it should be > dropped. Yeah. I think we should add a comment to the travis.yml to avoid future confusion. I'll do it unless you beat me to it with a re-roll. - Lars > >> I am a bit puzzled. Maybe the Icelandic language pack was not part >> of the Ubuntu image that was used when we introduced Travis CI? >> >> The Ubuntu default image was 12.04 back then: >> https://travis-ci.org/git/git/jobs/100241871 >> >> Nowadays it is 14.04. >> >> @Avar: >> Do you know what kind of errors we should expect if the language >> pack is not installed? >> >> Thanks, >> Lars >> >>> >>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> >>> --- >>> .travis.yml | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/.travis.yml b/.travis.yml >>> index 4684b3f4f..ea11b5af6 100644 >>> --- a/.travis.yml >>> +++ b/.travis.yml >>> @@ -17,7 +17,6 @@ compiler: >>> addons: >>> apt: >>> packages: >>> - - language-pack-is >>> - git-svn >>> - apache2 >>> >>> -- >>> 2.15.1.429.ga000dd9c7 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package 2017-12-18 22:17 ` Lars Schneider @ 2017-12-18 22:34 ` Junio C Hamano 0 siblings, 0 replies; 44+ messages in thread From: Junio C Hamano @ 2017-12-18 22:34 UTC (permalink / raw) To: Lars Schneider Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason, Git mailing list Lars Schneider <larsxschneider@gmail.com> writes: >> It seems we would loose coverage with this patch, so it should be >> dropped. > > Yeah. I think we should add a comment to the travis.yml to avoid > future confusion. I'll do it unless you beat me to it with a re-roll. Rather, it should be commented close to where is locale is used in tests, and possibly in t/README, I would think. Those who want to use t/*, not necessarily using Travis, would benefit from the hint, "installing icelandic locale may give you better test coverage", or something like that. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package 2017-12-18 22:04 ` SZEDER Gábor 2017-12-18 22:17 ` Lars Schneider @ 2017-12-19 12:22 ` SZEDER Gábor 1 sibling, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-19 12:22 UTC (permalink / raw) To: Lars Schneider Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Git mailing list On Mon, Dec 18, 2017 at 11:04 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote: > $ sudo apt-get install language-pack-is > [...] > $ ./t0204-gettext-reencode-sanity.sh > # lib-gettext: Found 'is_IS.utf8' as an is_IS UTF-8 locale > # lib-gettext: No is_IS ISO-8859-1 locale available > ok 1 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Icelandic > ok 2 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Runes > ok 3 # skip gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / > Icelandic (missing GETTEXT_ISO_LOCALE) > ok 4 # skip gettext: impossible ISO-8859-1 output (missing GETTEXT_ISO_LOCALE) > ok 5 - gettext: Fetching a UTF-8 msgid -> UTF-8 > ok 6 # skip gettext: Fetching a UTF-8 msgid -> ISO-8859-1 (missing > GETTEXT_ISO_LOCALE) > ok 7 - gettext.c: git init UTF-8 -> UTF-8 > ok 8 # skip gettext.c: git init UTF-8 -> ISO-8859-1 (missing > GETTEXT_ISO_LOCALE) > # passed all 8 test(s) > 1..8 > > I'd expect something like this in the Travis CI build jobs as well, but > prove hides the detailed output. > > It seems we would loose coverage with this patch, so it should be > dropped. Not so fast! Notice how there are still a few tests skipped above, because of missing GETTEXT_ISO_LOCALE. # grep is_IS /etc/locale.gen # is_IS ISO-8859-1 # is_IS.UTF-8 UTF-8 # sed -i -e 's/^# is_IS/is_IS/' /etc/locale.gen # locale-gen Generating locales (this might take a while)... [...] is_IS.ISO-8859-1... done is_IS.UTF-8... done Generation complete. Both UTF-8 and ISO Icelandic locales are generated, good. $ $ ./t0204-gettext-reencode-sanity.sh # lib-gettext: Found 'is_IS.utf8' as an is_IS UTF-8 locale # lib-gettext: Found 'is_IS.iso88591' as an is_IS ISO-8859-1 locale ok 1 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Icelandic ok 2 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Runes ok 3 - gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / Icelandic ok 4 - gettext: impossible ISO-8859-1 output ok 5 - gettext: Fetching a UTF-8 msgid -> UTF-8 ok 6 - gettext: Fetching a UTF-8 msgid -> ISO-8859-1 ok 7 - gettext.c: git init UTF-8 -> UTF-8 ok 8 - gettext.c: git init UTF-8 -> ISO-8859-1 # passed all 8 test(s) And now all those tests are run, great! But look what happens without the language pack: # dpkg -P language-pack-is{,-base} [...] # grep is_IS /etc/locale.gen is_IS ISO-8859-1 is_IS.UTF-8 UTF-8 buzz ~# locale-gen Generating locales (this might take a while)... [...] is_IS.ISO-8859-1... done is_IS.UTF-8... done Generation complete. Still both Icelandic locales are generated. $ ./t0204-gettext-reencode-sanity.sh # lib-gettext: Found 'is_IS.utf8' as an is_IS UTF-8 locale # lib-gettext: Found 'is_IS.iso88591' as an is_IS ISO-8859-1 locale ok 1 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Icelandic ok 2 - gettext: Emitting UTF-8 from our UTF-8 *.mo files / Runes ok 3 - gettext: Emitting ISO-8859-1 from our UTF-8 *.mo files / Icelandic ok 4 - gettext: impossible ISO-8859-1 output ok 5 - gettext: Fetching a UTF-8 msgid -> UTF-8 ok 6 - gettext: Fetching a UTF-8 msgid -> ISO-8859-1 ok 7 - gettext.c: git init UTF-8 -> UTF-8 ok 8 - gettext.c: git init UTF-8 -> ISO-8859-1 # passed all 8 test(s) And still all those tests are run! I did run all test scripts sourcing 'lib-gettext.sh' with both locales generated and didn't see any errors, independently from whether the language pack was installed or not. I still have a few skipped tests (because of no GETTEXT_POISON and something PCRE-related), but those, too, are independent from the language pack. So it seems that we don't need 'language-pack-is' after all; what we do need are the ISO and UTF-8 Icelandic locales. Not sure we can modify /etc/locale.gen without sudo in the Travis CI build job, though, and I've run out of time to try. Gábor ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 7/8] travis-ci: save prove state for the 32 bit Linux build 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (4 preceding siblings ...) 2017-12-16 12:57 ` [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package SZEDER Gábor @ 2017-12-16 12:58 ` SZEDER Gábor 2017-12-16 12:58 ` [PATCH v2 8/8] travis-ci: only print test failures if there are test results available SZEDER Gábor ` (2 subsequent siblings) 8 siblings, 0 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor This change follows suit of 6272ed319 (travis-ci: run previously failed tests first, then slowest to fastest, 2016-01-26), which did this for the Linux and OSX build jobs. Travis CI build jobs run the tests parallel, which is sligtly faster when tests are run in slowest to fastest order, shortening the overall runtime of this build job by about a minute / 10%. Note, that the 32 bit Linux build job runs the tests suite in a Docker container and we have to share the Travis CI cache directory with the container as a second volume. Otherwise we couldn't use a symlink pointing to the prove state file in the cache directory, because that's outside of the directory hierarchy accessible from within the container. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/run-linux32-build.sh | 1 + ci/run-linux32-docker.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index a8518eddf..c19c50c1c 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -27,6 +27,7 @@ test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) && # Build and test linux32 --32bit i386 su -m -l $CI_USER -c ' cd /usr/src/git && + ln -s /tmp/travis-cache/.prove t/.prove && make --jobs=2 && make --quiet test ' diff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh index 0edf63acf..3a8b2ba42 100755 --- a/ci/run-linux32-docker.sh +++ b/ci/run-linux32-docker.sh @@ -19,5 +19,6 @@ docker run \ --env GIT_TEST_OPTS \ --env GIT_TEST_CLONE_2GB \ --volume "${PWD}:/usr/src/git" \ + --volume "${HOME}/travis-cache:/tmp/travis-cache" \ daald/ubuntu32:xenial \ /usr/src/git/ci/run-linux32-build.sh $(id -u $USER) -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v2 8/8] travis-ci: only print test failures if there are test results available 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (5 preceding siblings ...) 2017-12-16 12:58 ` [PATCH v2 7/8] travis-ci: save prove state for the 32 bit Linux build SZEDER Gábor @ 2017-12-16 12:58 ` SZEDER Gábor 2017-12-16 18:32 ` Eric Sunshine 2017-12-16 16:43 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing Johannes Schindelin 2017-12-18 21:53 ` Lars Schneider 8 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 12:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor When a build job running the test suite fails, our 'ci/print-test-failures.sh' script scans all 't/test-results/*.exit' files to find failed tests and prints their verbose output. However, if a build job were to fail before it ever gets to run the test suite, then there will be no files to match the above pattern and the shell will take the pattern literally, resulting in errors like this in the trace log: cat: t/test-results/*.exit: No such file or directory ------------------------------------------------------------------------ t/test-results/*.out... ------------------------------------------------------------------------ cat: t/test-results/*.out: No such file or directory Check upfront and proceed only if there are any such files present. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/print-test-failures.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh index f757e616c..1a3d74d47 100755 --- a/ci/print-test-failures.sh +++ b/ci/print-test-failures.sh @@ -8,6 +8,12 @@ # Tracing executed commands would produce too much noise in this script. set +x +if test t/test-results/*.exit = "t/test-results/*.exit" +then + echo "Build job failed before the tests could have been run" + exit +fi + for TEST_EXIT in t/test-results/*.exit do if [ "$(cat "$TEST_EXIT")" != "0" ] -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 8/8] travis-ci: only print test failures if there are test results available 2017-12-16 12:58 ` [PATCH v2 8/8] travis-ci: only print test failures if there are test results available SZEDER Gábor @ 2017-12-16 18:32 ` Eric Sunshine 2017-12-16 22:48 ` [PATCH v2 8/8] travis-ci: only print test failures if there are SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: Eric Sunshine @ 2017-12-16 18:32 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Lars Schneider, Git List On Sat, Dec 16, 2017 at 7:58 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote: > When a build job running the test suite fails, our > 'ci/print-test-failures.sh' script scans all 't/test-results/*.exit' > files to find failed tests and prints their verbose output. However, > if a build job were to fail before it ever gets to run the test suite, > then there will be no files to match the above pattern and the shell > will take the pattern literally, resulting in errors like this in the > trace log: > > cat: t/test-results/*.exit: No such file or directory > ------------------------------------------------------------------------ > t/test-results/*.out... > ------------------------------------------------------------------------ > cat: t/test-results/*.out: No such file or directory > > Check upfront and proceed only if there are any such files present. > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh > @@ -8,6 +8,12 @@ > +if test t/test-results/*.exit = "t/test-results/*.exit" Isn't the above going to cause 'test' to error out? $ mkdir -p t/test-results $ >t/test-results/a.exit $ >t/test-results/b.exit $ test t/test-results/*.exit = 't/test-results/*.exit' -bash: test: too many arguments I'd think you'd want to capture the result of the expansion of t/test-result/*.exit as a string and compare that instead. > +then > + echo "Build job failed before the tests could have been run" > + exit > +fi ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 8/8] travis-ci: only print test failures if there are 2017-12-16 18:32 ` Eric Sunshine @ 2017-12-16 22:48 ` SZEDER Gábor 2017-12-17 0:02 ` Eric Sunshine 0 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-16 22:48 UTC (permalink / raw) To: Eric Sunshine, Junio C Hamano; +Cc: SZEDER Gábor, Lars Schneider, Git List > On Sat, Dec 16, 2017 at 7:58 AM, SZEDER G=C3=A1bor <szeder.dev@gmail.com> w= > rote: > > +if test t/test-results/*.exit =3D "t/test-results/*.exit" > > Isn't the above going to cause 'test' to error out? > > $ mkdir -p t/test-results > $ >t/test-results/a.exit > $ >t/test-results/b.exit > $ test t/test-results/*.exit =3D 't/test-results/*.exit' > -bash: test: too many arguments Indeed it does, though Travis CI's /bin/sh gives a different error message. Thanks for pointin it out, I didn't notice the error msg, because the script as a whole still did the right thing and then I didn't look close enough. > I'd think you'd want to capture the result of the expansion of > t/test-result/*.exit as a string and compare that instead. I'm not sure how the result of the expansion could be captured in the shell, because the shell performs the expansion only just before it executes a command. And if we do have to execute a command anyway, then we can simply rely on the command exiting with an error code upon not finding any files, and there's no need to capture the expansion or for the comparison for that matter. So I propose this updated patch, using 'ls' instead of 'test': -- >8 -- Subject: [PATCH v2.1 8/8] travis-ci: only print test failures if there are test results available When a build job running the test suite fails, our 'ci/print-test-failures.sh' script scans all 't/test-results/*.exit' files to find failed tests and prints their verbose output. However, if a build job were to fail before it ever gets to run the test suite, then there will be no files to match the above pattern and the shell will take the pattern literally, resulting in errors like this in the trace log: cat: t/test-results/*.exit: No such file or directory ------------------------------------------------------------------------ t/test-results/*.out... ------------------------------------------------------------------------ cat: t/test-results/*.out: No such file or directory Check upfront and proceed only if there are any such files present. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/print-test-failures.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh index f757e616c..b4a62ef98 100755 --- a/ci/print-test-failures.sh +++ b/ci/print-test-failures.sh @@ -8,6 +8,12 @@ # Tracing executed commands would produce too much noise in this script. set +x +if ! ls t/test-results/*.exit >/dev/null 2>/dev/null +then + echo "Build job failed before the tests could have been run" + exit +fi + for TEST_EXIT in t/test-results/*.exit do if [ "$(cat "$TEST_EXIT")" != "0" ] -- 2.15.1.429.ga000dd9c7 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 8/8] travis-ci: only print test failures if there are 2017-12-16 22:48 ` [PATCH v2 8/8] travis-ci: only print test failures if there are SZEDER Gábor @ 2017-12-17 0:02 ` Eric Sunshine 0 siblings, 0 replies; 44+ messages in thread From: Eric Sunshine @ 2017-12-17 0:02 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Lars Schneider, Git List On Sat, Dec 16, 2017 at 5:48 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote: > On Sat, Dec 16, 2017, Eric Sunshine <sunshine@sunshineco.com> wrote: >> I'd think you'd want to capture the result of the expansion of >> t/test-result/*.exit as a string and compare that instead. > > I'm not sure how the result of the expansion could be captured in the > shell, because the shell performs the expansion only just before it > executes a command. And if we do have to execute a command anyway, > then we can simply rely on the command exiting with an error code upon > not finding any files, and there's no need to capture the expansion or > for the comparison for that matter. > So I propose this updated patch, using 'ls' instead of 'test': To capture the expansion as a string, I was thinking along the lines of: x=$(echo t/test-results/*.exit) but that's a minor point. Checking the explicit exit code of a command, as you suggest, is probably cleaner. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (6 preceding siblings ...) 2017-12-16 12:58 ` [PATCH v2 8/8] travis-ci: only print test failures if there are test results available SZEDER Gábor @ 2017-12-16 16:43 ` Johannes Schindelin 2017-12-18 21:53 ` Lars Schneider 8 siblings, 0 replies; 44+ messages in thread From: Johannes Schindelin @ 2017-12-16 16:43 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Lars Schneider, git [-- Attachment #1: Type: text/plain, Size: 1517 bytes --] Hi Gábor, On Sat, 16 Dec 2017, SZEDER Gábor wrote: > While the build logic was embedded in our '.travis.yml', Travis CI > used to produce a nice trace log including all commands executed in > those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI > code into dedicated scripts, 2017-09-10), however, we only see the > name of the dedicated scripts, but not what those scripts are actually > doing, resulting in a less useful trace log about e.g. installing > dependencies. A patch later in this series will move setting > environment variables from '.travis.yml' to 'ci/lib-travisci.sh', so > not even those will be included in the trace log. Unrelated to > 657343a60, 'ci/test-documentation.sh' runs a bunch of 'test -s <file>' > checks which would fail quietly if something were wrong, leaving no > clue about which one of those checks triggered the failure. > > Use 'set -x' in 'ci/lib-travisci.sh' to get more detailed trace log > about the commands executed in the 'ci/*' scripts. Use it in > 'ci/run-linux32-build.sh' as well, which is run in a Docker container > and therefore doesn't source 'ci/lib-travisci.sh'. The secret token > used for the Windows builds is specified as an encrypted environment > variable in git/git repository settings on Travis CI and it's redacted > in the trace logs even with 'set -x'. However, disable this tracing > in 'ci/print-test-failures.sh', as it produces far too much noise in > the output of that script. ACK, Dscho ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor ` (7 preceding siblings ...) 2017-12-16 16:43 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing Johannes Schindelin @ 2017-12-18 21:53 ` Lars Schneider 8 siblings, 0 replies; 44+ messages in thread From: Lars Schneider @ 2017-12-18 21:53 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Johannes Schindelin, git > On 16 Dec 2017, at 13:54, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > While the build logic was embedded in our '.travis.yml', Travis CI > used to produce a nice trace log including all commands executed in > those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI > code into dedicated scripts, 2017-09-10), however, we only see the > name of the dedicated scripts, but not what those scripts are actually > doing, resulting in a less useful trace log about e.g. installing > dependencies. A patch later in this series will move setting > environment variables from '.travis.yml' to 'ci/lib-travisci.sh', so > not even those will be included in the trace log. Unrelated to > 657343a60, 'ci/test-documentation.sh' runs a bunch of 'test -s <file>' > checks which would fail quietly if something were wrong, leaving no > clue about which one of those checks triggered the failure. > > Use 'set -x' in 'ci/lib-travisci.sh' to get more detailed trace log > about the commands executed in the 'ci/*' scripts. Use it in > 'ci/run-linux32-build.sh' as well, which is run in a Docker container > and therefore doesn't source 'ci/lib-travisci.sh'. The secret token > used for the Windows builds is specified as an encrypted environment > variable in git/git repository settings on Travis CI and it's redacted > in the trace logs even with 'set -x'. However, disable this tracing > in 'ci/print-test-failures.sh', as it produces far too much noise in > the output of that script. I ACK too soon in my other email ;-) Can you disable the trace log for the loop in "run-windows-build.sh": https://github.com/git/git/blob/52015aaf9d19c97b52c47c7046058e6d029ff856/ci/run-windows-build.sh#L75-L88 Otherwise the Travis output look like this: https://travis-ci.org/git/git/jobs/316791071 - Lars > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > ci/lib-travisci.sh | 6 ++++-- > ci/print-test-failures.sh | 3 +++ > ci/run-linux32-build.sh | 2 ++ > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh > index ac05f1f46..2d0d1d613 100755 > --- a/ci/lib-travisci.sh > +++ b/ci/lib-travisci.sh > @@ -22,8 +22,10 @@ skip_branch_tip_with_tag () { > } > > # Set 'exit on error' for all CI scripts to let the caller know that > -# something went wrong > -set -e > +# something went wrong. > +# Set tracing executed commands, primarily setting environment variables > +# and installing dependencies. > +set -ex > > skip_branch_tip_with_tag > > diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh > index 8c8973cbf..f757e616c 100755 > --- a/ci/print-test-failures.sh > +++ b/ci/print-test-failures.sh > @@ -5,6 +5,9 @@ > > . ${0%/*}/lib-travisci.sh > > +# Tracing executed commands would produce too much noise in this script. > +set +x > + > for TEST_EXIT in t/test-results/*.exit > do > if [ "$(cat "$TEST_EXIT")" != "0" ] > diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh > index e30fb2cdd..a8518eddf 100755 > --- a/ci/run-linux32-build.sh > +++ b/ci/run-linux32-build.sh > @@ -6,6 +6,8 @@ > # run-linux32-build.sh [host-user-id] > # > > +set -x > + > # Update packages to the latest available versions > linux32 --32bit i386 sh -c ' > apt update >/dev/null && > -- > 2.15.1.429.ga000dd9c7 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 0/8] Travis CI cleanups 2017-12-16 12:54 ` [PATCH v2 0/8] Travis CI cleanups SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor @ 2017-12-18 21:46 ` Lars Schneider 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor 2 siblings, 0 replies; 44+ messages in thread From: Lars Schneider @ 2017-12-18 21:46 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Johannes Schindelin, git > On 16 Dec 2017, at 13:54, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > This is a reroll of 'sg/travis-fixes'. > > Changes since the previous round: > > - Patch 1 got updated following the discussion: > > - I went with enabling tracing executed commands everywhere, > including the Windows build job, except where we know it causes way > too much clutter, which is currently only > 'ci/print-test-failures.sh'. > > - Also enable this tracing in 'ci/run-linux32-build.sh', which > doesn't source 'ci/lib-travisci.sh' as it's run inside a Docker > container. > > - The commit message got updated accordingly, including a note about > the Windows build job's secret token. > I would like to get an Acked-by: from Dscho on this patch before it > gets merged. > > - Patches 5-8 are new. They are various fixes/cleanups unrelated to > the Travis CI scriptification, but I don't think it's worth to have > them in separate patch series. This cleanup series looks very good. ACK. Thank you, Gabor! The is the only potential nit I see: https://public-inbox.org/git/8F53EF33-6FDA-484C-91A4-49CF24C0B417@gmail.com/ - Lars > > SZEDER Gábor (8): > travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing > travis-ci: introduce a $jobname variable for 'ci/*' scripts > travis-ci: move setting environment variables to 'ci/lib-travisci.sh' > travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' > travis-ci: don't install default addon packages for the 32 bit Linux > build > travis-ci: don't install 'language-pack-is' package > travis-ci: save prove state for the 32 bit Linux build > travis-ci: only print test failures if there are test results > available > > .travis.yml | 28 ++++++---------------------- > ci/install-dependencies.sh | 8 +++----- > ci/lib-travisci.sh | 38 ++++++++++++++++++++++++++++++++++---- > ci/print-test-failures.sh | 9 +++++++++ > ci/run-linux32-build.sh | 3 +++ > ci/run-linux32-docker.sh | 1 + > 6 files changed, 56 insertions(+), 31 deletions(-) > > -- > 2.15.1.429.ga000dd9c7 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 0/4] Rest of the Travis CI fixes 2017-12-16 12:54 ` [PATCH v2 0/8] Travis CI cleanups SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor 2017-12-18 21:46 ` [PATCH v2 0/8] Travis CI cleanups Lars Schneider @ 2017-12-27 16:35 ` SZEDER Gábor 2017-12-27 16:36 ` [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts SZEDER Gábor ` (3 more replies) 2 siblings, 4 replies; 44+ messages in thread From: SZEDER Gábor @ 2017-12-27 16:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor These are the rest of the fixes that were in the second half of the v2 patch series but not already in 'sg/travis-fixes', which was already merged to next, perhaps a bit prematurely. Patch 1/4 fixes some rough edges related to tracing executed commands according to the discussion. Patches 2/4, 3/4 are the same as patches 5/8 and 7/8 in v2, while patch 4/4 includes the small update I sent out as v2.1 of patch 8/8. I dropped v2's patch 6/8 (travis-ci: don't install 'language-pack-is' package) for now: these locale-related things require more investigation. v2: https://public-inbox.org/git/20171216125418.10743-1-szeder.dev@gmail.com/ v1: https://public-inbox.org/git/20171211233446.10596-1-szeder.dev@gmail.com/ SZEDER Gábor (4): travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts travis-ci: don't install default addon packages for the 32 bit Linux build travis-ci: save prove state for the 32 bit Linux build travis-ci: only print test failures if there are test results available .travis.yml | 1 + ci/lib-travisci.sh | 4 +++- ci/print-test-failures.sh | 9 +++++++++ ci/run-linux32-build.sh | 3 +++ ci/run-linux32-docker.sh | 1 + ci/run-windows-build.sh | 5 +++++ 6 files changed, 22 insertions(+), 1 deletion(-) -- 2.15.1.500.g54ea76cc4 ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor @ 2017-12-27 16:36 ` SZEDER Gábor 2017-12-27 18:35 ` Lars Schneider 2017-12-27 16:36 ` [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor ` (2 subsequent siblings) 3 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-27 16:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor The change in commit 4f2636667 (travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output, 2017-12-12) left a couple of rough edges: - 'ci/run-linux32-build.sh' is executed in a Docker container and therefore doesn't source 'ci/lib-travisci.sh', which would enable tracing executed commands. Enable 'set -x' in this script, too. - 'ci/print-test-failures.sh' iterates over all the files containing the exit codes of all the execued test scripts. Since there are over 800 such files, the loop produces way too much noise with tracing executed commands enabled, so disable 'set -x' for this script. - 'ci/run-windows-build.sh' busily waits in a loop for the result of the Windows build, producing too much noise with tracing executed commands enabled as well. Disable 'set -x' for the duration of that loop. igned-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/lib-travisci.sh | 4 +++- ci/print-test-failures.sh | 3 +++ ci/run-linux32-build.sh | 2 ++ ci/run-windows-build.sh | 5 +++++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index 331d3eb3a..348fe3c3c 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -22,7 +22,9 @@ skip_branch_tip_with_tag () { } # Set 'exit on error' for all CI scripts to let the caller know that -# something went wrong +# something went wrong. +# Set tracing executed commands, primarily setting environment variables +# and installing dependencies. set -ex skip_branch_tip_with_tag diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh index 8c8973cbf..97cc05901 100755 --- a/ci/print-test-failures.sh +++ b/ci/print-test-failures.sh @@ -5,6 +5,9 @@ . ${0%/*}/lib-travisci.sh +# Tracing executed commands would produce too much noise in the loop below. +set +x + for TEST_EXIT in t/test-results/*.exit do if [ "$(cat "$TEST_EXIT")" != "0" ] diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index e30fb2cdd..a8518eddf 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -6,6 +6,8 @@ # run-linux32-build.sh [host-user-id] # +set -x + # Update packages to the latest available versions linux32 --32bit i386 sh -c ' apt update >/dev/null && diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh index 8757b3a97..86999268a 100755 --- a/ci/run-windows-build.sh +++ b/ci/run-windows-build.sh @@ -69,6 +69,10 @@ esac echo "Visual Studio Team Services Build #${BUILD_ID}" +# Tracing execued commands would produce too much noise in the waiting +# loop below. +set +x + # Wait until build job finished STATUS= RESULT= @@ -90,6 +94,7 @@ done # Print log echo "" echo "" +set -x gfwci "action=log&buildId=$BUILD_ID" | cut -c 30- # Set exit code for TravisCI -- 2.15.1.500.g54ea76cc4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts 2017-12-27 16:36 ` [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts SZEDER Gábor @ 2017-12-27 18:35 ` Lars Schneider 0 siblings, 0 replies; 44+ messages in thread From: Lars Schneider @ 2017-12-27 18:35 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, git > On 27 Dec 2017, at 17:36, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > The change in commit 4f2636667 (travis-ci: use 'set -x' in 'ci/*' > scripts for extra tracing output, 2017-12-12) left a couple of rough > edges: > > - 'ci/run-linux32-build.sh' is executed in a Docker container and > therefore doesn't source 'ci/lib-travisci.sh', which would enable > tracing executed commands. Enable 'set -x' in this script, too. > > - 'ci/print-test-failures.sh' iterates over all the files containing > the exit codes of all the execued test scripts. Since there are s/execued/executed/ > over 800 such files, the loop produces way too much noise with > tracing executed commands enabled, so disable 'set -x' for this > script. > > - 'ci/run-windows-build.sh' busily waits in a loop for the result of > the Windows build, producing too much noise with tracing executed > commands enabled as well. Disable 'set -x' for the duration of > that loop. > > igned-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > ci/lib-travisci.sh | 4 +++- > ci/print-test-failures.sh | 3 +++ > ci/run-linux32-build.sh | 2 ++ > ci/run-windows-build.sh | 5 +++++ > 4 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh > index 331d3eb3a..348fe3c3c 100755 > --- a/ci/lib-travisci.sh > +++ b/ci/lib-travisci.sh > @@ -22,7 +22,9 @@ skip_branch_tip_with_tag () { > } > > # Set 'exit on error' for all CI scripts to let the caller know that > -# something went wrong > +# something went wrong. > +# Set tracing executed commands, primarily setting environment variables > +# and installing dependencies. Maybe: # something went wrong. Enable tracing to ease debugging on TravisCI. I would move the "primarily setting environment ..." either to the top of the file or to the respective section below. But this is a minor nit. The rest of the patch looks very good. Thanks, Lars > set -ex > > skip_branch_tip_with_tag > diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh > index 8c8973cbf..97cc05901 100755 > --- a/ci/print-test-failures.sh > +++ b/ci/print-test-failures.sh > @@ -5,6 +5,9 @@ > > . ${0%/*}/lib-travisci.sh > > +# Tracing executed commands would produce too much noise in the loop below. > +set +x > + > for TEST_EXIT in t/test-results/*.exit > do > if [ "$(cat "$TEST_EXIT")" != "0" ] > diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh > index e30fb2cdd..a8518eddf 100755 > --- a/ci/run-linux32-build.sh > +++ b/ci/run-linux32-build.sh > @@ -6,6 +6,8 @@ > # run-linux32-build.sh [host-user-id] > # > > +set -x > + > # Update packages to the latest available versions > linux32 --32bit i386 sh -c ' > apt update >/dev/null && > diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh > index 8757b3a97..86999268a 100755 > --- a/ci/run-windows-build.sh > +++ b/ci/run-windows-build.sh > @@ -69,6 +69,10 @@ esac > > echo "Visual Studio Team Services Build #${BUILD_ID}" > > +# Tracing execued commands would produce too much noise in the waiting > +# loop below. > +set +x > + > # Wait until build job finished > STATUS= > RESULT= > @@ -90,6 +94,7 @@ done > # Print log > echo "" > echo "" > +set -x > gfwci "action=log&buildId=$BUILD_ID" | cut -c 30- > > # Set exit code for TravisCI > -- > 2.15.1.500.g54ea76cc4 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor 2017-12-27 16:36 ` [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts SZEDER Gábor @ 2017-12-27 16:36 ` SZEDER Gábor 2017-12-27 18:41 ` Lars Schneider 2017-12-27 16:36 ` [PATCH v3 3/4] travis-ci: save prove state " SZEDER Gábor 2017-12-27 16:36 ` [PATCH v3 4/4] travis-ci: only print test failures if there are test results available SZEDER Gábor 3 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-27 16:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor The 32 bit Linux build job compiles Git and runs the test suite in a Docker container, while the additional packages (apache2, git-svn, language-pack-is) are installed on the host, therefore don't have any effect and are unnecessary. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 7c9aa0557..4684b3f4f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,6 +42,7 @@ matrix: - env: jobname=Linux32 os: linux compiler: + addons: services: - docker before_install: -- 2.15.1.500.g54ea76cc4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build 2017-12-27 16:36 ` [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor @ 2017-12-27 18:41 ` Lars Schneider 0 siblings, 0 replies; 44+ messages in thread From: Lars Schneider @ 2017-12-27 18:41 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, git > On 27 Dec 2017, at 17:36, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > The 32 bit Linux build job compiles Git and runs the test suite in a > Docker container, while the additional packages (apache2, git-svn, > language-pack-is) are installed on the host, therefore don't have > any effect and are unnecessary. Nice optimization - ACK! Thanks, Lars > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > .travis.yml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.travis.yml b/.travis.yml > index 7c9aa0557..4684b3f4f 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -42,6 +42,7 @@ matrix: > - env: jobname=Linux32 > os: linux > compiler: > + addons: > services: > - docker > before_install: > -- > 2.15.1.500.g54ea76cc4 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor 2017-12-27 16:36 ` [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts SZEDER Gábor 2017-12-27 16:36 ` [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor @ 2017-12-27 16:36 ` SZEDER Gábor 2017-12-27 18:46 ` Lars Schneider 2017-12-27 16:36 ` [PATCH v3 4/4] travis-ci: only print test failures if there are test results available SZEDER Gábor 3 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-27 16:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor This change follows suit of 6272ed319 (travis-ci: run previously failed tests first, then slowest to fastest, 2016-01-26), which did this for the Linux and OSX build jobs. Travis CI build jobs run the tests parallel, which is sligtly faster when tests are run in slowest to fastest order, shortening the overall runtime of this build job by about a minute / 10%. Note, that the 32 bit Linux build job runs the tests suite in a Docker container and we have to share the Travis CI cache directory with the container as a second volume. Otherwise we couldn't use a symlink pointing to the prove state file in the cache directory, because that's outside of the directory hierarchy accessible from within the container. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/run-linux32-build.sh | 1 + ci/run-linux32-docker.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index a8518eddf..c19c50c1c 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -27,6 +27,7 @@ test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) && # Build and test linux32 --32bit i386 su -m -l $CI_USER -c ' cd /usr/src/git && + ln -s /tmp/travis-cache/.prove t/.prove && make --jobs=2 && make --quiet test ' diff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh index 0edf63acf..3a8b2ba42 100755 --- a/ci/run-linux32-docker.sh +++ b/ci/run-linux32-docker.sh @@ -19,5 +19,6 @@ docker run \ --env GIT_TEST_OPTS \ --env GIT_TEST_CLONE_2GB \ --volume "${PWD}:/usr/src/git" \ + --volume "${HOME}/travis-cache:/tmp/travis-cache" \ daald/ubuntu32:xenial \ /usr/src/git/ci/run-linux32-build.sh $(id -u $USER) -- 2.15.1.500.g54ea76cc4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build 2017-12-27 16:36 ` [PATCH v3 3/4] travis-ci: save prove state " SZEDER Gábor @ 2017-12-27 18:46 ` Lars Schneider 2017-12-27 21:42 ` SZEDER Gábor 0 siblings, 1 reply; 44+ messages in thread From: Lars Schneider @ 2017-12-27 18:46 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, git > On 27 Dec 2017, at 17:36, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > This change follows suit of 6272ed319 (travis-ci: run previously > failed tests first, then slowest to fastest, 2016-01-26), which did > this for the Linux and OSX build jobs. Travis CI build jobs run the > tests parallel, which is sligtly faster when tests are run in slowest > to fastest order, shortening the overall runtime of this build job by > about a minute / 10%. > > Note, that the 32 bit Linux build job runs the tests suite in a Docker > container and we have to share the Travis CI cache directory with the > container as a second volume. Otherwise we couldn't use a symlink > pointing to the prove state file in the cache directory, because > that's outside of the directory hierarchy accessible from within the > container. > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > ci/run-linux32-build.sh | 1 + > ci/run-linux32-docker.sh | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh > index a8518eddf..c19c50c1c 100755 > --- a/ci/run-linux32-build.sh > +++ b/ci/run-linux32-build.sh > @@ -27,6 +27,7 @@ test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) && > # Build and test > linux32 --32bit i386 su -m -l $CI_USER -c ' > cd /usr/src/git && > + ln -s /tmp/travis-cache/.prove t/.prove && > make --jobs=2 && > make --quiet test > ' > diff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh > index 0edf63acf..3a8b2ba42 100755 > --- a/ci/run-linux32-docker.sh > +++ b/ci/run-linux32-docker.sh > @@ -19,5 +19,6 @@ docker run \ > --env GIT_TEST_OPTS \ > --env GIT_TEST_CLONE_2GB \ > --volume "${PWD}:/usr/src/git" \ > + --volume "${HOME}/travis-cache:/tmp/travis-cache" \ I assume "${HOME}/travis-cache:/usr/src/git/t/.prove" would not work because that would be a mapping in another mapping? Thanks, Lars > daald/ubuntu32:xenial \ > /usr/src/git/ci/run-linux32-build.sh $(id -u $USER) > -- > 2.15.1.500.g54ea76cc4 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build 2017-12-27 18:46 ` Lars Schneider @ 2017-12-27 21:42 ` SZEDER Gábor 2017-12-28 11:17 ` Lars Schneider 0 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-27 21:42 UTC (permalink / raw) To: Lars Schneider; +Cc: Junio C Hamano, Git mailing list On Wed, Dec 27, 2017 at 7:46 PM, Lars Schneider <larsxschneider@gmail.com> wrote: >> + --volume "${HOME}/travis-cache:/tmp/travis-cache" \ > > I assume "${HOME}/travis-cache:/usr/src/git/t/.prove" would not > work because that would be a mapping in another mapping? 't/.prove' is a file, but '.../travis-cache' is a directory. It must be, because Travis CI caches whole directories. Gábor ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build 2017-12-27 21:42 ` SZEDER Gábor @ 2017-12-28 11:17 ` Lars Schneider 0 siblings, 0 replies; 44+ messages in thread From: Lars Schneider @ 2017-12-28 11:17 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Git mailing list > On 27 Dec 2017, at 22:42, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > On Wed, Dec 27, 2017 at 7:46 PM, Lars Schneider > <larsxschneider@gmail.com> wrote: >>> + --volume "${HOME}/travis-cache:/tmp/travis-cache" \ >> >> I assume "${HOME}/travis-cache:/usr/src/git/t/.prove" would not >> work because that would be a mapping in another mapping? > > 't/.prove' is a file, but '.../travis-cache' is a directory. It must > be, because Travis CI caches whole directories. Of course. Your solution is the right one. Thanks, Lars ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 4/4] travis-ci: only print test failures if there are test results available 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor ` (2 preceding siblings ...) 2017-12-27 16:36 ` [PATCH v3 3/4] travis-ci: save prove state " SZEDER Gábor @ 2017-12-27 16:36 ` SZEDER Gábor 2017-12-27 18:52 ` Lars Schneider 3 siblings, 1 reply; 44+ messages in thread From: SZEDER Gábor @ 2017-12-27 16:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, SZEDER Gábor When a build job running the test suite fails, our 'ci/print-test-failures.sh' script scans all 't/test-results/*.exit' files to find failed tests and prints their verbose output. However, if a build job were to fail before it ever gets to run the test suite, then there will be no files to match the above pattern and the shell will take the pattern literally, resulting in errors like this in the trace log: cat: t/test-results/*.exit: No such file or directory ------------------------------------------------------------------------ t/test-results/*.out... ------------------------------------------------------------------------ cat: t/test-results/*.out: No such file or directory Check upfront and proceed only if there are any such files present. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- ci/print-test-failures.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh index 97cc05901..4f261ddc0 100755 --- a/ci/print-test-failures.sh +++ b/ci/print-test-failures.sh @@ -8,6 +8,12 @@ # Tracing executed commands would produce too much noise in the loop below. set +x +if ! ls t/test-results/*.exit >/dev/null 2>/dev/null +then + echo "Build job failed before the tests could have been run" + exit +fi + for TEST_EXIT in t/test-results/*.exit do if [ "$(cat "$TEST_EXIT")" != "0" ] -- 2.15.1.500.g54ea76cc4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v3 4/4] travis-ci: only print test failures if there are test results available 2017-12-27 16:36 ` [PATCH v3 4/4] travis-ci: only print test failures if there are test results available SZEDER Gábor @ 2017-12-27 18:52 ` Lars Schneider 0 siblings, 0 replies; 44+ messages in thread From: Lars Schneider @ 2017-12-27 18:52 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, git > On 27 Dec 2017, at 17:36, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > When a build job running the test suite fails, our > 'ci/print-test-failures.sh' script scans all 't/test-results/*.exit' > files to find failed tests and prints their verbose output. However, > if a build job were to fail before it ever gets to run the test suite, > then there will be no files to match the above pattern and the shell > will take the pattern literally, resulting in errors like this in the > trace log: > > cat: t/test-results/*.exit: No such file or directory > ------------------------------------------------------------------------ > t/test-results/*.out... > ------------------------------------------------------------------------ > cat: t/test-results/*.out: No such file or directory > > Check upfront and proceed only if there are any such files present. > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > ci/print-test-failures.sh | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh > index 97cc05901..4f261ddc0 100755 > --- a/ci/print-test-failures.sh > +++ b/ci/print-test-failures.sh > @@ -8,6 +8,12 @@ > # Tracing executed commands would produce too much noise in the loop below. > set +x > > +if ! ls t/test-results/*.exit >/dev/null 2>/dev/null > +then > + echo "Build job failed before the tests could have been run" > + exit > +fi > + Look good to me! Minor nit: I am not 100% sure about the grammar but I am no native speaker and can't really tell. Thanks, Lars > for TEST_EXIT in t/test-results/*.exit > do > if [ "$(cat "$TEST_EXIT")" != "0" ] > -- > 2.15.1.500.g54ea76cc4 > ^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2017-12-28 11:17 UTC | newest] Thread overview: 44+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-01 11:55 [PATCH] travis-ci: fix running P4 and Git LFS tests in Linux build jobs SZEDER Gábor 2017-12-11 23:34 ` [PATCH 0/4] travis-ci: clean up setting environment variables SZEDER Gábor 2017-12-11 23:34 ` [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output SZEDER Gábor 2017-12-12 18:00 ` Lars Schneider 2017-12-12 18:43 ` SZEDER Gábor 2017-12-13 23:10 ` Lars Schneider 2017-12-14 23:51 ` SZEDER Gábor 2017-12-15 12:10 ` Johannes Schindelin 2017-12-15 13:06 ` SZEDER Gábor 2017-12-15 15:32 ` Johannes Schindelin 2017-12-11 23:34 ` [PATCH 2/4] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor 2017-12-11 23:34 ` [PATCH 3/4] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' SZEDER Gábor 2017-12-11 23:34 ` [PATCH 4/4] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 0/8] Travis CI cleanups SZEDER Gábor 2017-12-16 12:54 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing SZEDER Gábor 2017-12-16 12:55 ` [PATCH v2 2/8] travis-ci: introduce a $jobname variable for 'ci/*' scripts SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 3/8] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 4/8] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 5/8] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor 2017-12-16 12:57 ` [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package SZEDER Gábor 2017-12-18 21:33 ` Lars Schneider 2017-12-18 22:04 ` SZEDER Gábor 2017-12-18 22:17 ` Lars Schneider 2017-12-18 22:34 ` Junio C Hamano 2017-12-19 12:22 ` SZEDER Gábor 2017-12-16 12:58 ` [PATCH v2 7/8] travis-ci: save prove state for the 32 bit Linux build SZEDER Gábor 2017-12-16 12:58 ` [PATCH v2 8/8] travis-ci: only print test failures if there are test results available SZEDER Gábor 2017-12-16 18:32 ` Eric Sunshine 2017-12-16 22:48 ` [PATCH v2 8/8] travis-ci: only print test failures if there are SZEDER Gábor 2017-12-17 0:02 ` Eric Sunshine 2017-12-16 16:43 ` [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing Johannes Schindelin 2017-12-18 21:53 ` Lars Schneider 2017-12-18 21:46 ` [PATCH v2 0/8] Travis CI cleanups Lars Schneider 2017-12-27 16:35 ` [PATCH v3 0/4] Rest of the Travis CI fixes SZEDER Gábor 2017-12-27 16:36 ` [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts SZEDER Gábor 2017-12-27 18:35 ` Lars Schneider 2017-12-27 16:36 ` [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build SZEDER Gábor 2017-12-27 18:41 ` Lars Schneider 2017-12-27 16:36 ` [PATCH v3 3/4] travis-ci: save prove state " SZEDER Gábor 2017-12-27 18:46 ` Lars Schneider 2017-12-27 21:42 ` SZEDER Gábor 2017-12-28 11:17 ` Lars Schneider 2017-12-27 16:36 ` [PATCH v3 4/4] travis-ci: only print test failures if there are test results available SZEDER Gábor 2017-12-27 18:52 ` Lars Schneider
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).