On Mon, Dec 19 2022, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > [...] > 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?
