Re: [PATCH v4 5/8] ci: unify setup of some environment variables

[Victoria Dye <vdye@github.com>]

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)

> +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"