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

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

[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

[PATCH 0/4] travis-ci: clean up setting environment variables

[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

[PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[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

[PATCH 2/4] travis-ci: introduce a $jobname variable for 'ci/*' scripts

[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

[PATCH 3/4] travis-ci: move setting environment variables to 'ci/lib-travisci.sh'

[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

[PATCH 4/4] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh'

[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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Lars Schneider]

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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[SZEDER Gábor]

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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Lars Schneider]

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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[SZEDER Gábor]

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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Johannes Schindelin]

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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[SZEDER Gábor]

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

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Johannes Schindelin]

[-- 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

[PATCH v2 0/8] Travis CI cleanups

[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

[PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing

[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

[PATCH v2 2/8] travis-ci: introduce a $jobname variable for 'ci/*' scripts

[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

[PATCH v2 3/8] travis-ci: move setting environment variables to 'ci/lib-travisci.sh'

[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

[PATCH v2 4/8] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh'

[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

[PATCH v2 5/8] travis-ci: don't install default addon packages for the 32 bit Linux build

[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

[PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package

[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

[PATCH v2 7/8] travis-ci: save prove state for the 32 bit Linux build

[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

[PATCH v2 8/8] travis-ci: only print test failures if there are test results available

[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

Re: [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing

[Johannes Schindelin]

[-- 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

Re: [PATCH v2 8/8] travis-ci: only print test failures if there are test results available

[Eric Sunshine]

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

Re: [PATCH v2 8/8] travis-ci: only print test failures if there are

[SZEDER Gábor]

> 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

Re: [PATCH v2 8/8] travis-ci: only print test failures if there are

[Eric Sunshine]

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.

Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package

[Lars Schneider]

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

Re: [PATCH v2 0/8] Travis CI cleanups

[Lars Schneider]

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

Re: [PATCH v2 1/8] travis-ci: use 'set -x' in select 'ci/*' scripts for extra tracing

[Lars Schneider]

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

Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package

[SZEDER Gábor]

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

Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package

[Lars Schneider]

> 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

Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package

[Junio C Hamano]

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.

Re: [PATCH v2 6/8] travis-ci: don't install 'language-pack-is' package

[SZEDER Gábor]

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

[PATCH v3 0/4] Rest of the Travis CI fixes

[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

[PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts

[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

[PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build

[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

[PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build

[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

[PATCH v3 4/4] travis-ci: only print test failures if there are test results available

[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

Re: [PATCH v3 1/4] travis-ci: fine tune the use of 'set -x' in 'ci/*' scripts

[Lars Schneider]

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

Re: [PATCH v3 2/4] travis-ci: don't install default addon packages for the 32 bit Linux build

[Lars Schneider]

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

Re: [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build

[Lars Schneider]

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

Re: [PATCH v3 4/4] travis-ci: only print test failures if there are test results available

[Lars Schneider]

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

Re: [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build

[SZEDER Gábor]

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

Re: [PATCH v3 3/4] travis-ci: save prove state for the 32 bit Linux build

[Lars Schneider]

> 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