git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Kyle Lippincott <spectral@google.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 2/3] ci: avoid bare "gcc" for osx-gcc job
Date: Thu, 16 May 2024 11:54:44 +0200	[thread overview]
Message-ID: <ZkXX5MlN3EbaMhNG@tanuki> (raw)
In-Reply-To: <20240516071930.GB83658@coredump.intra.peff.net>

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

On Thu, May 16, 2024 at 03:19:30AM -0400, Jeff King wrote:
> On Sat, May 11, 2024 at 07:21:28PM +0200, Patrick Steinhardt wrote:
> 
> > On Fri, May 10, 2024 at 03:47:39PM -0700, Junio C Hamano wrote:
> > > Jeff King <peff@peff.net> writes:
> > [snip]
> > > > [1] Another quirk is that we run the whole test suite for both
> > > >     compilers, which is probably overkill. The main value in comparing
> > > >     gcc vs clang is that we don't use any constructs that the compiler
> > > >     complains about. It's _possible_ for there to be a construct that
> > > >     the compiler does not notice but which causes a runtime difference
> > > >     (say, undefined behavior which happens to work out on one compiler),
> > > >     but I think we're again hitting diminishing returns.
> > > 
> > > Yeah, that is a very good point.
> > 
> > On Linux, we have the "pedantic" job that runs Fedora and only compiles
> > the sources with DEVOPTS=pedantic without running any of the tests. We
> > could do the same on macOS.
> 
> Yeah, I think the infrastructure is there (looks like just resetting
> $run_tests). We probably could stand to use it in more places. E.g., is
> there even value in running the tests for linux-gcc and linux-clang?
> It's _possible_ for there to be a run-time difference in the compiler
> outputs, but we may be hitting diminishing returns. The main value I
> think is just seeing what the compilers complain about.

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 guess this is mostly a question of defaults though, and I think it is
partially influenced by the overall toolchain environment as configured
by my distro. Especially hardening options are for example quite likely
to lead to different behaviour.

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.

So I think dropping the compiler coverage completely is rather pointless
because we already run multiple different jobs per platform anyway. But
we should investigate whether we can cleverly combine those so that we
do not need a separate jobs just to test a specific compiler.

Patrick

> But I dunno. This thread argues there is value in running the tests with
> the separate compiler:
> 
>   https://lore.kernel.org/git/pull.266.git.gitgitgadget@gmail.com/
> 
> which I guess would argue for doing the same for osx-clang and osx-gcc
> (if the latter continues to exist).
> 
> -Peff

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

  parent reply	other threads:[~2024-05-16  9:54 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-09 16:22 [PATCH 0/3] un-breaking osx-gcc ci job Jeff King
2024-05-09 16:23 ` [PATCH 1/3] ci: drop mention of BREW_INSTALL_PACKAGES variable Jeff King
2024-05-09 16:24 ` [PATCH 2/3] ci: avoid bare "gcc" for osx-gcc job Jeff King
2024-05-10  7:00   ` Patrick Steinhardt
2024-05-10 20:16     ` Jeff King
2024-05-10 20:32   ` Kyle Lippincott
2024-05-10 20:48     ` Junio C Hamano
2024-05-10 22:02     ` Jeff King
2024-05-10 22:47       ` Junio C Hamano
2024-05-11 17:21         ` Patrick Steinhardt
2024-05-16  7:19           ` Jeff King
2024-05-16  7:27             ` Jeff King
2024-05-16  9:54             ` Patrick Steinhardt [this message]
2024-05-17  8:19               ` Jeff King
2024-05-17  8:33                 ` Patrick Steinhardt
2024-05-17 16:59                 ` Junio C Hamano
2024-05-23  9:10                   ` Jeff King
2024-05-23 15:35                     ` Junio C Hamano
2024-05-09 16:25 ` [PATCH 3/3] ci: stop installing "gcc-13" for osx-gcc Jeff King
2024-05-10  7:00   ` Patrick Steinhardt
2024-05-10 20:13     ` Jeff King
2024-05-11  7:17       ` Patrick Steinhardt
2024-05-16 12:36         ` Patrick Steinhardt
2024-05-17  8:11           ` Jeff King
2024-05-17  8:25             ` Patrick Steinhardt
2024-05-17 11:30               ` Patrick Steinhardt
2024-05-26  6:34                 ` Philip
2024-05-26 19:23                   ` Junio C Hamano
2024-05-27  5:12                     ` Patrick Steinhardt
2024-05-29  9:27                   ` Jeff King
2024-05-09 16:52 ` [PATCH 0/3] un-breaking osx-gcc ci job Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZkXX5MlN3EbaMhNG@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=spectral@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).