Skip to content

Commit 2e01908

Browse files
avargitster
avar
authored and
gitster
committed
CI: remove "run-build-and-tests.sh", run "make [test]" directly
Remove the already thin "ci/run-build-and-tests.sh" wrapper and instead make the CI run "make" or "make test" directly. By doing this we'll be able to easily see at a glance whether our failure was in the compilation or testing, whether that's via human eyes or improve machine readability. We also need to run our new "ci/check-unignored-build-artifacts.sh" on success() in the CI now, just like we already had a step conditional on failure() running ci/print-test-failures.sh. The reason we used a "ci/run-build-and-tests.sh" wrapper in the first place had to do with Travis CI-specific constraints that no longer apply to us, as the Travis CI support has been removed. Instead we can configure the CI in an earlier step by running "ci/lib.sh", which under GitHub CI will write the environment variables we need to the "$GITHUB_ENV" file. We'll then have access to them in subsequent steps, and crucially those variables will be prominently visible at the start of each step via an expandable drop-down in the UI.drop-do. I.e. this changes the CI run from a top-down flow like (pseudocode): - job: - step1: - use ci/lib.sh to set env vars - run a script like ci/run-build-and-tests.sh - step2: - if: failure() - use ci/lib.sh to set env vars - run ci/print-test-failures.sh To: - job: - step1: - set variables in $GITHUB_ENV using ci/lib.sh - step2: - make - step3: - make test - step4: - if: failure() - run ci/print-test-failures.sh - step5: - if: success() - run ci/check-unignored-build-artifacts.sh There is a proposal[2] to get some of the benefits of this approach by not re-arranging our variable setup in this way, but to instead use the GitHub CI grouping syntax to focus on the relevant parts of "make" or "make test" when we have failures. Doing it this way makes for better looking GitHub CI UI, and lays much better ground work for our CI going forward. Because: * The CI logic will be more portable to a future CI system, since a common feature of them is to run various commands in sequence, but a future system won't necessarily support the GitHub-specifics syntax of "grouping" output within a "step". Even if those systems don't support a "$GITHUB_ENV" emulating will be much easier than to deal with some CI-specific grouping syntax. * At the start of every step the GitHub CI presents an expandable list of environment variables from "$GITHUB_ENV". We'll now see exactly what variables affected that step (although we currently overshoot that a bit, and always define all variables). * CI failures will be easier to reproduce locally, as this makes the relevant ci/* scripts something that sets up our environment, but leaves "make" and "make test" working as they do locally. To reproduce a run the user only needs to set the variables discussed in the drop-down above, either manually or by running "ci/lib.sh". * The output will be less verbose. The "ci/lib.sh" script uses "set -x", and before this e.g. "ci/static-analysis.sh" would start with 40 lines of trace output, culminating in using "export" to export the relevant environment variables. 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. * 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 make a choice between using it for e.g. "make" v.s. "make test", or individual test failures. See "sadly" in [3]. 1. https://lore.kernel.org/git/211120.86k0h30zuw.gmgdl@evledraar.gmail.com/ 2. https://lore.kernel.org/git/pull.1117.git.1643050574.gitgitgadget@gmail.com/ 3. https://lore.kernel.org/git/9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 04c9ad6 commit 2e01908

File tree

8 files changed

+62
-56
lines changed

8 files changed

+62
-56
lines changed

‎.github/workflows/main.yml‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,10 @@ jobs:
268268
steps:
269269
- uses: actions/checkout@v2
270270
- run: ci/install-dependencies.sh
271-
- run: ci/run-build-and-tests.sh
271+
- run: ci/lib.sh
272+
- run: make
273+
- run: make test
274+
if: success()
272275
- run: ci/print-test-failures.sh
273276
if: failure()
274277
- name: Upload failed tests' directories
@@ -292,16 +295,20 @@ jobs:
292295
image: daald/ubuntu32:xenial
293296
- jobname: pedantic
294297
image: fedora
298+
skip-tests: no
295299
env:
296300
jobname: ${{matrix.vector.jobname}}
297301
runs-on: ubuntu-latest
298302
container: ${{matrix.vector.image}}
299303
steps:
300304
- uses: actions/checkout@v1
301305
- run: ci/install-docker-dependencies.sh
302-
- run: ci/run-build-and-tests.sh
306+
- run: ci/lib.sh
307+
- run: make
308+
- run: make test
309+
if: success() && matrix.vector.skip-tests != 'no'
303310
- run: ci/print-test-failures.sh
304-
if: failure()
311+
if: failure() && matrix.vector.skip-tests != 'no'
305312
- name: Upload failed tests' directories
306313
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
307314
uses: actions/upload-artifact@v1
@@ -317,6 +324,7 @@ jobs:
317324
steps:
318325
- uses: actions/checkout@v2
319326
- run: ci/install-dependencies.sh
327+
- run: ci/lib.sh
320328
- run: make ci-static-analysis
321329
sparse:
322330
needs: ci-config

‎ci/check-unignored-build-artifacts.sh‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/bin/sh
2+
#
3+
# Check whether the build created anything not in our .gitignore
4+
#
5+
6+
. ${0%/*}/lib.sh
7+
8+
check_unignored_build_artifacts

‎ci/install-dependencies.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ ubuntu-latest)
2222
P4_PATH="$HOME/custom/p4"
2323
GIT_LFS_PATH="$HOME/custom/git-lfs"
2424
export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
25+
if test -n "$GITHUB_PATH"
26+
then
27+
echo "$PATH" >>"$GITHUB_PATH"
28+
fi
2529

2630
P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION
2731
LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION

‎ci/lib.sh‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#!/bin/sh
2+
13
# Library of functions shared by all CI scripts
24

35
# Set 'exit on error' for all CI scripts to let the caller know that
@@ -27,7 +29,14 @@ setenv () {
2729
val=$2
2830
shift 2
2931

30-
eval "export $key=\"$val\""
32+
if test -n "$GITHUB_ENV"
33+
then
34+
echo "$key=$val" >>"$GITHUB_ENV"
35+
else
36+
# For local debugging. Not used by the GitHub CI
37+
# itself.
38+
eval "export $key=\"$val\""
39+
fi
3140
}
3241

3342
check_unignored_build_artifacts ()
@@ -96,6 +105,35 @@ macos-latest)
96105
esac
97106

98107
case "$jobname" in
108+
linux-gcc)
109+
setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME main
110+
;;
111+
linux-TEST-vars)
112+
setenv --test GIT_TEST_SPLIT_INDEX yes
113+
setenv --test GIT_TEST_MERGE_ALGORITHM recursive
114+
setenv --test GIT_TEST_FULL_IN_PACK_ARRAY true
115+
setenv --test GIT_TEST_OE_SIZE 10
116+
setenv --test GIT_TEST_OE_DELTA_SIZE 5
117+
setenv --test GIT_TEST_COMMIT_GRAPH 1
118+
setenv --test GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS 1
119+
setenv --test GIT_TEST_MULTI_PACK_INDEX 1
120+
setenv --test GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP 1
121+
setenv --test GIT_TEST_ADD_I_USE_BUILTIN 1
122+
setenv --test GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME master
123+
setenv --test GIT_TEST_WRITE_REV_INDEX 1
124+
setenv --test GIT_TEST_CHECKOUT_WORKERS 2
125+
;;
126+
linux-clang)
127+
setenv --test GIT_TEST_DEFAULT_HASH sha1
128+
;;
129+
linux-sha256)
130+
setenv --test GIT_TEST_DEFAULT_HASH sha256
131+
;;
132+
pedantic)
133+
# Don't run the tests; we only care about whether Git can be
134+
# built.
135+
setenv --build DEVOPTS pedantic
136+
;;
99137
linux32)
100138
CC=gcc
101139
;;

‎ci/make-test-artifacts.sh‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,3 @@ mkdir -p "$1" # in case ci/lib.sh decides to quit early
88
. ${0%/*}/lib.sh
99

1010
make artifacts-tar ARTIFACTS_DIRECTORY="$1"
11-
12-
check_unignored_build_artifacts

‎ci/run-build-and-tests.sh‎

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎ci/run-test-slice.sh‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,3 @@
88
make --quiet -C t T="$(cd t &&
99
./helper/test-tool path-utils slice-tests "$1" "$2" t[0-9]*.sh |
1010
tr '\n' ' ')"
11-
12-
check_unignored_build_artifacts

‎ci/test-documentation.sh‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,3 @@ test -s Documentation/git.html
3939
grep '<meta name="generator" content="Asciidoctor ' Documentation/git.html
4040

4141
rm -f stdout.log stderr.log stderr.raw
42-
check_unignored_build_artifacts

0 commit comments

Comments
 (0)