Travis CI: check unignored build artifacts
cover Travis CI: check unignored build artifacts
Every once in a while our explicit .gitignore files get out of sync when our build process learns to create new artifacts, but the .gitignore files are not updated accordingly. It was recently that we got a report about unignored test helper executables, see 44103f419 (t/helper: ignore everything but sources, 2017-12-12).
This short patch series teaches our Travis CI build scripts to detect unignored build artifacts at the end of builds, in the hope to catch these issues earlier.
These patches should go on top 'sg/travis-skip-identical-test'. The two patch series are conceptually independent, but would have a couple of conflicts when applied separately and then merged together, and I don't think it's worth carrying them in separate branches.
SZEDER Gábor (2):
travis-ci: don't store P4 and Git LFS in the working tree
travis-ci: check that all build artifacts are .gitignore-d
ci/lib-travisci.sh | 14 ++++++++++++-- ci/run-linux32-docker.sh | 2 ++ ci/run-tests.sh | 2 ++ ci/test-documentation.sh | 6 ++++++ 4 files changed, 22 insertions(+), 2 deletions(-)
Signature · 1 line
2.16.0.rc0.67.g3a46dbca7
PATCH 1/2 travis-ci: don't store P4 and Git LFS in the working tree
The Clang and GCC 64 bit Linux build jobs download and store the P4 and Git LFS executables under the current directory, which is the working tree that we are about to build and test. This means that Git commands like 'status' or 'ls-files' would list these files as untracked. The next commit is about to make sure that there are no untracked files present after the build, and the downloaded executables in the working tree are interfering with those upcoming checks.
Therefore, let's download P4 and Git LFS in the home directory, outside of the working tree.
--- ci/lib-travisci.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ci/lib-travisci.sh +2 -2
@@ -99,8 +99,8 @@ linux-clang|linux-gcc) 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" + P4_PATH="$HOME/custom/p4" + GIT_LFS_PATH="$HOME/custom/git-lfs" export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" ;; osx-clang|osx-gcc)
Signature · 1 line
2.16.0.rc0.67.g3a46dbca7
Re: [PATCH 1/2] travis-ci: don't store P4 and Git LFS in the working tree
Quoted 12 lines · SZEDER Gábor
The Clang and GCC 64 bit Linux build jobs download and store the P4 and Git LFS executables under the current directory, which is the working tree that we are about to build and test. This means that Git commands like 'status' or 'ls-files' would list these files as untracked. The next commit is about to make sure that there are no untracked files present after the build, and the downloaded executables in the working tree are interfering with those upcoming checks.
Quoted 2 lines
Therefore, let's download P4 and Git LFS in the home directory, outside of the working tree.
I was concerned for a moment that the executables would not be available to the 32-bit build anymore... but we don't use them in that build anyways.
Looks good to me!
- Lars
Quoted 24 lines
Signed-off-by SZEDER Gábor <[email protected]>---
ci/lib-travisci.sh| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)ci/lib-travisci.sh +2
-2@@ -99,8 +99,8 @@ linux-clang|linux-gcc) 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" + P4_PATH="$HOME/custom/p4" + GIT_LFS_PATH="$HOME/custom/git-lfs" export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH" ;; osx-clang|osx-gcc)Signature · 1 line
2.16.0.rc0.67.g3a46dbca7
PATCH 2/2 travis-ci: check that all build artifacts are .gitignore-d
In reply to SZEDER GáborEvery once in a while our explicit .gitignore files get out of sync when our build process learns to create new artifacts, like test helper executables, but the .gitignore files are not updated accordingly.
Use Travis CI to help catch such issues earlier: check that there are no untracked files at the end of any build jobs building Git (i.e. the 64 bit Clang and GCC Linux and OSX build jobs, plus the GETTEXT_POISON and 32 bit Linux build jobs) or its documentation, and fail the build job if there are any present.
--- ci/lib-travisci.sh | 10 ++++++++++ ci/run-linux32-docker.sh | 2 ++ ci/run-tests.sh | 2 ++ ci/test-documentation.sh | 6 ++++++ 4 files changed, 20 insertions(+)
ci/lib-travisci.sh +10 -0
@@ -67,6 +67,16 @@ skip_good_tree () { exit 0 } +check_unignored_build_artifacts () +{ + ! git ls-files --other --exclude-standard --error-unmatch \ + -- ':/*' 2>/dev/null || + { + echo "$(tput setaf 1)error: found unignored build artifacts$(tput sgr0)" + false + } +} + # Set 'exit on error' for all CI scripts to let the caller know that # something went wrong. # Set tracing executed commands, primarily setting environment variables
ci/run-linux32-docker.sh +2 -0
@@ -23,4 +23,6 @@ docker run \ daald/ubuntu32:xenial \ /usr/src/git/ci/run-linux32-build.sh $(id -u $USER) +check_unignored_build_artifacts + save_good_tree
ci/run-tests.sh +2 -0
@@ -8,4 +8,6 @@ ln -s $HOME/travis-cache/.prove t/.prove make --quiet test +check_unignored_build_artifacts + save_good_tree
ci/test-documentation.sh +6 -0
@@ -18,6 +18,9 @@ test -s Documentation/git.xml test -s Documentation/git.1 grep '<meta name="generator" content="AsciiDoc ' Documentation/git.html +rm -f stdout.log stderr.log +check_unignored_build_artifacts + # Build docs with AsciiDoctor make clean make --jobs=2 USE_ASCIIDOCTOR=1 doc > >(tee stdout.log) 2> >(tee stderr.log >&2)
@@ -26,4 +29,7 @@ sed '/^GIT_VERSION = / d' stderr.log test -s Documentation/git.html grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html +rm -f stdout.log stderr.log +check_unignored_build_artifacts + save_good_tree
Signature · 1 line
2.16.0.rc0.67.g3a46dbca7
Re: [PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d
Quoted 27 lines · SZEDER Gábor
Every once in a while our explicit .gitignore files get out of sync when our build process learns to create new artifacts, like test helper executables, but the .gitignore files are not updated accordingly.
Use Travis CI to help catch such issues earlier: check that there are no untracked files at the end of any build jobs building Git (i.e. the 64 bit Clang and GCC Linux and OSX build jobs, plus the
GETTEXT_POISONand 32 bit Linux build jobs) or its documentation, and fail the build job if there are any present.Signed-off-by SZEDER Gábor <[email protected]>---
ci/lib-travisci.sh | 10 ++++++++++ ci/run-linux32-docker.sh | 2 ++ ci/run-tests.sh | 2 ++ ci/test-documentation.sh | 6 ++++++ 4 files changed, 20 insertions(+)ci/lib-travisci.sh +0
-0
ci/lib-travisci.sh skip_good_tree () { · line 73
Quoted 7 lines
ci/lib-travisci.sh +4
-0@@ -67,6 +67,16 @@ skip_good_tree () { exit 0 } +check_unignored_build_artifacts () +{ + ! git ls-files --other --exclude-standard --error-unmatch \ + -- ':/*' 2>/dev/null ||
What does "-- ':/*'" do? Plus, why do you redirect stderr?
--
Both patches look good to me!
Thanks,
Lars
Re: [PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d
Quoted 30 lines · Lars Schneider
Quoted 30 lines · SZEDER Gábor
Every once in a while our explicit .gitignore files get out of sync when our build process learns to create new artifacts, like test helper executables, but the .gitignore files are not updated accordingly.
Use Travis CI to help catch such issues earlier: check that there are no untracked files at the end of any build jobs building Git (i.e. the 64 bit Clang and GCC Linux and OSX build jobs, plus the
GETTEXT_POISONand 32 bit Linux build jobs) or its documentation, and fail the build job if there are any present.Signed-off-by SZEDER Gábor <[email protected]>---
ci/lib-travisci.sh | 10 ++++++++++ ci/run-linux32-docker.sh | 2 ++ ci/run-tests.sh | 2 ++ ci/test-documentation.sh | 6 ++++++ 4 files changed, 20 insertions(+)ci/lib-travisci.sh +0
-0@@ -67,6 +67,16 @@ skip_good_tree () { exit 0 }
Quoted 6 lines
ci/lib-travisci.sh skip_good_tree () { · line 73
Quoted 4 lines
ci/lib-travisci.sh +4
-0+check_unignored_build_artifacts () +{ + ! git ls-files --other --exclude-standard --error-unmatch \ + -- ':/*' 2>/dev/null ||What does "-- ':/*'" do?
It makes the whole thing work :)
':/*' is a pattern matching all paths, and '--others --exclude-standard' limit the command to list files that are both untracked and unignored. '--error-unmatch' causes the command to error out if any of the files given on the command line doesn't match anything in the working tree, in this case if there are no untracked-unignored files. Without a path given on the cmdline '--error-unmatch' has basically no effect[1].
In a clean worktree:
$ git ls-files --other --exclude-standard --error-unmatch ; echo $? 0 $ git ls-files --other --exclude-standard --error-unmatch ':/*' ; echo $? error: pathspec ':/*' did not match any file(s) known to git. Did you forget to 'git add'? 1
The disambiguating double-dash is not really necessary, because the :/* pattern can't be confused with any --options, but doesn't hurt, either.
Quoted 1 line
Plus, why do you redirect stderr?
To prevent the above error message from appearing in the trace log of a successful build.
[1] - Which makes me think whether we should consider '--error-unmatch' without any paths given on the command line as an error. On a related note: that error message doesn't seem to make much sense with '--other'...
Re: [PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d
Quoted 41 lines · SZEDER Gábor
Quoted 30 lines · Lars Schneider
Quoted 30 lines · SZEDER Gábor
Every once in a while our explicit .gitignore files get out of sync when our build process learns to create new artifacts, like test helper executables, but the .gitignore files are not updated accordingly.
Use Travis CI to help catch such issues earlier: check that there are no untracked files at the end of any build jobs building Git (i.e. the 64 bit Clang and GCC Linux and OSX build jobs, plus the
GETTEXT_POISONand 32 bit Linux build jobs) or its documentation, and fail the build job if there are any present.Signed-off-by SZEDER Gábor <[email protected]>---
ci/lib-travisci.sh | 10 ++++++++++ ci/run-linux32-docker.sh | 2 ++ ci/run-tests.sh | 2 ++ ci/test-documentation.sh | 6 ++++++ 4 files changed, 20 insertions(+)ci/lib-travisci.sh +0
-0@@ -67,6 +67,16 @@ skip_good_tree () { exit 0 }Quoted 6 lines
ci/lib-travisci.sh skip_good_tree () { · line 73
Quoted 4 lines
ci/lib-travisci.sh +4
-0+check_unignored_build_artifacts () +{ + ! git ls-files --other --exclude-standard --error-unmatch \ + -- ':/*' 2>/dev/null ||What does "-- ':/*'" do?
Quoted 7 lines
It makes the whole thing work :)
':/*' is a pattern matching all paths, and '--others --exclude-standard' limit the command to list files that are both untracked and unignored. '--error-unmatch' causes the command to error out if any of the files given on the command line doesn't match anything in the working tree, in this case if there are no untracked-unignored files. Without a path given on the cmdline '--error-unmatch' has basically no effect[1].
That was the missing piece. Thank you! I've used the exact command before but I've never looked at the exit code. I just assumed that it would behave like grep if there is no match (as you described in [1])
Quoted 9 lines
In a clean worktree:
$ git ls-files --other --exclude-standard --error-unmatch ; echo $? 0 $ git ls-files --other --exclude-standard --error-unmatch ':/*' ; echo $? error: pathspec ':/*' did not match any file(s) known to git. Did you forget to 'git add'? 1
Quoted 7 lines
The disambiguating double-dash is not really necessary, because the :/* pattern can't be confused with any --options, but doesn't hurt, either.
Quoted 1 line
Plus, why do you redirect stderr?
To prevent the above error message from appearing in the trace log of a successful build.
Makes sense. As I never specified the path, I did never see the error :-)
Quoted 2 lines
[1] - Which makes me think whether we should consider '--error-unmatch' without any paths given on the command line as an error.
Agreed!
- Lars