Re: [PATCH 2/3] ci: avoid bare "gcc" for osx-gcc job

Patrick Steinhardt
2024-05-17T08:33:48Z

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

On Fri, May 17, 2024 at 04:19:09AM -0400, Jeff King wrote:
> On Thu, May 16, 2024 at 11:54:44AM +0200, Patrick Steinhardt wrote:
>
> > That's certainly the biggest part, yeah. But I have been hitting lots of
> > compiler-dependent behaviour. This is mostly in the area of bugs though,
> > where for example toolchain A may initialize variables on the stack to
> > all zeroes while toolchain B does not.
>
> I've definitely run into differing runtime outcomes for undefined
> behavior stuff like that. But in my experience most of that is
> consistently found by ASan/UBSan (which we do run in CI these days).
>
> It's possible there are cases that those sanitizers don't catch but that
> cause differing behavior. But I can't think of one off the top of my
> head where that has happened.

True, these should be sufficient indeed.

> > I'm not sure whether this is sufficient reason on its own to warrant
> > testing with several toolchains. But we can easily combine this with
> > additional tuning knobs. Two separate test jobs with GCC and Clang are
> > comparatively boring. But if we make it GCC+SHA1 and Clang+SHA256 then
> > it becomes more interesting.
>
> Yeah. Combining orthogonal properties into a single job lets us cover
> both (for the common case of success on both) with less CPU. But:
>
> - it can sometimes be hard to figure out which of the properties was
> responsible for a failure. That was the very subject of the thread I
> referenced earlier, where "linux-gcc" was "use gcc" and also "set
> lots of knobs".

That's true. But for me the problem typically is that you need to be
aware that the job uses different properties in the first place -- this
is quite hidden away. Figuring out that a job uses "main" as default
branch just because it is called "linux-gcc" is quite hard unless you
are aware of how exactly our CI systems work. And besides being hard to
discover, it's also really fragile.

I wish that we got rid of relying on job names and made this more
discoverable. The obvious way to do so would be to instead declare the
`GIT_TEST_` variables in the CI definitions, which would make them easy
to spot and change. But it of course has the big downside that it is now
quite easy for the different CI platforms to diverge.

> - they might not actually be orthogonal. If you care about checking
> runtime behavior in the output of two compilers, then that _could_
> manifest only in the sha256 code. Or as you get into more
> properties, they may overlap in other ways. I think reftable+sha256
> is an interesting (eventual) combo to test on top of reftable+sha1.

Yes, I really want to have reftable+sha256, as well. I didn't feel like
adding it back then because we already have a ton of jobs, and adding
another job felt like pushing the limits.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]