{"thread":{"id":"47518","subject":"[PATCH 0/2] Travis CI: check unignored build artifacts","startedAt":"2017-12-31T16:02:33Z","lastAt":"2018-01-03T09:45:50Z","messageCount":7,"participants":["SZEDER Gábor","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"335583","messageId":"20171231160206.19481-1-szeder.dev@gmail.com","threadId":"47518","inReplyTo":null,"subject":"[PATCH 0/2] Travis CI: check unignored build artifacts","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-12-31T16:02:04Z","receivedAt":"2017-12-31T16:02:33Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Every once in a while our explicit .gitignore files get out of sync when\nour build process learns to create new artifacts, but the .gitignore\nfiles are not updated accordingly. It was recently that we got a report\nabout unignored test helper executables, see 44103f419 (t/helper: ignore\neverything but sources, 2017-12-12).\n\nThis short patch series teaches our Travis CI build scripts to detect\nunignored build artifacts at the end of builds, in the hope to catch\nthese issues earlier.\n\nThese patches should go on top 'sg/travis-skip-identical-test'. The two\npatch series are conceptually independent, but would have a couple of\nconflicts when applied separately and then merged together, and I don't\nthink it's worth carrying them in separate branches.\n\n\nSZEDER Gábor (2):\n travis-ci: don't store P4 and Git LFS in the working tree\n travis-ci: check that all build artifacts are .gitignore-d\n\n ci/lib-travisci.sh | 14 ++++++++++++--\n ci/run-linux32-docker.sh | 2 ++\n ci/run-tests.sh | 2 ++\n ci/test-documentation.sh | 6 ++++++\n 4 files changed, 22 insertions(+), 2 deletions(-)\n\n-- \n2.16.0.rc0.67.g3a46dbca7\n\n"},{"id":"335584","messageId":"20171231160206.19481-2-szeder.dev@gmail.com","threadId":"47518","inReplyTo":"20171231160206.19481-1-szeder.dev@gmail.com","subject":"[PATCH 1/2] travis-ci: don't store P4 and Git LFS in the working tree","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-12-31T16:02:05Z","receivedAt":"2017-12-31T16:02:36Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The Clang and GCC 64 bit Linux build jobs download and store the P4\nand Git LFS executables under the current directory, which is the\nworking tree that we are about to build and test. This means that Git\ncommands like 'status' or 'ls-files' would list these files as\nuntracked. The next commit is about to make sure that there are no\nuntracked files present after the build, and the downloaded\nexecutables in the working tree are interfering with those upcoming\nchecks.\n\nTherefore, let's download P4 and Git LFS in the home directory,\noutside of the working tree.\n\nSigned-off-by: SZEDER Gábor \n---\n ci/lib-travisci.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\nindex bade71617..1543b7959 100755\n--- a/ci/lib-travisci.sh\n+++ b/ci/lib-travisci.sh\n@@ -99,8 +99,8 @@ linux-clang|linux-gcc)\n \texport LINUX_P4_VERSION=\"16.2\"\n \texport LINUX_GIT_LFS_VERSION=\"1.5.2\"\n \n-\tP4_PATH=\"$(pwd)/custom/p4\"\n-\tGIT_LFS_PATH=\"$(pwd)/custom/git-lfs\"\n+\tP4_PATH=\"$HOME/custom/p4\"\n+\tGIT_LFS_PATH=\"$HOME/custom/git-lfs\"\n \texport PATH=\"$GIT_LFS_PATH:$P4_PATH:$PATH\"\n \t;;\n osx-clang|osx-gcc)\n-- \n2.16.0.rc0.67.g3a46dbca7\n\n"},{"id":"335585","messageId":"20171231160206.19481-3-szeder.dev@gmail.com","threadId":"47518","inReplyTo":"20171231160206.19481-1-szeder.dev@gmail.com","subject":"[PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-12-31T16:02:06Z","receivedAt":"2017-12-31T16:02:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Every once in a while our explicit .gitignore files get out of sync\nwhen our build process learns to create new artifacts, like test\nhelper executables, but the .gitignore files are not updated\naccordingly.\n\nUse Travis CI to help catch such issues earlier: check that there are\nno untracked files at the end of any build jobs building Git (i.e. the\n64 bit Clang and GCC Linux and OSX build jobs, plus the GETTEXT_POISON\nand 32 bit Linux build jobs) or its documentation, and fail the build\njob if there are any present.\n\nSigned-off-by: SZEDER Gábor \n---\n ci/lib-travisci.sh | 10 ++++++++++\n ci/run-linux32-docker.sh | 2 ++\n ci/run-tests.sh | 2 ++\n ci/test-documentation.sh | 6 ++++++\n 4 files changed, 20 insertions(+)\n\ndiff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\nindex 1543b7959..07f27c727 100755\n--- a/ci/lib-travisci.sh\n+++ b/ci/lib-travisci.sh\n@@ -67,6 +67,16 @@ skip_good_tree () {\n \texit 0\n }\n \n+check_unignored_build_artifacts ()\n+{\n+\t! git ls-files --other --exclude-standard --error-unmatch \\\n+\t\t-- ':/*' 2>/dev/null ||\n+\t{\n+\t\techo \"$(tput setaf 1)error: found unignored build artifacts$(tput sgr0)\"\n+\t\tfalse\n+\t}\n+}\n+\n # Set 'exit on error' for all CI scripts to let the caller know that\n # something went wrong.\n # Set tracing executed commands, primarily setting environment variables\ndiff --git a/ci/run-linux32-docker.sh b/ci/run-linux32-docker.sh\nindex 870a41246..4f191c5bb 100755\n--- a/ci/run-linux32-docker.sh\n+++ b/ci/run-linux32-docker.sh\n@@ -23,4 +23,6 @@ docker run \\\n \tdaald/ubuntu32:xenial \\\n \t/usr/src/git/ci/run-linux32-build.sh $(id -u $USER)\n \n+check_unignored_build_artifacts\n+\n save_good_tree\ndiff --git a/ci/run-tests.sh b/ci/run-tests.sh\nindex eb5ba4058..22355f009 100755\n--- a/ci/run-tests.sh\n+++ b/ci/run-tests.sh\n@@ -8,4 +8,6 @@\n ln -s $HOME/travis-cache/.prove t/.prove\n make --quiet test\n \n+check_unignored_build_artifacts\n+\n save_good_tree\ndiff --git a/ci/test-documentation.sh b/ci/test-documentation.sh\nindex 3d62e6c95..a20de9ca1 100755\n--- a/ci/test-documentation.sh\n+++ b/ci/test-documentation.sh\n@@ -18,6 +18,9 @@ test -s Documentation/git.xml\n test -s Documentation/git.1\n grep ' >(tee stdout.log) 2> >(tee stderr.log >&2)\n@@ -26,4 +29,7 @@ sed '/^GIT_VERSION = / d' stderr.log\n test -s Documentation/git.html\n grep ' On 31 Dec 2017, at 17:02, SZEDER Gábor wrote:\n> \n> The Clang and GCC 64 bit Linux build jobs download and store the P4\n> and Git LFS executables under the current directory, which is the\n> working tree that we are about to build and test. This means that Git\n> commands like 'status' or 'ls-files' would list these files as\n> untracked. The next commit is about to make sure that there are no\n> untracked files present after the build, and the downloaded\n> executables in the working tree are interfering with those upcoming\n> checks.\n> \n> Therefore, let's download P4 and Git LFS in the home directory,\n> outside of the working tree.\n\nI was concerned for a moment that the executables would not be \navailable to the 32-bit build anymore... but we don't use them\nin that build anyways.\n\nLooks good to me!\n\n- Lars\n\n> \n> Signed-off-by: SZEDER Gábor \n> ---\n> ci/lib-travisci.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\n> index bade71617..1543b7959 100755\n> --- a/ci/lib-travisci.sh\n> +++ b/ci/lib-travisci.sh\n> @@ -99,8 +99,8 @@ linux-clang|linux-gcc)\n> \texport LINUX_P4_VERSION=\"16.2\"\n> \texport LINUX_GIT_LFS_VERSION=\"1.5.2\"\n> \n> -\tP4_PATH=\"$(pwd)/custom/p4\"\n> -\tGIT_LFS_PATH=\"$(pwd)/custom/git-lfs\"\n> +\tP4_PATH=\"$HOME/custom/p4\"\n> +\tGIT_LFS_PATH=\"$HOME/custom/git-lfs\"\n> \texport PATH=\"$GIT_LFS_PATH:$P4_PATH:$PATH\"\n> \t;;\n> osx-clang|osx-gcc)\n> -- \n> 2.16.0.rc0.67.g3a46dbca7\n> \n\n"},{"id":"335623","messageId":"D977F37C-D0E5-4EEB-98B4-693A5179F7B1@gmail.com","threadId":"47518","inReplyTo":"20171231160206.19481-3-szeder.dev@gmail.com","subject":"Re: [PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-02T19:40:19Z","receivedAt":"2018-01-02T19:40:26Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 31 Dec 2017, at 17:02, SZEDER Gábor wrote:\n> \n> Every once in a while our explicit .gitignore files get out of sync\n> when our build process learns to create new artifacts, like test\n> helper executables, but the .gitignore files are not updated\n> accordingly.\n> \n> Use Travis CI to help catch such issues earlier: check that there are\n> no untracked files at the end of any build jobs building Git (i.e. the\n> 64 bit Clang and GCC Linux and OSX build jobs, plus the GETTEXT_POISON\n> and 32 bit Linux build jobs) or its documentation, and fail the build\n> job if there are any present.\n> \n> Signed-off-by: SZEDER Gábor \n> ---\n> ci/lib-travisci.sh | 10 ++++++++++\n> ci/run-linux32-docker.sh | 2 ++\n> ci/run-tests.sh | 2 ++\n> ci/test-documentation.sh | 6 ++++++\n> 4 files changed, 20 insertions(+)\n> \n> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\n> index 1543b7959..07f27c727 100755\n> --- a/ci/lib-travisci.sh\n> +++ b/ci/lib-travisci.sh\n> @@ -67,6 +67,16 @@ skip_good_tree () {\n> \texit 0\n> }\n> \n> +check_unignored_build_artifacts ()\n> +{\n> +\t! git ls-files --other --exclude-standard --error-unmatch \\\n> +\t\t-- ':/*' 2>/dev/null ||\n\nWhat does \"-- ':/*'\" do? Plus, why do you redirect stderr?\n\n--\n\nBoth patches look good to me!\n\nThanks,\nLars"},{"id":"335643","messageId":"CAM0VKjmj7VGr5rcON=2VEug-fGtBrvNqhopH9x7pjB237V796g@mail.gmail.com","threadId":"47518","inReplyTo":"D977F37C-D0E5-4EEB-98B4-693A5179F7B1@gmail.com","subject":"Re: [PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-01-02T23:12:47Z","receivedAt":"2018-01-02T23:12:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jan 2, 2018 at 8:40 PM, Lars Schneider wrote:\n>\n>> On 31 Dec 2017, at 17:02, SZEDER Gábor wrote:\n>>\n>> Every once in a while our explicit .gitignore files get out of sync\n>> when our build process learns to create new artifacts, like test\n>> helper executables, but the .gitignore files are not updated\n>> accordingly.\n>>\n>> Use Travis CI to help catch such issues earlier: check that there are\n>> no untracked files at the end of any build jobs building Git (i.e. the\n>> 64 bit Clang and GCC Linux and OSX build jobs, plus the GETTEXT_POISON\n>> and 32 bit Linux build jobs) or its documentation, and fail the build\n>> job if there are any present.\n>>\n>> Signed-off-by: SZEDER Gábor \n>> ---\n>> ci/lib-travisci.sh | 10 ++++++++++\n>> ci/run-linux32-docker.sh | 2 ++\n>> ci/run-tests.sh | 2 ++\n>> ci/test-documentation.sh | 6 ++++++\n>> 4 files changed, 20 insertions(+)\n>>\n>> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\n>> index 1543b7959..07f27c727 100755\n>> --- a/ci/lib-travisci.sh\n>> +++ b/ci/lib-travisci.sh\n>> @@ -67,6 +67,16 @@ skip_good_tree () {\n>> exit 0\n>> }\n>>\n>> +check_unignored_build_artifacts ()\n>> +{\n>> + ! git ls-files --other --exclude-standard --error-unmatch \\\n>> + -- ':/*' 2>/dev/null ||\n>\n> What does \"-- ':/*'\" do?\n\nIt makes the whole thing work :)\n':/*' is a pattern matching all paths, and '--others --exclude-standard'\nlimit the command to list files that are both untracked and unignored.\n'--error-unmatch' causes the command to error out if any of the files\ngiven on the command line doesn't match anything in the working tree,\nin this case if there are no untracked-unignored files. Without a path\ngiven on the cmdline '--error-unmatch' has basically no effect[1].\n\nIn a clean worktree:\n\n $ git ls-files --other --exclude-standard --error-unmatch ; echo $?\n 0\n $ git ls-files --other --exclude-standard --error-unmatch ':/*' ; echo $?\n error: pathspec ':/*' did not match any file(s) known to git.\n Did you forget to 'git add'?\n 1\n\nThe disambiguating double-dash is not really necessary, because the :/*\npattern can't be confused with any --options, but doesn't hurt, either.\n\n> Plus, why do you redirect stderr?\n\nTo prevent the above error message from appearing in the trace log of a\nsuccessful build.\n\n\n[1] - Which makes me think whether we should consider '--error-unmatch'\n without any paths given on the command line as an error.\n On a related note: that error message doesn't seem to make much\n sense with '--other'...\n"},{"id":"335709","messageId":"041D7B2A-F7C2-4842-9A83-2020CEAC3B75@gmail.com","threadId":"47518","inReplyTo":"CAM0VKjmj7VGr5rcON=2VEug-fGtBrvNqhopH9x7pjB237V796g@mail.gmail.com","subject":"Re: [PATCH 2/2] travis-ci: check that all build artifacts are .gitignore-d","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-03T09:45:42Z","receivedAt":"2018-01-03T09:45:50Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 03 Jan 2018, at 00:12, SZEDER Gábor wrote:\n> \n> On Tue, Jan 2, 2018 at 8:40 PM, Lars Schneider wrote:\n>> \n>>> On 31 Dec 2017, at 17:02, SZEDER Gábor wrote:\n>>> \n>>> Every once in a while our explicit .gitignore files get out of sync\n>>> when our build process learns to create new artifacts, like test\n>>> helper executables, but the .gitignore files are not updated\n>>> accordingly.\n>>> \n>>> Use Travis CI to help catch such issues earlier: check that there are\n>>> no untracked files at the end of any build jobs building Git (i.e. the\n>>> 64 bit Clang and GCC Linux and OSX build jobs, plus the GETTEXT_POISON\n>>> and 32 bit Linux build jobs) or its documentation, and fail the build\n>>> job if there are any present.\n>>> \n>>> Signed-off-by: SZEDER Gábor \n>>> ---\n>>> ci/lib-travisci.sh | 10 ++++++++++\n>>> ci/run-linux32-docker.sh | 2 ++\n>>> ci/run-tests.sh | 2 ++\n>>> ci/test-documentation.sh | 6 ++++++\n>>> 4 files changed, 20 insertions(+)\n>>> \n>>> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\n>>> index 1543b7959..07f27c727 100755\n>>> --- a/ci/lib-travisci.sh\n>>> +++ b/ci/lib-travisci.sh\n>>> @@ -67,6 +67,16 @@ skip_good_tree () {\n>>> exit 0\n>>> }\n>>> \n>>> +check_unignored_build_artifacts ()\n>>> +{\n>>> + ! git ls-files --other --exclude-standard --error-unmatch \\\n>>> + -- ':/*' 2>/dev/null ||\n>> \n>> What does \"-- ':/*'\" do?\n> \n> It makes the whole thing work :)\n> ':/*' is a pattern matching all paths, and '--others --exclude-standard'\n> limit the command to list files that are both untracked and unignored.\n> '--error-unmatch' causes the command to error out if any of the files\n> given on the command line doesn't match anything in the working tree,\n> in this case if there are no untracked-unignored files. Without a path\n> given on the cmdline '--error-unmatch' has basically no effect[1].\n\nThat was the missing piece. Thank you! I've used the exact command before \nbut I've never looked at the exit code. I just assumed that it would\nbehave like grep if there is no match (as you described in [1])\n\n\n> In a clean worktree:\n> \n> $ git ls-files --other --exclude-standard --error-unmatch ; echo $?\n> 0\n> $ git ls-files --other --exclude-standard --error-unmatch ':/*' ; echo $?\n> error: pathspec ':/*' did not match any file(s) known to git.\n> Did you forget to 'git add'?\n> 1\n> \n> The disambiguating double-dash is not really necessary, because the :/*\n> pattern can't be confused with any --options, but doesn't hurt, either.\n> \n>> Plus, why do you redirect stderr?\n> \n> To prevent the above error message from appearing in the trace log of a\n> successful build.\n\nMakes sense. As I never specified the path, I did never see the error :-)\n\n\n> [1] - Which makes me think whether we should consider '--error-unmatch'\n> without any paths given on the command line as an error.\n\nAgreed!\n\n\n- Lars"}]}