Re: What's cooking in git.git (Jan 2020, #04; Wed, 22)

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Yang Zhao <yang.zhao@skyboxlabs.com>
Subject: Re: What's cooking in git.git (Jan 2020, #04; Wed, 22)
Date: Thu, 6 Feb 2020 09:57:51 +0100 (CET)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2002060930210.3718@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20200206002754.GM10482@szeder.dev>

[-- Attachment #1: Type: text/plain, Size: 7146 bytes --]

Hi Gábor,

On Thu, 6 Feb 2020, SZEDER Gábor wrote:

> On Wed, Feb 05, 2020 at 01:01:50PM -0800, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > >> Do they have to shout that loudly in the name?
> > >>
> > >> We could rename these jobs to e.g. 'linux-clang-py2' and the like, but
> > >> I think it would bring little benefit, if any.  In our Travis CI
> > >> builds these Linux/OSX Clang/GCC jobs come from the build matrix,
> > >> therefore the jobname is not visible on the Travis CI web interface or
> > >> API, only in the build logs.  There are some pages on Azure Pipelines
> > >> that do show the jobname (and some that could, but hide it instead),
> > >> but it's just too convoluted (or sometimes even impossible, well, for
> > >> me anyway) to get there.
> > >>
> > >> And if the requested Python binary can't be found, which will
> > >> eventually happen with 'python2', then the non-zero exit code of
> > >> 'which' will abort the build, no matter how the job is called.
> > >
> > > I am mostly worried about contributors whose PRs break for "magic"
> > > reasons. If it is not clear where the difference between `linux-gcc` and
> > > `linux-clang` lies, that can cause unintended frustration, and I do not
> > > want to cause that.
>
> I'm not worried about that.  If a contributor doesn't touch any of our
> Python scripts, then I don't see why using a different Python version
> in the build would cause any issues.  And if they do modify one of the
> Python scripts, then they should make sure that their modifications
> work both with Python 2 and 3 in the first place.

If the frequent problems with downloading the Perforce binariers taught me
anything, it is that the most likely explanation for failures in the
linux-gcc job is that Perforce, once again, updated their binaries,
uploaded them _to the exact same URL as before_, and that there is nothing
wrong in the PR or the patches.

That _is_ the most likely explanation, given our record.

So what are contributors supposed to do with that? Nothing in the name
`linux-gcc` cries out loud: Hey, this is a Homebrew problem, there is most
likely already a PR up to fix it, and the job needs to be re-run once that
PR is merged, that's all, please ignore for a day.

Now, with the log that we currently have, it is still somewhat easy to
figure out what is going wrong. Somewhere along the lines it spits out an
error that talks about some sort of package and about some sort of
checksum. Most developers deduce from that message that it's not their
fault and move on.

So this is _already_ an annoying problem, but it gets worse: every once in
a while, a build is "green" _except_ in linux-gcc. The contributor runs
the test locally, it passes, so they conclude that the test must be flaky
or at least that it is not their fault.

How on this dear planet should they know to run the test again, but this
time with `GIT_TEST_SPLIT_INDEX=yes GIT_TEST_FULL_IN_PACK_ARRAY=true
GIT_TEST_OE_SIZE=10 GIT_TEST_OE_DELTA_SIZE=5 GIT_TEST_COMMIT_GRAPH=1
GIT_TEST_MULTI_PACK_INDEX=1 GIT_TEST_ADD_I_USE_BUILTIN=1`?

You know that they should do that. I know that they should do that. They
don't. And why don't they know that? Because _we make it hard for them to
know that_.

I agree that your changes make sense, from a lot of points of view. Except
one. The one where a contributor has to spend an unnecessarily long time
to figure out how to proceed given a test failure in `linux-clang` and
`osx-clang` that they never saw in their development (e.g. because they
only run with a non-EOL-ed Python).

What we need, therefore, is a way to let the users know _precisely_ what
is going on and even more importantly _what they should do now_.

Simply tacking on the Python3 stuff onto -clang (or -gcc? I forgot which
one, see, it even confuses _me_) is not going to do that.

Granted, this is not at all a new problem, it is related to that "let's
pile another test run with all kinds of `GIT_TEST_*` knobs turned to the
non-default settings onto, well, let's see, how about `linux-gcc`?"
problem.

In this light, I kind of agree that it is not the responsibility of the
py2 vs py3 changes you proposed to fix this. But they make the problem
even worse.

Ideally, I would prefer a new job into which the second half of
`linux-gcc` is moved, just like I proposed many moons ago. This would also
help the problem where flaky tests require a re-run of that insanely
long-running job.

Of course, you might find a clever way to enhance the failed test's log such
that it makes it obvious that this was run with special options (similar in
spirit to ffe1afe67c0 (tests: show the test name and number at the start of
verbose output, 2019-08-05)), and then tacking on the py3 thing onto -clang
(or was it -gcc? I am _still_ confused about that) would still be okay.

But then, I would contend that we should do the same for
`GIT_TEST_SPLIT_INDEX=yes GIT_TEST_FULL_IN_PACK_ARRAY=true
GIT_TEST_OE_SIZE=10 GIT_TEST_OE_DELTA_SIZE=5 GIT_TEST_COMMIT_GRAPH=1
GIT_TEST_MULTI_PACK_INDEX=1 GIT_TEST_ADD_I_USE_BUILTIN=1`. Only use them in
`-clang` (or was it `-gcc`? I still cannot remember), and run the other with
default settings only.

> > So, what, if any, decision have we reached?
> >
> > If linux-gcc and linux-clang labels are not visible, linux-clang-py2
> > and osx-py3 would not be, either, so...
>
> The 'linux-gcc' and 'linux-clang' labels are not visible on Travis CI,
> because those jobs as part of the build matrix, and, consequently, we
> can't set the a 'jobname' environment variable for them in
> '.travis.yml'.  If we were to include additional jobs for the Python
> scripts, then for those we can (and should!) set
> 'jobname=linux-python' or something, and that would be visible on the
> Travis CI web interface, just like e.g. 'jobname=StaticAnalysis'.

I think we can see that jobname very well, though. If you direct your web
browser to
https://travis-ci.org/git/git/builds/646646192?utm_source=github_status&utm_medium=notification
you will see something like this:

    Build jobs		View config

! 5281.1 AMD64		Compiler: clang Xcode: xcode10.1 C	no environment variables set	8 min 20 sec
! 5281.2 AMD64		Compiler: gcc Xcode: xcode10.1 C	no environment variables set	8 min 23 sec
X 5281.3 AMD64		Compiler: clang Xcode: xcode10.1 C	no environment variables set	1 min 57 sec
X 5281.4 AMD64		Compiler: gcc Xcode: xcode10.1 C	no environment variables set	2 min 41 sec
! 5281.5 AMD64		Xcode: xcode10.1 C			jobname=GIT_TEST_GETTEXT_POISON	5 min 14 sec
X 5281.6 AMD64		Xcode: xcode10.1 C			jobname=linux-gcc-4.8		1 min 13 sec
! 5281.7 AMD64		Xcode: xcode10.1 C			jobname=Linux32			6 min 50 sec
✓ 5281.8 AMD64		Xcode: xcode10.1 C			jobname=StaticAnalysis		10 min 56 sec
✓ 5281.9 AMD64		Xcode: xcode10.1 C			jobname=Documentation		6 min 15 sec

Never mind that it is somewhat dubious to see that the Linux32 job is run with
Xcode... but you definitely see the jobname, loud and clear.

Ciao,
Dscho