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