Re: [PATCH 1/6] ci: remove GETTEXT_POISON jobs

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Wed, Feb 03, 2021 at 01:13:32PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Feb 01 2021, SZEDER Gábor wrote:
> 
> > On Wed, Jan 27, 2021 at 01:47:27AM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> 
> >> On Wed, Jan 20 2021, SZEDER Gábor wrote:
> >> 
> >> > On Wed, Jan 20, 2021 at 06:59:14PM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> >> 
> >> >> On Tue, Jan 12 2021, SZEDER Gábor wrote:
> >> >> 
> >> >> > On Mon, Jan 11, 2021 at 03:47:35PM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> >> >> A subsequent commit will remove GETTEXT_POISON entirely, let's start
> >> >> >> by removing the CI jobs that enable the option.
> >> >> >> 
> >> >> >> We cannot just remove the job because the CI is implicitly depending
> >> >> >> on the "poison" job being a sort of "default" job.
> >> >> >
> >> >> > I don't understand what you mean here with a "default job" that the CI
> >> >> > is implicitly depending on.  There is certainly no such default job on
> >> >> > Travis CI, and I don't think there is one on the GitHub thing.
> >> >> 
> >> >> I'll reword this. I meant that the poison job was using the default
> >> >> compiler etc., whereas the other ones were setting custom values.
> >> >> 
> >> >> I vaguely remember some list traffic about this in the past, i.e. the
> >> >> reliance on the poison job not just for that, but also as "the default
> >> >> OS image" setup.
> >> >
> >> > Oh, I didn't mean that the commit message should be clarified.  I
> >> > meant that the GETTEXT_POISON job can simply be deleted.  Sorry for
> >> > not being clear enough.
> >> 
> >> I still don't get it, and my patch still seems fine as it is to me. But
> >> I'm probably missing something.
> >> 
> >> I'm looking at these, both pointing at e6362826a04 (The fourth batch,
> >> 2021-01-25):
> >> 
> >>     # Currently says "Please be aware travis-ci.org will be shutting
> >>     # down in several weeks"
> >
> > ... and continues with:
> >
> >   "with all accounts migrating to travis-ci.com.  Please stay tuned
> >    here for more information."
> >
> > FWIW, I recently migrated manually, and apart from the different TLD
> > haven't noticed anything different (though somehow the jobs in my .org
> > builds sometimes took a while to start lately, but on .com they all
> > started real fast so far, as they should).
> >
> >>     https://www.travis-ci.org/github/git/git/builds/756176098
> >> 
> >>     https://github.com/git/git/actions/runs/510676142
> >> 
> >> For the GitHub one, there's only two things that are linux && gcc. The
> >> linux-gcc job, and GETTEXT_POISON (renamed to linux-gcc-default in this
> >> series).
> >> 
> >> In ci/install-dependencies.sh we install GCC 8 for linux-gcc, but also
> >> perforce, apache, git-lfs etc.
> >> 
> >> Then in ci/lib.sh we set CC=gcc-8 on linux-gcc, but retain the default
> >> on GETTEXT_POISON. We also set GIT_TEST_HTTPD=true.
> >> 
> >> So hence "linux-gcc-default", we're running as close to a default set of
> >> packages we can get and build/run git at all. I.e. just "apt install
> >> $UBUNTU_COMMON_PKGS".
> >
> > We started installing newer compilers, because we wanted to benefit
> > from their better error reporting and improved warnings.  The reason
> > we did that in existing CI jobs without adding a "default" job was
> > that we don't care enough about the default compiler and configuration
> > to justify its additional runtime, because we run the test suite
> > enough times already.
> 
> So in summary, you'd like it removed because the combination of
> GETTEXT_POISON + old compiler is no longer worth it, but while we were
> running with GETTEXT_POISON anyway we might as well run it on the old
> compiler?

Yes, though I suspect that "no one gave a damn about the compiler used
in the GETTEXT_POISON job" might be closer to the truth.  Or even "no
one gave a damn about the GETTEXT_POISON job"...

