On 4/13/22 3:51 PM, Ævar Arnfjörð Bjarmason wrote:
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.
s/output non-trace output/non-trace output/
* 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@xxxxxxxxxxxxxxxxxxx/
2. https://lore.kernel.org/git/pull.1117.git.1643050574.gitgitgadget@xxxxxxxxx/
3. https://lore.kernel.org/git/9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@xxxxxxxxx/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
