Re: [PATCH 2/2] ci: also run the `scalar` tests

[Derrick Stolee <derrickstolee@github.com>]

On 6/2/2022 5:05 AM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Since Scalar depends on `libgit.a`, it makes sense to ensure in the CI
> and the PR builds that it does not get broken in case of industrious
> refactorings of the core Git code (speaking from experience here).
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  .github/workflows/main.yml | 15 +++++++++++++++
>  ci/run-build-and-tests.sh  |  2 ++
>  ci/run-test-slice.sh       |  5 +++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index c35200defb9..785222aa7b3 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -91,6 +91,13 @@ jobs:
>          HOME: ${{runner.workspace}}
>          NO_PERL: 1
>        run: . /etc/profile && ci/make-test-artifacts.sh artifacts
> +    - name: build Scalar
> +      shell: bash
> +      run: |
> +        make -C contrib/scalar &&
> +        mkdir -p artifacts/bin-wrappers artifacts/contrib/scalar &&
> +        cp contrib/scalar/scalar.exe artifacts/contrib/scalar/ &&
> +        cp bin-wrappers/scalar artifacts/bin-wrappers/

I see later you have a "copy Scalar" step which has some duplication
here. The only difference is that you have "make -C contrib/scalar".

Doesn't Scalar get built in our basic "make" build when the
environment includes INCLUDE_SCALAR=YesPlease? So, for that reason I
expected the environment to change, but not need this "make -C ..."

>      - name: zip up tracked files
>        run: git archive -o artifacts/tracked.tar.gz HEAD
>      - name: upload tracked files and build artifacts
> @@ -161,6 +168,8 @@ jobs:
>        run: compat\vcbuild\vcpkg_copy_dlls.bat release
>      - name: generate Visual Studio solution
>        shell: bash
> +      env:
> +        INCLUDE_SCALAR: YesPlease

This is a bit isolated. Is there a way to specify the environment
more generally?

>        run: |
>          cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
>          -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> @@ -174,6 +183,12 @@ jobs:
>        run: |
>          mkdir -p artifacts &&
>          eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)"
> +    - name: copy Scalar
> +      shell: bash
> +      run: |
> +        mkdir -p artifacts/bin-wrappers artifacts/contrib/scalar &&
> +        cp contrib/scalar/scalar.exe artifacts/contrib/scalar/ &&
> +        cp bin-wrappers/scalar artifacts/bin-wrappers/
>      - name: zip up tracked files
>        run: git archive -o artifacts/tracked.tar.gz HEAD
>      - name: upload tracked files and build artifacts
> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index 280dda7d285..661edb85d1b 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -51,4 +51,6 @@ esac
>  make $MAKE_TARGETS
>  check_unignored_build_artifacts
>  
> +make -C contrib/scalar $MAKE_TARGETS
> +

Again, this should "just work" when using INCLUDE_SCALAR in the
environment, right?

>  save_good_tree
> diff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh
> index f8c2c3106a2..b741fd8f361 100755
> --- a/ci/run-test-slice.sh
> +++ b/ci/run-test-slice.sh
> @@ -14,4 +14,9 @@ make --quiet -C t T="$(cd t &&
>  	./helper/test-tool path-utils slice-tests "$1" "$2" t[0-9]*.sh |
>  	tr '\n' ' ')"
>  
> +if test 0 = "$1"
> +then
> +	make -C contrib/scalar test
> +fi
> +

This is still necessary for now.

Thanks,
-Stolee