From f63b12392aa16a8a7a68900afa906b0382345aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 29 Jan 2018 18:17:09 +0100 Subject: travis-ci: use 'set -x' for the commands under 'su' in the 32 bit Linux build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- ci/run-linux32-build.sh | 1 + 1 file changed, 1 insertion(+) (limited to 'ci') diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index c19c50c1c9..5a36a8d7c0 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -26,6 +26,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 ' + set -x && cd /usr/src/git && ln -s /tmp/travis-cache/.prove t/.prove && make --jobs=2 && -- cgit v1.2.3 From 04d47e969a1fb952baffbd12a1a0dd2dc6cd2746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 29 Jan 2018 18:17:10 +0100 Subject: travis-ci: use 'set -e' in the 32 bit Linux build job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script 'ci/run-linux32-build.sh' running inside the Docker container of the 32 bit Linux build job uses an && chain to break the build if one of the commands fails. This is problematic for two reasons: - The && chain is broken, because there is this in the middle: test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) && Luckily it is broken in a way that it didn't lead to false successes. If installing dependencies fails, then the rest of the first && chain is skipped and execution resumes after the || operator. At that point $HOST_UID is still unset, causing 'useradd' to error out with "invalid user ID 'ci'", which in turn causes the second && chain to abort the script and thus break the build. - All other 'ci/*' scripts use 'set -e' to break the build if one of the commands fails. This inconsistency among these scripts is asking for trouble: I forgot about the && chain more than once while working on this patch series. Enable 'set -e' for the whole script and for the commands executed under 'su' as well. While touching every line in the 'su' command block anyway, change their indentation to use a tab instead of spaces. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- ci/run-linux32-build.sh | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'ci') diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index 5a36a8d7c0..248183982b 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -6,29 +6,29 @@ # run-linux32-build.sh [host-user-id] # -set -x +set -ex # Update packages to the latest available versions linux32 --32bit i386 sh -c ' apt update >/dev/null && apt install -y build-essential libcurl4-openssl-dev libssl-dev \ libexpat-dev gettext python >/dev/null -' && +' # If this script runs inside a docker container, then all commands are # usually executed as root. Consequently, the host user might not be # able to access the test output files. # If a host user id is given, then create a user "ci" with the host user # id to make everything accessible to the host user. -HOST_UID=$1 && -CI_USER=$USER && -test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) && +HOST_UID=$1 +CI_USER=$USER +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 ' - set -x && - cd /usr/src/git && - ln -s /tmp/travis-cache/.prove t/.prove && - make --jobs=2 && - make --quiet test + set -ex + cd /usr/src/git + ln -s /tmp/travis-cache/.prove t/.prove + make --jobs=2 + make --quiet test ' -- cgit v1.2.3 From b2cbaa091ca676478d131afd40ca4717cc5eed39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 29 Jan 2018 18:17:11 +0100 Subject: travis-ci: don't repeat the path of the cache directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of our 'ci/*' scripts repeat the name or full path of the Travis CI cache directory, and the following patches will add new places using that path. Use a variable to refer to the path of the cache directory instead, so it's hard-coded only in a single place. Pay extra attention to the 32 bit Linux build: it runs in a Docker container, so pass the path of the cache directory from the host to the container in an environment variable. Note that an environment variable passed this way is exported inside the container, therefore its value is directly available in the 'su' snippet even though that snippet is single quoted. Furthermore, use the variable in the container only if it's been assigned a non-empty value, to prevent errors when someone is running or debugging the Docker build locally, because in that case the variable won't be set as there won't be any Travis CI cache. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- ci/lib-travisci.sh | 7 ++++--- ci/run-linux32-build.sh | 2 +- ci/run-linux32-docker.sh | 5 ++++- ci/run-tests.sh | 3 ++- 4 files changed, 11 insertions(+), 6 deletions(-) (limited to 'ci') diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index 07f27c7270..1efee55ef7 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -21,8 +21,6 @@ skip_branch_tip_with_tag () { fi } -good_trees_file="$HOME/travis-cache/good-trees" - # Save some info about the current commit's tree, so we can skip the build # job if we encounter the same tree again and can provide a useful info # message. @@ -83,7 +81,10 @@ check_unignored_build_artifacts () # and installing dependencies. set -ex -mkdir -p "$HOME/travis-cache" +cache_dir="$HOME/travis-cache" +good_trees_file="$cache_dir/good-trees" + +mkdir -p "$cache_dir" skip_branch_tip_with_tag skip_good_tree diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index 248183982b..d020b762ca 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -28,7 +28,7 @@ test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) linux32 --32bit i386 su -m -l $CI_USER -c ' set -ex cd /usr/src/git - ln -s /tmp/travis-cache/.prove t/.prove + test -n "$cache_dir" && ln -s "$cache_dir/.prove" t/.prove make --jobs=2 make --quiet test ' diff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh index 4f191c5bb1..15288ea2cf 100755 --- a/ci/run-linux32-docker.sh +++ b/ci/run-linux32-docker.sh @@ -11,6 +11,8 @@ docker pull daald/ubuntu32:xenial # $ docker run -itv "${PWD}:/usr/src/git" --entrypoint /bin/bash daald/ubuntu32:xenial # root@container:/# /usr/src/git/ci/run-linux32-build.sh +container_cache_dir=/tmp/travis-cache + docker run \ --interactive \ --env DEVELOPER \ @@ -18,8 +20,9 @@ docker run \ --env GIT_PROVE_OPTS \ --env GIT_TEST_OPTS \ --env GIT_TEST_CLONE_2GB \ + --env cache_dir="$container_cache_dir" \ --volume "${PWD}:/usr/src/git" \ - --volume "${HOME}/travis-cache:/tmp/travis-cache" \ + --volume "$cache_dir:$container_cache_dir" \ daald/ubuntu32:xenial \ /usr/src/git/ci/run-linux32-build.sh $(id -u $USER) diff --git a/ci/run-tests.sh b/ci/run-tests.sh index 22355f0091..deba73d9b5 100755 --- a/ci/run-tests.sh +++ b/ci/run-tests.sh @@ -5,7 +5,8 @@ . ${0%/*}/lib-travisci.sh -ln -s $HOME/travis-cache/.prove t/.prove +ln -s "$cache_dir/.prove" t/.prove + make --quiet test check_unignored_build_artifacts -- cgit v1.2.3 From 533033024a15ad2aa7b853277cbb8f04d74edc48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 29 Jan 2018 18:17:12 +0100 Subject: travis-ci: don't run the test suite as root in the 32 bit Linux build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Travis CI runs the 32 bit Linux build job in a Docker container, where all commands are executed as root by default. Therefore, ever since we added this build job in 88dedd5e7 (Travis: also test on 32-bit Linux, 2017-03-05), we have a bit of code to create a user in the container matching the ID of the host user and then to run the test suite as this user. Matching the host user ID is important, because otherwise the host user would have no access to any files written by processes running in the container, notably the logs of failed tests couldn't be included in the build job's trace log. Alas, this piece of code never worked, because it sets the variable holding the user name ($CI_USER) in a subshell, meaning it doesn't have any effect by the time we get to the point to actually use the variable to switch users with 'su'. So all this time we were running the test suite as root. Reorganize that piece of code in 'ci/run-linux32-build.sh' a bit to avoid that problematic subshell and to ensure that we switch to the right user. Furthermore, make the script's optional host user ID option mandatory, so running the build accidentally as root will become harder when debugging locally. If someone really wants to run the test suite as root, whatever the reasons might be, it'll still be possible to do so by explicitly passing '0' as host user ID. Finally, one last catch: since commit 7e72cfcee (travis-ci: save prove state for the 32 bit Linux build, 2017-12-27) the 'prove' test harness has been writing its state to the Travis CI cache directory from within the Docker container while running as root. After this patch 'prove' will run as a regular user, so in future build jobs it won't be able overwrite a previously written, still root-owned state file, resulting in build job failures. To resolve this we should manually delete caches containing such root-owned files, but that would be a hassle. Instead, work this around by changing the owner of the whole contents of the cache directory to the host user ID. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- ci/run-linux32-build.sh | 30 +++++++++++++++++++++++++----- ci/run-linux32-docker.sh | 2 +- 2 files changed, 26 insertions(+), 6 deletions(-) (limited to 'ci') diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index d020b762ca..8c1b500e63 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -3,11 +3,17 @@ # Build and test Git in a 32-bit environment # # Usage: -# run-linux32-build.sh [host-user-id] +# run-linux32-build.sh # set -ex +if test $# -ne 1 || test -z "$1" +then + echo >&2 "usage: run-linux32-build.sh " + exit 1 +fi + # Update packages to the latest available versions linux32 --32bit i386 sh -c ' apt update >/dev/null && @@ -18,11 +24,25 @@ linux32 --32bit i386 sh -c ' # If this script runs inside a docker container, then all commands are # usually executed as root. Consequently, the host user might not be # able to access the test output files. -# If a host user id is given, then create a user "ci" with the host user -# id to make everything accessible to the host user. +# If a non 0 host user id is given, then create a user "ci" with that +# user id to make everything accessible to the host user. HOST_UID=$1 -CI_USER=$USER -test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) +if test $HOST_UID -eq 0 +then + # Just in case someone does want to run the test suite as root. + CI_USER=root +else + CI_USER=ci + useradd -u $HOST_UID $CI_USER + # Due to a bug the test suite was run as root in the past, so + # a prove state file created back then is only accessible by + # root. Now that bug is fixed, the test suite is run as a + # regular user, but the prove state file coming from Travis + # CI's cache might still be owned by root. + # Make sure that this user has rights to any cached files, + # including an existing prove state file. + test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir" +fi # Build and test linux32 --32bit i386 su -m -l $CI_USER -c ' diff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh index 15288ea2cf..21637903ce 100755 --- a/ci/run-linux32-docker.sh +++ b/ci/run-linux32-docker.sh @@ -9,7 +9,7 @@ docker pull daald/ubuntu32:xenial # Use the following command to debug the docker build locally: # $ docker run -itv "${PWD}:/usr/src/git" --entrypoint /bin/bash daald/ubuntu32:xenial -# root@container:/# /usr/src/git/ci/run-linux32-build.sh +# root@container:/# /usr/src/git/ci/run-linux32-build.sh container_cache_dir=/tmp/travis-cache -- cgit v1.2.3 From 6b995760dc463d050791d3815c20e4cb78ce4c2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Mon, 29 Jan 2018 18:17:13 +0100 Subject: travis-ci: don't fail if user already exists on 32 bit Linux build job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 32 bit Linux build job runs in a Docker container, which lends itself to running and debugging locally, too. Especially during debugging one usually doesn't want to start with a fresh container every time, to save time spent on installing a bunch of dependencies. However, that doesn't work quite smootly, because the script running in the container always creates a new user, which then must be removed every time before subsequent executions, or the build script fails. Make this process more convenient and don't try to create that user if it already exists and has the right user ID in the container, so developers don't have to bother with running a 'userdel' each time before they run the build script. The build job on Travis CI always starts with a fresh Docker container, so this change doesn't make a difference there. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- ci/run-linux32-build.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'ci') diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index 8c1b500e63..2c60d2e70a 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -33,7 +33,13 @@ then CI_USER=root else CI_USER=ci - useradd -u $HOST_UID $CI_USER + if test "$(id -u $CI_USER 2>/dev/null)" = $HOST_UID + then + echo "user '$CI_USER' already exists with the requested ID $HOST_UID" + else + useradd -u $HOST_UID $CI_USER + fi + # Due to a bug the test suite was run as root in the past, so # a prove state file created back then is only accessible by # root. Now that bug is fixed, the test suite is run as a -- cgit v1.2.3