| Message ID | 20210806205235.988761-1-gitster@pobox.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | detect-compiler: clang updates | expand |
On Fri, Aug 06 2021, Junio C Hamano wrote: > So here is a mini-series that summarizes what has been suggested so > far on the topic. > > Carlo Marcelo Arenas Belón (1): > build: update detect-compiler for newer Xcode version > > Jeff King (1): > build: clang version may not be followed by extra words > > Junio C Hamano (1): > build: catch clang that identifies itself as "$VENDOR clang" > > detect-compiler | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) Perhaps I've missed some obvious reason not to do this, but why are we parsing the --version output of two modern compilers, as opposed to just asking them what type/version they are via their usual macro facilities? I.e. something like the below: diff --git a/detect-compiler b/detect-compiler index 70b754481c8..37908ac9ea8 100755 --- a/detect-compiler +++ b/detect-compiler @@ -5,19 +5,47 @@ CC="$*" -# we get something like (this is at least true for gcc and clang) +#!/bin/sh # -# FreeBSD clang version 3.4.1 (tags/RELEASE...) -get_version_line() { - $CC -v 2>&1 | grep ' version ' -} +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +v=$($CC -E - <<-EOF 2>&1 | grep -e '=') +GNUC=__GNUC__ +GNUC_MINOR=__GNUC_MINOR__ +GNUC_PATCHLEVEL=__GNUC_PATCHLEVEL__ +clang=__clang__ +clang_major=__clang_major__ +clang_minor=__clang_minor__ +clang_patchlevel=__clang_patchlevel__ +EOF +eval "$v" get_family() { - get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' + # Clang also sets the GNUC macros, but GCC does not set + # clang's. + if test "$clang" != "__clang__" + then + echo clang + elif test "$GNUC" != "__GNUC__" + then + echo gcc + else + echo unknown + fi } get_version() { - get_version_line | sed 's/^.* version \([0-9][^ ]*\) .*/\1/' + case "$(get_family)" in + clang) + echo "$clang_major.$clang_minor.$clang_patchlevel" + ;; + gcc) + echo "$GNUC.$GNUC_MINOR.$GNUC_PATCHLEVEL" + ;; + esac } print_flags() { @@ -41,12 +69,6 @@ gcc) clang) print_flags clang ;; -"FreeBSD clang") - print_flags clang - ;; -"Apple LLVM") - print_flags clang - ;; *) : unknown compiler family ;;
On Sat, Aug 07, 2021 at 04:02:45AM +0200, Ævar Arnfjörð Bjarmason wrote: > Perhaps I've missed some obvious reason not to do this, but why are we > parsing the --version output of two modern compilers, as opposed to just > asking them what type/version they are via their usual macro facilities? > I.e. something like the below: That would probably work OK in practice, but it actually seems more complex to me (how do other random compilers react to "-E -"? Is it possible for us to get other output from the preprocessor that would confuse an eval?). It could perhaps make the Apple version-string confusion go away, though, which might make it worth doing. -Peff
On Fri, Aug 06 2021, Jeff King wrote: > On Sat, Aug 07, 2021 at 04:02:45AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> Perhaps I've missed some obvious reason not to do this, but why are we >> parsing the --version output of two modern compilers, as opposed to just >> asking them what type/version they are via their usual macro facilities? >> I.e. something like the below: > > That would probably work OK in practice, but it actually seems more > complex to me (how do other random compilers react to "-E -"? We only care about gcc and clang in that script, which I think have supported that form of "-E" on stdin input for any version we're likely to care about for the purposes of config.mak.dev. It seems unlikely that we'll care about non-modern compilers in config.mak.dev, so using more modern features there seems fine (it's all for opting us into even more modern warning flags and the like...). > Is it possible for us to get other output from the preprocessor that > would confuse an eval?). Probably, I just meant that as a POC. We could pipe it into some awk/grep/cut/perl or whatever that would be more strict.
On Sat, Aug 07, 2021 at 04:56:04AM +0200, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Aug 07, 2021 at 04:02:45AM +0200, Ævar Arnfjörð Bjarmason wrote: > > > >> Perhaps I've missed some obvious reason not to do this, but why are we > >> parsing the --version output of two modern compilers, as opposed to just > >> asking them what type/version they are via their usual macro facilities? > >> I.e. something like the below: > > > > That would probably work OK in practice, but it actually seems more > > complex to me (how do other random compilers react to "-E -"? > > We only care about gcc and clang in that script, which I think have > supported that form of "-E" on stdin input for any version we're likely > to care about for the purposes of config.mak.dev. It seems unlikely that > we'll care about non-modern compilers in config.mak.dev, so using more > modern features there seems fine (it's all for opting us into even more > modern warning flags and the like...). Yeah, but we don't find out what we have until we run the script in question. I guess it is OK as long as we redirect stderr, ignore the exit code, and only look for a positive outcome in the output (your patch does the latter two already). I also wondered how this might interact with CC="ccache gcc" (where caching might fail to notice version changes). But from some quick testing, it looks like it doesn't cache in this case (neither stdin, nor with -E). > > Is it possible for us to get other output from the preprocessor that > > would confuse an eval?). > > Probably, I just meant that as a POC. We could pipe it into some > awk/grep/cut/perl or whatever that would be more strict. That would probably be better. I would be curious to hear from somebody with a mac if this technique gives more sensible version numbers for the Apple-clang compiler. -Peff
On Sat, Aug 07 2021, Jeff King wrote: > On Sat, Aug 07, 2021 at 04:56:04AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> > On Sat, Aug 07, 2021 at 04:02:45AM +0200, Ævar Arnfjörð Bjarmason wrote: >> > >> >> Perhaps I've missed some obvious reason not to do this, but why are we >> >> parsing the --version output of two modern compilers, as opposed to just >> >> asking them what type/version they are via their usual macro facilities? >> >> I.e. something like the below: >> > >> > That would probably work OK in practice, but it actually seems more >> > complex to me (how do other random compilers react to "-E -"? >> >> We only care about gcc and clang in that script, which I think have >> supported that form of "-E" on stdin input for any version we're likely >> to care about for the purposes of config.mak.dev. It seems unlikely that >> we'll care about non-modern compilers in config.mak.dev, so using more >> modern features there seems fine (it's all for opting us into even more >> modern warning flags and the like...). > > Yeah, but we don't find out what we have until we run the script in > question. I guess it is OK as long as we redirect stderr, ignore the > exit code, and only look for a positive outcome in the output (your > patch does the latter two already). > > I also wondered how this might interact with CC="ccache gcc" (where > caching might fail to notice version changes). But from some quick > testing, it looks like it doesn't cache in this case (neither stdin, nor > with -E). > >> > Is it possible for us to get other output from the preprocessor that >> > would confuse an eval?). >> >> Probably, I just meant that as a POC. We could pipe it into some >> awk/grep/cut/perl or whatever that would be more strict. > > That would probably be better. I would be curious to hear from somebody > with a mac if this technique gives more sensible version numbers for the > Apple-clang compiler. It does, on the gcc304 box on the gccfarm (recent apple M1 Mac Mini): avar@minimac ~ % uname -a Darwin minimac.moose.housegordon.com 20.4.0 Darwin Kernel Version 20.4.0: Thu Apr 22 21:46:41 PDT 2021; root:xnu-7195.101.2~1/RELEASE_ARM64_T8101 arm64 avar@minimac ~ % clang --version Apple clang version 12.0.5 (clang-1205.0.22.9) Target: arm64-apple-darwin20.4.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin avar@minimac ~ % cat >f GNUC=__GNUC__ GNUC_MINOR=__GNUC_MINOR__ GNUC_PATCHLEVEL=__GNUC_PATCHLEVEL__ clang=__clang__ clang_major=__clang_major__ clang_minor=__clang_minor__ clang_patchlevel=__clang_patchlevel__ ^C avar@minimac ~ % clang -E - <f # 1 "<stdin>" # 1 "<built-in>" 1 # 1 "<built-in>" 3 # 384 "<built-in>" 3 # 1 "<command line>" 1 # 1 "<built-in>" 2 # 1 "<stdin>" 2 GNUC=4 GNUC_MINOR=2 GNUC_PATCHLEVEL=1 clang=1 clang_major=12 clang_minor=0 clang_patchlevel=5 I think nobody who's using clang derivatives is screwing with these macro variables, they're just changing whatever the "product name" or whatever is in the --version output.
On Sat, Aug 07, 2021 at 04:26:33PM +0200, Ævar Arnfjörð Bjarmason wrote: > > That would probably be better. I would be curious to hear from somebody > > with a mac if this technique gives more sensible version numbers for the > > Apple-clang compiler. > > It does, on the gcc304 box on the gccfarm (recent apple M1 Mac Mini): > > avar@minimac ~ % uname -a > Darwin minimac.moose.housegordon.com 20.4.0 Darwin Kernel Version 20.4.0: Thu Apr 22 21:46:41 PDT 2021; root:xnu-7195.101.2~1/RELEASE_ARM64_T8101 arm64 > avar@minimac ~ % clang --version > Apple clang version 12.0.5 (clang-1205.0.22.9) > Target: arm64-apple-darwin20.4.0 > Thread model: posix > InstalledDir: /Library/Developer/CommandLineTools/usr/bin > > avar@minimac ~ % cat >f > GNUC=__GNUC__ > GNUC_MINOR=__GNUC_MINOR__ > GNUC_PATCHLEVEL=__GNUC_PATCHLEVEL__ > clang=__clang__ > clang_major=__clang_major__ > clang_minor=__clang_minor__ > clang_patchlevel=__clang_patchlevel__ > > ^C > > avar@minimac ~ % clang -E - <f > # 1 "<stdin>" > # 1 "<built-in>" 1 > # 1 "<built-in>" 3 > # 384 "<built-in>" 3 > # 1 "<command line>" 1 > # 1 "<built-in>" 2 > # 1 "<stdin>" 2 > GNUC=4 > GNUC_MINOR=2 > GNUC_PATCHLEVEL=1 > clang=1 > clang_major=12 > clang_minor=0 > clang_patchlevel=5 Hmm, now I'm really confused, though. Is that really clang 12 (for which there is no 12.0.5; 12.0.1 is the latest version, shipped in July)? Or is it XCode 12, shipping with LLVM 11, according to the table in: https://en.wikipedia.org/wiki/Xcode#Xcode_11.x_-_13.x_(since_SwiftUI_framework) (sorry, there are actually _two_ tables with that same anchor on the page; the one you want is the second one, under "Toolchain versions"). The distinction does not matter for our script (where we only care about "clang4" and up). I guess the most relevant test would be to get XCode 8.x and see what it says. I expect it to claim "clang 8.1.0" or similar, but actually be clang-3. And therefore not support -Wtautological-constant-out-of-range-compare. If we can't get easily get hold of such a platform, then maybe that is a good indication that this conversation is too academic for now, and we should wait until somebody wants to add a more recent version-specifier to config.mak.dev. ;) -Peff
On Sat, Aug 07 2021, Jeff King wrote: > On Sat, Aug 07, 2021 at 04:26:33PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> > That would probably be better. I would be curious to hear from somebody >> > with a mac if this technique gives more sensible version numbers for the >> > Apple-clang compiler. >> >> It does, on the gcc304 box on the gccfarm (recent apple M1 Mac Mini): >> >> avar@minimac ~ % uname -a >> Darwin minimac.moose.housegordon.com 20.4.0 Darwin Kernel >> Version 20.4.0: Thu Apr 22 21:46:41 PDT 2021; >> root:xnu-7195.101.2~1/RELEASE_ARM64_T8101 arm64 >> avar@minimac ~ % clang --version >> Apple clang version 12.0.5 (clang-1205.0.22.9) >> Target: arm64-apple-darwin20.4.0 >> Thread model: posix >> InstalledDir: /Library/Developer/CommandLineTools/usr/bin >> >> avar@minimac ~ % cat >f >> GNUC=__GNUC__ >> GNUC_MINOR=__GNUC_MINOR__ >> GNUC_PATCHLEVEL=__GNUC_PATCHLEVEL__ >> clang=__clang__ >> clang_major=__clang_major__ >> clang_minor=__clang_minor__ >> clang_patchlevel=__clang_patchlevel__ >> >> ^C >> >> avar@minimac ~ % clang -E - <f >> # 1 "<stdin>" >> # 1 "<built-in>" 1 >> # 1 "<built-in>" 3 >> # 384 "<built-in>" 3 >> # 1 "<command line>" 1 >> # 1 "<built-in>" 2 >> # 1 "<stdin>" 2 >> GNUC=4 >> GNUC_MINOR=2 >> GNUC_PATCHLEVEL=1 >> clang=1 >> clang_major=12 >> clang_minor=0 >> clang_patchlevel=5 > > Hmm, now I'm really confused, though. Is that really clang 12 (for which > there is no 12.0.5; 12.0.1 is the latest version, shipped in July)? Or > is it XCode 12, shipping with LLVM 11, according to the table in: > > https://en.wikipedia.org/wiki/Xcode#Xcode_11.x_-_13.x_(since_SwiftUI_framework) > > (sorry, there are actually _two_ tables with that same anchor on the > page; the one you want is the second one, under "Toolchain versions"). > > The distinction does not matter for our script (where we only care about > "clang4" and up). I guess the most relevant test would be to get XCode > 8.x and see what it says. I expect it to claim "clang 8.1.0" or similar, > but actually be clang-3. And therefore not support > -Wtautological-constant-out-of-range-compare. > > If we can't get easily get hold of such a platform, then maybe that is a > good indication that this conversation is too academic for now, and we > should wait until somebody wants to add a more recent version-specifier > to config.mak.dev. ;) I think it's clang 12.0.5, and Apple just takes upstream versions and increments them, e.g. I found this: https://gist.github.com/yamaya/2924292 So you can presumably rely on it for having clang 12 features, and we'd only ever care about the clang_major...
On Sat, Aug 7, 2021 at 7:40 AM Jeff King <peff@peff.net> wrote: > The distinction does not matter for our script (where we only care about > "clang4" and up). I guess the most relevant test would be to get XCode > 8.x and see what it says. I expect it to claim "clang 8.1.0" or similar, > but actually be clang-3. And therefore not support > -Wtautological-constant-out-of-range-compare. uses Xcode 7.3 (based on clang 3.8) and either does support that flag or ignores it silently https://www.travis-ci.com/github/carenas/git/builds/234772346 the same was observed with Xcode 8 both error later and fail to build because of a valid (but harmless) -Wformat-extra-args warning that doesn't trigger in later versions Carlo
On Sat, Aug 07, 2021 at 05:30:39PM -0700, Carlo Arenas wrote: > On Sat, Aug 7, 2021 at 7:40 AM Jeff King <peff@peff.net> wrote: > > The distinction does not matter for our script (where we only care about > > "clang4" and up). I guess the most relevant test would be to get XCode > > 8.x and see what it says. I expect it to claim "clang 8.1.0" or similar, > > but actually be clang-3. And therefore not support > > -Wtautological-constant-out-of-range-compare. > > uses Xcode 7.3 (based on clang 3.8) and either does support that flag > or ignores it silently > > https://www.travis-ci.com/github/carenas/git/builds/234772346 > > the same was observed with Xcode 8 > > both error later and fail to build because of a valid (but harmless) > -Wformat-extra-args warning that doesn't trigger in later versions Thanks for testing. I think I was wrong that clang4 is the limit for that option, though. It comes originally from: https://lore.kernel.org/git/20180317160832.GB15772@sigill.intra.peff.net/ where clang4 just happened to be the oldest thing I had access to at the time, so we used that as a minimum. So probably all of our "clang4" could really be "any clang" (but it is probably OK to leave it as-is). -Peff
On Sat, Aug 07, 2021 at 05:36:57PM +0200, Ævar Arnfjörð Bjarmason wrote: > > Hmm, now I'm really confused, though. Is that really clang 12 (for which > > there is no 12.0.5; 12.0.1 is the latest version, shipped in July)? Or > > is it XCode 12, shipping with LLVM 11, according to the table in: > > > > https://en.wikipedia.org/wiki/Xcode#Xcode_11.x_-_13.x_(since_SwiftUI_framework) > > > > (sorry, there are actually _two_ tables with that same anchor on the > > page; the one you want is the second one, under "Toolchain versions"). > > > > The distinction does not matter for our script (where we only care about > > "clang4" and up). I guess the most relevant test would be to get XCode > > 8.x and see what it says. I expect it to claim "clang 8.1.0" or similar, > > but actually be clang-3. And therefore not support > > -Wtautological-constant-out-of-range-compare. > > > > If we can't get easily get hold of such a platform, then maybe that is a > > good indication that this conversation is too academic for now, and we > > should wait until somebody wants to add a more recent version-specifier > > to config.mak.dev. ;) > > I think it's clang 12.0.5, and Apple just takes upstream versions and > increments them, e.g. I found this: > https://gist.github.com/yamaya/2924292 > > So you can presumably rely on it for having clang 12 features, and we'd > only ever care about the clang_major... From the reading I've done, I'm unconvinced that this is _actually_ clang 12, and they aren't simply doing funky things with the version numbers. But since we lack an easy test of how the different versions behave (even my suggested clang4 test turned out not to be robust!), it probably doesn't matter much either way. -Peff