840512 - clang version string has changed on OS X; CLANG_CXX and CLANG_CC need generalization.

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Felix S. Klock II [:pnkfelix, :fklock]]

The Mac OS X version of clang included with Xcode circa 4.5.2 has changed the output format so that it no longer includes the substring "clang version"

See below:

% xcodebuild -version
Xcode 4.5.2
Build version 4G2008a
% which clang++
/usr/bin/clang++
% ls -l /usr/bin/clang++ 
lrwxr-xr-x  1 root  wheel  5 Feb 12 11:34 /usr/bin/clang++ -> clang
% ls -l /usr/bin/clang
-rwxr-xr-x  1 root  wheel  26291280 Feb 12 11:34 /usr/bin/clang
% clang++ --version
Apple LLVM version 4.2 (clang-425.0.24) (based on LLVM 3.2svn)
Target: x86_64-apple-darwin12.2.1
Thread model: posix


This breaks the way that we detect the presence of clang in toolchain.m4:

CLANG_CC=
CLANG_CXX=
if test "$GCC" = yes; then
   if test "`$CC -v 2>&1 | grep -c 'clang version'`" != "0"; then
     CLANG_CC=1
   fi
fi


And subsequently in the configure script, we end up not setting certain parameters that were put in place to accommodate clang.

Comment 1

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: patch A v1: generalize test command in both copies of toolchain.m4

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: patch A v2: generalize test command in both copies of toolchain.m4

Took out ansi color annotations that corrupted patch output.  Otherwise same as patch A v1.

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 712911 [details] [diff] [review]
patch A v2: generalize test command in both copies of toolchain.m4

(folks in #jsapi channel said :ted or :glandium are good potential reviewers.)

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 712911 [details] [diff] [review]
patch A v2: generalize test command in both copies of toolchain.m4

Review of attachment 712911 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/autoconf/toolchain.m4
@@ +55,5 @@
>  
>  CLANG_CC=
>  CLANG_CXX=
>  if test "$GCC" = yes; then
> +   if test "`$CC -v 2>&1 | grep -c 'clang version\\|Apple.*clang'`" != "0"; then

I doubt this works with two backslashes.

Comment 5

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 712911 [details] [diff] [review]
patch A v2: generalize test command in both copies of toolchain.m4

Review of attachment 712911 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/autoconf/toolchain.m4
@@ +55,5 @@
>  
>  CLANG_CC=
>  CLANG_CXX=
>  if test "$GCC" = yes; then
> +   if test "`$CC -v 2>&1 | grep -c 'clang version\\|Apple.*clang'`" != "0"; then

Hmm, it works for me (and I had thought it was necessary to escape the backslash in the quoted string), but yes, retesting shows they are not actually necessary.  So I will remove them.

Comment 6

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: patch A v3: generalize test command in both copies of toolchain.m4

(Removed backslash-escape in response to review comments.)

Comment 7

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: patch A v4: generalize test command in both copies of toolchain.m4

Comment 8

[Felix S. Klock II [:pnkfelix, :fklock]]

I've done a try build (using all targets except windows, as suggested in #developers):

  https://tbpl.mozilla.org/?tree=Try&rev=be6908e27730

So now someone with appropriate permissions can check this in.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1dc58b4dbeb3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1dc58b4dbeb3