[PATCH v4 00/31] CI: run "make" in CI "steps", improve UX
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Apr 18, 2022, 16:28 UTC
- Message-ID
- <cover-v4-00.31-00000000000-20220418T132809Z-avarab@gmail.com>
- In-Reply-To
- <cover-v3-00.29-00000000000-20220413T194847Z-avarab@gmail.com>
This series changes CI "step" targets that are shellscripts that do N things to instead be single command invocations at the "step" level, driven by the CI recipe itself.
To do that we need to pass state that we previously re-setup for every "step" via $GITHUB_ENV, whose state we then helpfully show (this is just a standard GitHub CI feature) in a drop-down at the start of every "step".
I.e. at the tip of this series you can reliably look at that $GITHUB_ENV view to see what the full and relevant environment was for that "make", "make test" or whatever.
Changes since v3:
* Typo/grammar fixes pointed out by Eric Sunshine
* The ci/print-test-failures.sh scripts still had some dead Travis code (noticed while re-rolling). Removed it, and squashed into the travis removal commit.
* Split up the part of "CI: combine
ci/install{,-docker}-dependencies.sh" that was replacing bash
constructs with POSIX ones into a preceding commit doing only that.* There was a regression in "CI: set CC in MAKEFLAGS directly, don't add it to the environment" where we didn't properly install the right CI package. The whole CC_PACKAGE is now contained within "ci/install-dependencies.sh".
* In addition to the regression Carlo noted already on "master" where the osx-gcc job is using clang. That's now fixed in a new "CI: have osx-gcc use gcc, not clang".
* Extend the last commit to have an "nproc" on BSD/OSX/Windows too, this is only for the case of ad-hoc running ci/lib.sh locally.
Ævar Arnfjörð Bjarmason (31):
CI: run "set -ex" early in ci/lib.sh
CI: make "$jobname" explicit, remove fallback
CI: remove more dead Travis CI support
CI: remove dead "tree skipping" code
CI: remove unused Azure ci/* code
CI/lib.sh: stop adding leading whitespace to $MAKEFLAGS
CI: don't have "git grep" invoke a pager in tree content check
CI: have "static-analysis" run a "make ci-static-analysis" target
CI: have "static-analysis" run "check-builtins", not "documentation"
CI: move p4 and git-lfs variables to ci/install-dependencies.sh
CI: consistently use "export" in ci/lib.sh
CI: export variables via a wrapper
CI: remove "run-build-and-tests.sh", run "make [test]" directly
ci/lib.sh: use "test" instead of "["
CI: check ignored unignored build artifacts in "win[+VS] build" too
CI: invoke "make artifacts-tar" directly in windows-build
CI: split up and reduce "ci/test-documentation.sh"
CI: make ci/install-dependencies.sh POSIX-compatible
CI: combine ci/install{,-docker}-dependencies.sh
CI: move "env" definitions into ci/lib.sh
ci/run-test-slice.sh: replace shelling out with "echo"
CI: pre-select test slice in Windows & VS tests
CI: only invoke ci/lib.sh as "steps" in main.yml
CI: narrow down variable definitions in --build and --test
CI: add more variables to MAKEFLAGS, except under vs-build
CI: set CC in MAKEFLAGS directly, don't add it to the environment
CI: set SANITIZE=leak in MAKEFLAGS directly
CI: set PYTHON_PATH setting for osx-{clang,gcc} into "$jobname" case
CI: don't use "set -x" in "ci/lib.sh" output
CI: have osx-gcc use gcc, not clang
CI: make it easy to use ci/*.sh outside of CI.github/workflows/main.yml | 100 ++++--- Makefile | 31 ++- ci/check-directional-formatting.bash | 2 +- ci/check-unignored-build-artifacts.sh | 20 ++ ci/install-dependencies.sh | 74 ++++- ci/install-docker-dependencies.sh | 22 -- ci/lib-ci-type.sh | 6 + ci/lib-online_cpus.sh | 26 ++ ci/lib-tput.sh | 5 + ci/lib.sh | 379 +++++++++++++++----------- ci/make-test-artifacts.sh | 12 - ci/mount-fileshare.sh | 25 -- ci/print-test-failures.sh | 53 +--- ci/run-build-and-tests.sh | 54 ---- ci/run-docker-build.sh | 66 ----- ci/run-docker.sh | 47 ---- ci/run-static-analysis.sh | 32 --- ci/run-test-slice.sh | 17 -- ci/select-test-slice.sh | 10 + ci/test-documentation.sh | 37 +-- ci/util/extract-trash-dirs.sh | 50 ---- shared.mak | 1 + 22 files changed, 453 insertions(+), 616 deletions(-) create mode 100755 ci/check-unignored-build-artifacts.sh delete mode 100755 ci/install-docker-dependencies.sh create mode 100644 ci/lib-ci-type.sh create mode 100644 ci/lib-online_cpus.sh create mode 100644 ci/lib-tput.sh delete mode 100755 ci/make-test-artifacts.sh delete mode 100755 ci/mount-fileshare.sh delete mode 100755 ci/run-build-and-tests.sh delete mode 100755 ci/run-docker-build.sh delete mode 100755 ci/run-docker.sh delete mode 100755 ci/run-static-analysis.sh delete mode 100755 ci/run-test-slice.sh create mode 100755 ci/select-test-slice.sh delete mode 100755 ci/util/extract-trash-dirs.sh
Range-diff against v3:
1: 3cb4749c5ea = 1: ed27ca0b7e7 CI: run "set -ex" early in ci/lib.sh
2: 7de95ff437e = 2: 737fe1e2c4c CI: make "$jobname" explicit, remove fallback
3: 820c4e551d9 ! 3: d875069fc28 CI: remove more dead Travis CI support
@@ Commit message
would do the wrong thing, because the different test slices would each
try to clobber the same "t/.prove" file.
- If a subsequent run then used the -"-state=failed,slow,save" it would
+ If a subsequent run then used the "--state=failed,slow,save" it would
defeat the purpose of "ci/run-test-slice.sh", since all slices would
then run all tests. I.e. behavior of prove's "--state" options is to
select tests to run from the provided "--state" file, in addition to
@@ Commit message
".github/workflows/main.yml" calls the same entry points as the main
"regular" job.
+ For "ci/print-test-failures.sh" the creation of the tarball added in
+ aea8879a6ac (travis-ci: include the trash directories of failed tests
+ in the trace log, 2018-08-01) has been dead since my 4a6e4b96026,
+ which removed the fall-through case from the "case/esac" statement.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## ci/lib.sh ##
@@ ci/lib.sh: then
echo "Could not identify CI type" >&2
## ci/print-test-failures.sh ##
-@@ ci/print-test-failures.sh: do
- fi
- combined_trash_size=$new_combined_trash_size
+@@ ci/print-test-failures.sh: then
+ exit
+ fi
+-case "$jobname" in
+-osx-clang|osx-gcc)
+- # base64 in OSX doesn't wrap its output at 76 columns by
+- # default, but prints a single, very long line.
+- base64_opts="-b 76"
+- ;;
+-esac
+-
+-combined_trash_size=0
+ for TEST_EXIT in test-results/*.exit
+ do
+ if [ "$(cat "$TEST_EXIT")" != "0" ]
+@@ ci/print-test-failures.sh: do
+ azure-pipelines)
+ mkdir -p failed-test-artifacts
+ mv "$trash_dir" failed-test-artifacts
+- continue
+ ;;
+ github-actions)
+ mkdir -p failed-test-artifacts
+ echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
+ cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
+ tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+- continue
+ ;;
+ *)
+ echo "Unhandled CI type: $CI_TYPE" >&2
+ exit 1
+ ;;
+ esac
+- trash_tgz_b64="trash.$test_name.base64"
+- if [ -d "$trash_dir" ]
+- then
+- tar czp "$trash_dir" |base64 $base64_opts >"$trash_tgz_b64"
+-
+- trash_size=$(wc -c <"$trash_tgz_b64")
+- if [ $trash_size -gt 1048576 ]
+- then
+- # larger than 1MB
+- echo "$(tput setaf 1)Didn't include the trash directory of '$test_name' in the trace log, it's too big$(tput sgr0)"
+- continue
+- fi
+-
+- new_combined_trash_size=$(($combined_trash_size + $trash_size))
+- if [ $new_combined_trash_size -gt 1048576 ]
+- then
+- echo "$(tput setaf 1)Didn't include the trash directory of '$test_name' in the trace log, there is plenty of trash in there already.$(tput sgr0)"
+- continue
+- fi
+- combined_trash_size=$new_combined_trash_size
+-
- # DO NOT modify these two 'echo'-ed strings below
- # without updating 'ci/util/extract-trash-dirs.sh'
- # as well.
- echo "$(tput setaf 1)Start of trash directory of '$test_name':$(tput sgr0)"
- cat "$trash_tgz_b64"
- echo "$(tput setaf 1)End of trash directory of '$test_name'$(tput sgr0)"
+- echo "$(tput setaf 1)Start of trash directory of '$test_name':$(tput sgr0)"
+- cat "$trash_tgz_b64"
+- echo "$(tput setaf 1)End of trash directory of '$test_name'$(tput sgr0)"
+- fi
+ fi
+ done
## ci/run-build-and-tests.sh ##
@@
4: 914e81118b8 = 4: 2590459db33 CI: remove dead "tree skipping" code
5: b2e456f9a9b ! 5: e311ea8b9b3 CI: remove unused Azure ci/* code
@@ ci/print-test-failures.sh: do
- azure-pipelines)
- mkdir -p failed-test-artifacts
- mv "$trash_dir" failed-test-artifacts
-- continue
- ;;
github-actions)
mkdir -p failed-test-artifacts
6: 30968e36bdd = 6: 151f28e3818 CI/lib.sh: stop adding leading whitespace to $MAKEFLAGS
7: cc81b9fe37e = 7: 6c2a8ee9c4e CI: don't have "git grep" invoke a pager in tree content check
8: 2387b0c5842 = 8: 64a0a1fc8ce CI: have "static-analysis" run a "make ci-static-analysis" target
9: 54a4d79bf8d = 9: 0f780bf9e3c CI: have "static-analysis" run "check-builtins", not "documentation"
10: 9a31b7d5011 = 10: 3cdeae9c141 CI: move p4 and git-lfs variables to ci/install-dependencies.sh
11: 8ab4e81e1ca = 11: 1d3d357dcef CI: consistently use "export" in ci/lib.sh
12: 9fe00566767 = 12: 5ef0756d095 CI: export variables via a wrapper
13: 13feda050c0 ! 13: e0ce614eb6d CI: remove "run-build-and-tests.sh", run "make [test]" directly
@@ Commit message
Now that verbosity is in the earlier "ci/lib.sh" step, and not in
any subsequent one. The "make" targets then start out with the
- relevant output non-trace output right away.
+ relevant non-trace output right away.
* If we do want to use the grouping syntax within a "step" it'll now
be easier to do so. It doesn't support nesting, so we'd have to
14: d3909b8e896 = 14: a8e70124929 ci/lib.sh: use "test" instead of "["
15: d63a689eae0 = 15: bb335d1c0ca CI: check ignored unignored build artifacts in "win[+VS] build" too
16: ac1ffe9c642 = 16: 5458e6dab0b CI: invoke "make artifacts-tar" directly in windows-build
17: 8837bfa5433 ! 17: 5e5a5b71700 CI: split up and reduce "ci/test-documentation.sh"
@@ Commit message
parts in as one command in the CI job itself, and to run the two "make
doc" commands at the top-level.
- It'll now be obvious from the title of the step if if we failed in the
+ It'll now be obvious from the title of the step if we failed in the
asciidoc or asciidoctor step.
Since the "check_unignored_build_artifacts()" function is now only
-: ----------- > 18: eec7a11376e CI: make ci/install-dependencies.sh POSIX-compatible
18: aa491990c1a ! 19: a9c98582de1 CI: combine ci/install{,-docker}-dependencies.sh
@@ Commit message
statement in the latter only cared about "$jobname", and can be folded
into the same "case" statement in the former.
- The reason they split up is historical, and because the
- "ci/install-dependencies.sh" used "ci/lib.sh", which requires
- "bash". At least one of the docker containers doesn't have "bash". To
- make the existing code POSIX-compatible we need to replace pushd/popd
- with a sub-shell, but no other changes were needed.
+ The reason they split up is historical, until a preceding commit
+ "ci/lib.sh" required "bash", which might not have been available in
+ "docker".
This also fixes issue in "ci/install-docker-dependencies.sh" where
we'd hide errors due to not using "set -e". Now that we include
@@ .github/workflows/main.yml: jobs:
- run: make test
## ci/install-dependencies.sh ##
-@@
--#!/usr/bin/env bash
-+#!/bin/sh
- #
- # Install dependencies required to build and test Git on Linux and macOS
- #
-@@ ci/install-dependencies.sh: ubuntu-latest)
- sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
- $UBUNTU_COMMON_PKGS $CC_PACKAGE
- mkdir --parents "$P4_PATH"
-- pushd "$P4_PATH"
-+ (
-+ cd "$P4_PATH"
- wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
- wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
- chmod u+x p4d
- chmod u+x p4
-- popd
-+ )
- mkdir --parents "$GIT_LFS_PATH"
-- pushd "$GIT_LFS_PATH"
-+ (
-+ cd "$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 .
-- popd
-+ )
- ;;
- macos-latest)
- export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
@@ ci/install-dependencies.sh: linux-gcc-default)
sudo apt-get -q update
sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
19: 39b13298425 = 20: 96e7eb68906 CI: move "env" definitions into ci/lib.sh
20: 0a92c478d28 = 21: fdbe8554f5d ci/run-test-slice.sh: replace shelling out with "echo"
21: 16ac7f5c210 = 22: f257ab59971 CI: pre-select test slice in Windows & VS tests
22: 939ef573b75 = 23: 1d95088da29 CI: only invoke ci/lib.sh as "steps" in main.yml
23: 8e79c3e39e0 = 24: a7434b215db CI: narrow down variable definitions in --build and --test
24: d1d5c1e2f65 = 25: 78f4609a9c1 CI: add more variables to MAKEFLAGS, except under vs-build
25: 1268792233f ! 26: 26a34f1d4b9 CI: set CC in MAKEFLAGS directly, don't add it to the environment
@@ Metadata
## Commit message ##
CI: set CC in MAKEFLAGS directly, don't add it to the environment
- Rather than pass a "$CC" and "$CC_PACKAGE" in the environment to be
- picked up in ci/lib.sh let's instead have ci/lib.sh itself add it
- directly to MAKEFLAGS.
+ Rather than pass a "$CC" in the environment to be picked up in
+ ci/lib.sh let's instead have ci/lib.sh itself add it directly to
+ MAKEFLAGS. For "$CC_PACKAGE" its setting and use can stay within
+ ci/install-dependencies.sh.
Setting CC=gcc by default made for confusing trace output, and since a
preceding change to carry it and others over across "steps" in the
GitHub CI it's been even more misleading. E.g. the "win+VS build" job
confusingly has CC=gcc set, even though it builds with MSVC.
- Let's instead reply on the Makefile default of CC=cc, and only
- override it for those jobs where it's needed. This does mean that
- we'll need to set it for the "pedantic" job, which previously relied
- on the default CC=gcc in case "clang" become the default on that
- platform.
+ Let's instead rely on the Makefile default of CC=cc, and only override
+ it for those jobs where it's needed. This does mean that we'll need to
+ set it for the "pedantic" job, which previously relied on the default
+ CC=gcc in case "clang" become the default on that platform.
This partially reverts my 707d2f2fe86 (CI: use "$runs_on_pool", not
"$jobname" to select packages & config, 2021-11-23), i.e. we're now
@@ .github/workflows/main.yml: jobs:
runs_on_pool: ${{matrix.vector.pool}}
runs-on: ${{matrix.vector.pool}}
+ ## ci/install-dependencies.sh ##
+@@ ci/install-dependencies.sh: UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
+ tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
+ libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
+
++CC_PACKAGE=
++case "$jobname" in
++linux-gcc | linux-TEST-vars)
++ CC_PACKAGE=gcc-8
++ ;;
++osx-gcc)
++ CC_PACKAGE=gcc-9
++ ;;
++esac
++
+ case "$runs_on_pool" in
+ ubuntu-latest)
+ # The Linux build installs the defined dependency versions below.
+
## ci/lib.sh ##
@@ ci/lib.sh: setenv () {
fi
@@ ci/lib.sh: setenv () {
+# Clear variables that may come from the outside world.
+CC=
-+CC_PACKAGE=
+
# How many jobs to run in parallel?
NPROC=10
@@ ci/lib.sh: vs-test)
;;
linux-gcc)
+ CC=gcc
-+ CC_PACKAGE=gcc-8
setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME main
;;
+linux-gcc-default)
@@ ci/lib.sh: vs-test)
+ ;;
linux-TEST-vars)
+ CC=gcc
-+ CC_PACKAGE=gcc-8
setenv --test GIT_TEST_SPLIT_INDEX yes
setenv --test GIT_TEST_MERGE_ALGORITHM recursive
setenv --test GIT_TEST_FULL_IN_PACK_ARRAY true
@@ ci/lib.sh: linux-TEST-vars)
;;
+osx-gcc)
+ CC=gcc
-+ CC_PACKAGE=gcc-9
+ ;;
+osx-clang)
+ CC=clang
26: 83138dacd3e = 27: 514de8d16b0 CI: set SANITIZE=leak in MAKEFLAGS directly
27: 0508777c68e ! 28: 6501059c594 CI: set PYTHON_PATH setting for osx-{clang,gcc} into "$jobname" case
@@ ci/lib.sh: linux-TEST-vars)
osx-gcc)
+ MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
CC=gcc
- CC_PACKAGE=gcc-9
;;
osx-clang)
+ MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
28: c0ce0fd3a80 = 29: 1bdcf1399d9 CI: don't use "set -x" in "ci/lib.sh" output
-: ----------- > 30: 94abb826627 CI: have osx-gcc use gcc, not clang
29: 2e3c02fa0df ! 31: 4fc67e668da CI: make it easy to use ci/*.sh outside of CI
@@ Commit message
environment if we're outside of CI, in case the user has e.g. "jN" or
other flags to "make" that they'd prefer configured already.
+ Using "ci/lib.sh" as a stand-alone script is much more useful if it
+ doesn't hardcode NPROC=10, let's provide a poor shellscript
+ replacement for the online_cpus() we have in thread-utils.c to cover
+ the most common OS's. It was suggested to use "2>&1" to invoke
+ "command -v", but per my reading of [2] and my own testing that
+ doesn't seem to be needed. Perhaps it's only needed for "which(1)"?
+
+ 1. https://lore.kernel.org/git/214f8670-91d5-f4b6-efa1-76966c3ab1ee@sunshineco.com/
+ 2. https://pubs.opengroup.org/onlinepubs/009604499/utilities/command.html
+
+ Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## ci/lib-ci-type.sh ##
@@ ci/lib-ci-type.sh
- exit 1
fi
+ ## ci/lib-online_cpus.sh (new) ##
+@@
++#!/bin/sh
++
++# TODO: Ideally we'd compile t/helper/test-online-cpus.c early, but
++# that presents a chicken & egg problem. But if we move it to a
++# stand-oline command...
++online_cpus() {
++ NPROC=
++
++ if command -v nproc >/dev/null
++ then
++ # GNU coreutils
++ NPROC=$(nproc)
++ elif command -v sysctl >/dev/null
++ then
++ # BSD & Mac OS X
++ NPROC=$(sysctl -n hw.ncpu)
++ elif test -n "$NUMBER_OF_PROCESSORS"
++ then
++ # Windows
++ NPROC="$NUMBER_OF_PROCESSORS"
++ else
++ NPROC=1
++ fi
++
++ echo $NPROC
++}
+
## ci/lib.sh ##
@@
#!/bin/sh
@@ ci/lib.sh: setenv () {
fi
echo "SET: '$key=$val'" >&2
-@@ ci/lib.sh: CC_PACKAGE=
+@@ ci/lib.sh: CC=
# How many jobs to run in parallel?
NPROC=10
+case "$CI_TYPE" in
+'')
-+ if command -v nproc >/dev/null
-+ then
-+ NPROC=$(nproc)
-+ else
-+ NPROC=1
-+ fi
++ . ${0%/*}/lib-online_cpus.sh
++ NPROC=$(online_cpus)
+
+ if test -n "$MAKEFLAGS"
+ then
@@ ci/lib.sh: github-actions)
## ci/print-test-failures.sh ##
@@ ci/print-test-failures.sh: do
- test_name="${TEST_EXIT%.exit}"
- test_name="${test_name##*/}"
- trash_dir="trash directory.$test_name"
-- case "$CI_TYPE" in
-- github-actions)
-+ if test "$CI_TYPE" = "github-actions"
-+ then
- mkdir -p failed-test-artifacts
- echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
- continue
-- ;;
+ ;;
- *)
- echo "Unhandled CI type: $CI_TYPE" >&2
- exit 1
- ;;
-- esac
-+ fi
- trash_tgz_b64="trash.$test_name.base64"
- if [ -d "$trash_dir" ]
- then
+ esac
+ fi
+ done-- 2.36.0.rc2.898.gdf6bbac50ff