From bf427a94517dfcd09b564d327526b16908bcc0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 12 Dec 2017 00:34:44 +0100 Subject: [PATCH 1/4] travis-ci: introduce a $jobname variable for 'ci/*' scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- .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 281f101f31..88435e11c0 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 5bd06fe900..4687885664 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 ac05f1f469..6ab8bd7622 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -27,8 +27,13 @@ set -e 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" From e3371e9260149b016b7e0d80eca5dd2a3e0b4d61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 12 Dec 2017 00:34:45 +0100 Subject: [PATCH 2/4] travis-ci: move setting environment variables to 'ci/lib-travisci.sh' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- .travis.yml | 18 +----------------- ci/lib-travisci.sh | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.travis.yml b/.travis.yml index 88435e11c0..7c9aa0557e 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 6ab8bd7622..8e75982a91 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 From a1157b76ebb480a4a41a22f44cfa2d411693f326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 12 Dec 2017 00:34:46 +0100 Subject: [PATCH 3/4] travis-ci: set GIT_TEST_HTTPD in 'ci/lib-travisci.sh' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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 4687885664..75a9fd2475 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 8e75982a91..0e1c38491c 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! From 4f263666791e76adae0128641fa362179b6caa6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 12 Dec 2017 00:34:43 +0100 Subject: [PATCH 4/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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 0e1c38491c..331d3eb3a6 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