travis-ci: fix running P4 and Git LFS tests in Linux build jobs

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.

---
 ci/install-dependencies.sh | 10 ++++------
 ci/lib-travisci.sh         |  8 ++++++++
 2 files changed, 12 insertions(+), 6 deletions(-)

index a29246af3..5bd06fe90 100755
@@ -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)

index b3ed0a0dd..ac05f1f46 100755
@@ -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

(Was: [PATCH] travis-ci: fix running P4 and Git LFS tests in Linux build jobs)

 .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

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.

---
 ci/lib-travisci.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index ac05f1f46..a0c8ae03f 100755
@@ -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

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

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

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

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

Hi,

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

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

Hi Gábor,

That's good enough.

Thanks for making sure,
Dscho

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.

---
 .travis.yml                | 10 +++++-----
 ci/install-dependencies.sh |  6 +++---
 ci/lib-travisci.sh         |  9 +++++++--
 3 files changed, 15 insertions(+), 10 deletions(-)

index 281f101f3..88435e11c 100644
@@ -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:

index 5bd06fe90..468788566 100755
@@ -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

index a0c8ae03f..b60e93797 100755
@@ -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

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.

---
 .travis.yml        | 18 +-----------------
 ci/lib-travisci.sh | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 17 deletions(-)

index 88435e11c..7c9aa0557 100644
@@ -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:

index b60e93797..e85571298 100755
@@ -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

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'.)

---
 ci/install-dependencies.sh | 2 --
 ci/lib-travisci.sh         | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

index 468788566..75a9fd247 100755
@@ -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"

index e85571298..331d3eb3a 100755
@@ -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

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

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.

---
 ci/lib-travisci.sh        | 6 ++++--
 ci/print-test-failures.sh | 3 +++
 ci/run-linux32-build.sh   | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

index ac05f1f46..2d0d1d613 100755
@@ -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
 

index 8c8973cbf..f757e616c 100755
@@ -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" ]

index e30fb2cdd..a8518eddf 100755
@@ -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

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.

---
 .travis.yml                | 10 +++++-----
 ci/install-dependencies.sh |  6 +++---
 ci/lib-travisci.sh         |  9 +++++++--
 3 files changed, 15 insertions(+), 10 deletions(-)

index 281f101f3..88435e11c 100644
@@ -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:

index 5bd06fe90..468788566 100755
@@ -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

index 2d0d1d613..4b3c5fdd0 100755
@@ -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

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.

---
 .travis.yml        | 18 +-----------------
 ci/lib-travisci.sh | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 17 deletions(-)

index 88435e11c..7c9aa0557 100644
@@ -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:

index 4b3c5fdd0..c5c5cb1bf 100755
@@ -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

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'.)

---
 ci/install-dependencies.sh | 2 --
 ci/lib-travisci.sh         | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

index 468788566..75a9fd247 100755
@@ -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"

index c5c5cb1bf..348fe3c3c 100755
@@ -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

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.

---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

index 7c9aa0557..4684b3f4f 100644
@@ -42,6 +42,7 @@ matrix:
     - env: jobname=Linux32
       os: linux
       compiler:
+      addons:
       services:
         - docker
       before_install:

2.15.1.429.ga000dd9c7

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.

---
 .travis.yml | 1 -
 1 file changed, 1 deletion(-)

index 4684b3f4f..ea11b5af6 100644
@@ -17,7 +17,6 @@ compiler:
 addons:
   apt:
     packages:
-    - language-pack-is
     - git-svn
     - apache2
 

2.15.1.429.ga000dd9c7

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

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.

Ahh. That explains it!

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

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.

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

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.

---
 ci/run-linux32-build.sh  | 1 +
 ci/run-linux32-docker.sh | 1 +
 2 files changed, 2 insertions(+)

index a8518eddf..c19c50c1c 100755
@@ -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
 '

index 0edf63acf..3a8b2ba42 100755
@@ -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

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.

---
 ci/print-test-failures.sh | 6 ++++++
 1 file changed, 6 insertions(+)

index f757e616c..1a3d74d47 100755
@@ -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

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'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.

---
 ci/print-test-failures.sh | 6 ++++++
 1 file changed, 6 insertions(+)

index f757e616c..b4a62ef98 100755
@@ -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

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.

Hi Gábor,

ACK,
Dscho

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

This cleanup series looks very good. ACK. Thank you, Gabor!

The is the only potential nit I see:
https://public-inbox.org/git/[email protected]/

- Lars

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/[email protected]/
v1: https://public-inbox.org/git/[email protected]/

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

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.

---
 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(-)

index 331d3eb3a..348fe3c3c 100755
@@ -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

index 8c8973cbf..97cc05901 100755
@@ -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" ]

index e30fb2cdd..a8518eddf 100755
@@ -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 &&

index 8757b3a97..86999268a 100755
@@ -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

s/execued/executed/

  # something went wrong. Enable tracing to ease debugging on TravisCI.

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.

---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

index 7c9aa0557..4684b3f4f 100644
@@ -42,6 +42,7 @@ matrix:
     - env: jobname=Linux32
       os: linux
       compiler:
+      addons:
       services:
         - docker
       before_install:

2.15.1.500.g54ea76cc4

Nice optimization - ACK!

Thanks,
Lars

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.

---
 ci/run-linux32-build.sh  | 1 +
 ci/run-linux32-docker.sh | 1 +
 2 files changed, 2 insertions(+)

index a8518eddf..c19c50c1c 100755
@@ -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
 '

index 0edf63acf..3a8b2ba42 100755
@@ -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

't/.prove' is a file, but '.../travis-cache' is a directory. It must be, because Travis CI caches whole directories.

Gábor

Of course. Your solution is the right one.

Thanks,
Lars

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.

---
 ci/print-test-failures.sh | 6 ++++++
 1 file changed, 6 insertions(+)

index 97cc05901..4f261ddc0 100755
@@ -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