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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ævar,

On Mon, 19 Dec 2022, Ævar Arnfjörð Bjarmason wrote:

> On Mon, Dec 19 2022, Johannes Schindelin via GitGitGadget wrote:
>
> > 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.

I really do not see any value in that.

If you _really_ want to run those tests, you can easily add a commit on
top of your branch.

It's not like you always want to run the win+VS tests on a certain subset
of your branches.

But. A big part of the reason for this patch is to codify that it
is _not_ the responsibility of core Git contributors to take care of these
tests. Trying to do this via the utterly convoluted and under-documented
`ci-config` would only water down this quite clear statement.

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

I fear that the `tb/ci-concurrency` topic is a good example of "death by
committee". Let's put in a collective effort to avoid that here.

> > 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?

That might have been the motivation, but there have been preciously few
patches coming in from that side.

So few, in fact, that the question "is it worth spending all of that
energy to run these builds and tests all the time" most likely has to be
answered with a resounding "No".

I can think of just two bugs that were identified in the MSVC builds, one
where we had to change some code to explicitly use a stable sort (which
MSVC would otherwise not have used), and one where we now avoid an
unsigned/signed comparison where MSVC cast the signed value to a
now-insanely-large unsigned, i.e. in the wrong direction.

That's two bugs identified in how many years?

We still can catch those bugs in git-for-windows/git. And avoid some of
the long build times.

> > 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?

It's not only `linux-asan`. The overall build time of every single job has
gone up. I picked randomly two runs that were roughly 6 months apart,
https://github.com/git/git/actions/runs/2537915353 and
https://github.com/git/git/actions/runs/3728047162. As an indicator, the
`osx-gcc` job took 35m half a year ago, now it takes 40m. The overall time
went up from 7h40 (which is already _quite_ a cost!) to 8h01.

I don't think that we, collectively, are judicious enough about the
balance between cost and benefit here.

Having said that, there is a silver lining: while the linux-asan job is
new and takes a whopping 44m to complete, the difference between the total
run time 6 months ago and today is less than that, so we must have saved
on _something_, _somewhere_. Or GitHub bought faster hardware.

Ciao,
Johannes

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux