Re: [PATCH v2 3/3] build: catch clang that identifies itself as "$VENDOR clang"

From: Jeff King <peff@peff.net>
Date: Aug 7, 2021, 02:09 UTC
Message-ID: <YQ3raB5ZtPHBQWSQ@coredump.intra.peff.net>
In-Reply-To: <20210806205235.988761-4-gitster@pobox.com>

On Fri, Aug 06, 2021 at 01:52:35PM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> diff --git a/detect-compiler b/detect-compiler
> index 955be1c906..11d60da5b7 100755
> --- a/detect-compiler
> +++ b/detect-compiler
> @@ -38,13 +38,10 @@ case "$(get_family)" in
>  gcc)
>  	print_flags gcc
>  	;;
> -clang)
> +clang | *" clang")
>  	print_flags clang
>  	;;
> -"FreeBSD clang")
> -	print_flags clang
> -	;;
> -"Apple LLVM"|"Apple clang")
> +"Apple LLVM")
>  	print_flags clang
>  	;;

All three patches look fine to me, and functionality-wise are a strict improvement over the status quo. But I suspect in the long run we'd need to keep all of the Apple bits in their own case-arm, like:

  # this must come first, so we prefer it over "* clang".
  "Apple LLVM" | "Apple clang")
	print_apple_magic
        ;;
  clang | *" clang")
        print_flags clang
        ;;

and then apple_magic does the version conversion from Wikipedia I linked to earlier.

I don't think your patch is really making it significantly harder to get there, though splitting up "Apple LLVM" and "Apple clang" feels a bit like it's the wrong direction.

I wasn't personally planning to take that next step, as I lack the platform to test it on. And as noted, unless you have a pretty old version of Xcode, it doesn't matter either way (so I'm content to leave it until dev with a mac is bitten by it and cares enough to make it more accurate).

-Peff