From: Eric Sunshine Date: Thu, 14 Apr 2022 06:46:09 GMT Subject: Re: [PATCH v3 13/29] CI: remove "run-build-and-tests.sh", run "make [test]" directly Message-ID: <407028e3-ea88-a396-2d00-d7bf5977ba20@sunshineco.com> In-Reply-To: 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@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