Re: [PATCH] ci: only run win+VS build & tests in Git for Windows' fork

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Dec 19 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> [...]
>     Explicitly make CMake-based CI failures the responsibility of "Windows
>     folks"
>     
>     Ævar and I have brain-stormed off-list what would be the best way to
>     resolve the mounting frustration with CI failures that are caused by
>     needing to mirror Makefile changes into the CMake-based build, a burden
>     that the Git project never wanted to bear.
>     
>     While he still wants to improve the CMake support (which will benefit
>     Git for Windows), the main driver of trying to extend CMake support to
>     Linux (which does not need it because make works very well there,
>     indeed) was said frustration with CI failures.
>     
>     A much quicker method to reduce that friction to close to nil is to
>     simply disable the win+VS jobs, which is what this proposal is about.
>     (Almost, at least, we still want to keep those job definitions and run
>     them in Git for Windows' fork to ensure that CMake-based builds still
>     work.)
>     
>     A very welcome side effect is to reduce the CI build time again, which
>     became alarmingly long as of recent, causing friction on its own.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1445%2Fdscho%2Fonly-run-win%2BVS-in-the-git-for-windows-fork-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1445/dscho/only-run-win+VS-in-the-git-for-windows-fork-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1445
>
>  .github/workflows/main.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index e67847a682c..8af3c67f605 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -132,7 +132,7 @@ jobs:
>    vs-build:
>      name: win+VS build
>      needs: ci-config
> -    if: needs.ci-config.outputs.enabled == 'yes'
> +    if: github.event.repository.owner.login == 'git-for-windows' && needs.ci-config.outputs.enabled == 'yes'
>      env:
>        NO_PERL: 1
>        GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"

In terms of implementation: I would like this to use ci-config.

Yes, it will take us some CI time just to spin up the image to decide on
that (only a few seconds), but it's preferrable to hardcoding
github.com/git-for-windows, as you'll be able to opt-in to this to test
changes.

The outstanding "tb/ci-concurrency" topic shows how to do that (and
tweaks an earlier submission of yours where it was similarly hardcoded).

> The purpose of these win+VS jobs is to maintain the CMake-based build
> of Git, with the target audience being Visual Studio users on Windows
> who are typically quite unfamiliar with `make` and POSIX shell

I thought the initial purpose of it was to test compiling & testing with
MSVC rather than GCC?

It's only after 4c2c38e800f (ci: modification of main.yml to use cmake
for vs-build job, 2020-06-26) that it got co-opted to test cmake too.

Whatever we do about the job at this point not discussing 4c2c38e800f in
the commit seems like a big omission, why not revert & adjust it if the
cmake part of it is a perceived hassle?

Now, I don't think that it's worth spending time on running two sets of
Windows tests all the time just to tease out GCC v.s. MSVC differences,
but I think we should still run the build on both. That takes around
~5m, and seems worth it to avoid and flag any portability issues.

Before this we test on gcc/clang/msvc, after this we'll have dropped
that last one. Whatever we do about the tests and/or cmake I think we
really should avoid compiler monoculture, and fix portability issues
early.

> A very welcome side effect is to reduce the CI build time again, which
> became alarmingly long as of recent, causing friction on its own.

I think this is a good goal, but what does the "as of recent" refer to
here? When the ASAN job was introduced?