Re: [PATCH v4 5/8] ci: unify setup of some environment variables
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Nov 1, 2023, 11:44 UTC
- Message-ID
- <ZUI6I8_r7Lff0ruj@tanuki>
- In-Reply-To
- <31ebe4c9-84aa-4d42-9aeb-712e2a6cece3@github.com>
On Tue, Oct 31, 2023 at 10:06:24AM -0700, Victoria Dye wrote:
Show 79 quoted lines
> Patrick Steinhardt wrote: > > Both GitHub Actions and Azue Pipelines set up the environment variables > > s/Azue/Azure > > > GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are > > actually the same, the setup is completely duplicate. With the upcoming > > support for GitLab CI this duplication would only extend even further. > > > > Unify the setup of those environment variables so that only the uncommon > > parts are separated. While at it, we also perform some additional small > > improvements: > > > > - We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS. > > It doesn't hurt on platforms where we don't persist the state, so > > this further reduces boilerplate. > > > > - When running on Windows systems we set `--no-chain-lint` and > > `--no-bin-wrappers`. Interestingly though, we did so _after_ > > already having exported the respective environment variables. > > > > - We stop using `export VAR=value` syntax, which is a Bashism. It's > > not quite worth it as we still use this syntax all over the place, > > but it doesn't hurt readability either. > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > --- > > ci/lib.sh | 24 ++++++++++++++---------- > > 1 file changed, 14 insertions(+), 10 deletions(-) > > > > diff --git a/ci/lib.sh b/ci/lib.sh > > index 9ffdf743903..9a9b92c05b3 100755 > > --- a/ci/lib.sh > > +++ b/ci/lib.sh > > @@ -175,11 +175,7 @@ then > > # among *all* phases) > > cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME" > > > > - export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save" > > - export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml" > > - MAKEFLAGS="$MAKEFLAGS --jobs=10" > > - test windows_nt != "$CI_OS_NAME" || > > - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" > > + GIT_TEST_OPTS="--write-junit-xml" > > elif test true = "$GITHUB_ACTIONS" > > then > > CI_TYPE=github-actions > > @@ -198,17 +194,25 @@ then > > > > cache_dir="$HOME/none" > > > > - export GIT_PROVE_OPTS="--timer --jobs 10" > > - export GIT_TEST_OPTS="--verbose-log -x --github-workflow-markup" > > - MAKEFLAGS="$MAKEFLAGS --jobs=10" > > - test windows != "$CI_OS_NAME" || > > - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS" > > + GIT_TEST_OPTS="--github-workflow-markup" > > else > > echo "Could not identify CI type" >&2 > > env >&2 > > exit 1 > > fi > > > > +MAKEFLAGS="$MAKEFLAGS --jobs=10" > > +GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save" > > + > > +GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x" > > +if test windows = "$CI_OS_NAME" > > Based on the deleted lines above, I think this would need to be: > > if test windows = "$CI_OS_NAME" || test windows_nt = "$CI_OS_NAME" > > I believe these settings are required on all Windows builds, though, so you could > instead match on the first 7 characters of $CI_OS_NAME: > > if test windows = "$(echo "$CI_OS_NAME" | cut -c1-7)" > > (full disclosure: I'm not 100% confident in the correctness of that shell syntax)
Oh, right. I didn't notice the slight difference between "windows" and "windows_nt". Thanks!
Patrick
Show 11 quoted lines
> > +then > > + GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers" > > +fi > > + > > +export GIT_TEST_OPTS > > +export GIT_PROVE_OPTS > > + > > good_trees_file="$cache_dir/good-trees" > > > > mkdir -p "$cache_dir" >