Re: [PATCH 2/2] ci: let pedantic job compile with -Og

From: Patrick Steinhardt <ps@pks.im>
Date: Jun 6, 2024, 08:25 UTC
Message-ID: <ZmFyi1fQ14tVy7Xg@tanuki>
In-Reply-To: <20240606080552.GA658959@coredump.intra.peff.net>

On Thu, Jun 06, 2024 at 04:05:52AM -0400, Jeff King wrote:

Show 27 quoted lines

> On Thu, Jun 06, 2024 at 09:41:56AM +0200, Patrick Steinhardt wrote:
> > I kind of wonder whether we should revamp this pedantic job in the first
> > place. The consequence of that job is that our codebase needs to be
> > compile cleanly with `-Wpedantic`. So if that is a requirement anyway,
> > why don't we run all jobs with `DEVOPTS=pedantic` and just drop this job
> > altogether? This may surface some additional warnings on platforms where
> > we currently don't set that, but is that a bad thing?
> 
> Yeah, if we always compile cleanly with pedantic, then I don't see why
> it wouldn't just be the default for DEVELOPER=1. The point of that flag
> is to be as picky as possible so that we catch things early. If some
> platform can't handle it (let's imagine Windows or something), then I
> think we should be explicitly _disabling_ pedantic there.
> 
> > The only downside I can think of is that we stop compiling on Fedora,
> > which may have a more up-to-date GCC version than Ubuntu. But if the
> > goal of this job was to _really_ get an up-to-date compiler toolchain,
> > then we should rather pick a rolling release distro like Arch. Otherwise
> > I find this to be of dubious benefit.
> 
> There may be some value in general in compiling on multiple distros, as
> a sort of "unknown unknowns" thing. We don't know what we might turn up,
> but exposing ourselves to more variables may lead to catching failures
> before users see them.
> 
> I don't know if Fedora was specifically chosen for recent gcc there, or
> if it was simply for variety.

True enough. But even so, I think the better solution here would be to drop one of the Ubuntu-based jobs and then convert the Fedora one into a fully-fledged job that also runs tests.

That's something for a later iteration, though.

> Once again, these overlapping variables within various jobs make it hard
> to reason about (but I don't propose normalizing all of them; that would
> increase the amount of CPU work by a lot; I am just grumbling).

No arguing there, it certainly is hard to discover overall. I don't really think there's a way around this if we want to have different combinations while not running a full matrix of jobs, because the combinations are always going to be arbitrary.

> But yeah, between Arch and Fedora, I don't have an opinion. Doing both
> might even be valuable, if we are shoving random variations into random
> jobs. ;)

True.

Show 7 quoted lines

> > If we merge it into the other jobs, then I'd just pick any random job
> > that uses "ubuntu:latest" like "linux-gcc-default" to compile with
> > `-Og`.
> 
> That would be OK with me. I also think it would be OK to do nothing for
> now. We saw one case where "-Og" found a warning that "-O2" didn't. We
> could wait to see if it happens twice before acting. I dunno.

I don't think it hurts to have a job with `-Og`, and if it detects some corner cases that we otherwise don't I think there's a real benefit. In the best case, I'd want to give everyone the tools to avoid sending patch series to the mailing list which are broken and where that could have been detected earlier. In the end, it saves everybody's time.

Patrick