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

From: Patrick Steinhardt <ps@pks.im>
Date: May 17, 2024, 08:33 UTC
Message-ID: <ZkcWZncvfmoKGyKx@tanuki>
In-Reply-To: <20240517081909.GB1517321@coredump.intra.peff.net>

On Fri, May 17, 2024 at 04:19:09AM -0400, Jeff King wrote:

Show 14 quoted lines

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

Show 13 quoted lines

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

Show 5 quoted lines

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