Re: [PATCH v3 13/29] CI: remove "run-build-and-tests.sh", run "make [test]" directly
On 4/13/22 3:51 PM, Ævar Arnfjörð Bjarmason wrote:
Show 86 quoted lines
> 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/
Show 10 quoted lines
> * 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>