And apart from the occasional translation-related failure (i.e. 'grep
"<translated message>" actual') and one flaky test that happened to
fail in the GETTEXT_POISON job I can't recall a single case where
building and testing with the default compiler discovered an issue
that the other jobs didn't.

> > Considering that the number of CI jobs has grown since then, I would
> > rather see the build complete faster than a default job running the
> > test suite for the N+1th time.
> 
> [...]
> 
> >> Then in ci/run-build-and-tests.sh the "linux-gcc" job is also our "let's
> >> turn on every GIT_TEST_* option you've heard of" job. Whereas on the
> >> GETTEXT_POISON job we just ran "make test", well, with
> >> GIT_TEST_GETTEXT_POISON=true before this series, but that was the only
> >> non-default, now we've got no non-defaults.
> >> 
> >> I can see why we'd want this on top:
> >>     
> >>     diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> >>     index 50e0b90073f..3071099eaca 100755
> >>     --- a/ci/run-build-and-tests.sh
> >>     +++ b/ci/run-build-and-tests.sh
> >>     @@ -32,11 +32,6 @@ linux-clang)
> >>             export GIT_TEST_DEFAULT_HASH=sha256
> >>             make test
> >>             ;;
> >>     -linux-gcc-4.8)
> >>     -       # Don't run the tests; we only care about whether Git can be
> >>     -       # built with GCC 4.8, as it errors out on some undesired (C99)
> >>     -       # constructs that newer compilers seem to quietly accept.
> >>     -       ;;
> >>      *)
> >>             make test
> >>             ;;
> >> 
> >> Because there we're skipping tests on linux-gcc-4.8, but on travis it's
> >> the only Ubuntu Trusty image (default is Xenial), so yes we build with
> >> gcc 4.8, but we're also missing out in seeing if our tests are working
> >> on a slightly older Ubuntu.
> >
> > The above comment you propose to remove explains why you should not
> > remove it...  And, again, running the test suite doesn't seem to worth
> > the extra runtime.
> 
> How is it extra runtime in any meaningful sense?
> 
> Yes, we spend more CPU time, but as far as I can tell (and I'm looking
> at an ongoing run on GitHub now)

(Well, I'm looking at eight ongoing builds now...)

> these jobs are run concurrently. Some
> of that was covered in the quoted paragraph below that you didn't reply
> to.
> 
> I.e. if I push a commit I don't care if 10 minutes of CPU time are being
> wasted by a machine sitting in some rack somewhere, the important part
> is that I may be saving 20 minutes of my time, since the faster 10
> minute test (running concurrently with the ~30m tests) might fail
> earlier.
> 
> Or are there some job limits involved here that I'm missing?

Travis CI usually runs 3 Linux and 2 OSX jobs in parallel, and I doubt
that GitHub would be foolish enough to run all my jobs parallel if I
were to use it.

> As I was writing this the CI job for GETTEXT_POISON just finished in
> 10m16s showing OK=green, and the rest are still churning.

Indeed, but you should also consider that:

  - At 10m16s that job is notably slower than even my tiny old laptop,
    even though it runs less tests.  If you really want to save time,
    then run 'make test' locally.

  - Without the GETTEXT_POISON job the whole build would be finished
    ~10 minutes earlier.  With six builds that's an hour.


> >> Also on GitHub's page the GETTEXT_POISON job finishes in ~10m but all
> >> the other ones in ~30m, I see some run "make test" twice, as an aside is
> >> there some limit on jobs but not overall CPU? Otherwise those would
> >> finish faster if that was split up.
> >> 
> >> But "make test" twice doesn't account for a ~3x increase, so it's got to
> >> be the extra tests we're running, i.e. svn tests, httpd etc.
> >> 
> >> It seems to me to provide value in itself to run some shorter version of
> >> the CI for people using it as a poor man's edit/push/ci/edit loop.
>