Last Comment Bug 840512 - clang version string has changed on OS X; CLANG_CXX and CLANG_CC need generalization.
: clang version string has changed on OS X; CLANG_CXX and CLANG_CC need general...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla21
Assigned To: Felix S. Klock II [:pnkfelix, :fklock]
:
Mentors:
Depends on: 842131
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-12 06:56 PST by Felix S. Klock II [:pnkfelix, :fklock]
Modified: 2013-02-17 06:11 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch A v1: generalize test command in both copies of toolchain.m4 (2.07 KB, patch)
2013-02-12 07:36 PST, Felix S. Klock II [:pnkfelix, :fklock]
no flags Details | Diff | Splinter Review
patch A v2: generalize test command in both copies of toolchain.m4 (1.78 KB, patch)
2013-02-12 07:38 PST, Felix S. Klock II [:pnkfelix, :fklock]
mh+mozilla: review-
Details | Diff | Splinter Review
patch A v3: generalize test command in both copies of toolchain.m4 (1.73 KB, patch)
2013-02-13 02:31 PST, Felix S. Klock II [:pnkfelix, :fklock]
mh+mozilla: review+
Details | Diff | Splinter Review
patch A v4: generalize test command in both copies of toolchain.m4 (1.68 KB, patch)
2013-02-13 04:44 PST, Felix S. Klock II [:pnkfelix, :fklock]
no flags Details | Diff | Splinter Review

Description Felix S. Klock II [:pnkfelix, :fklock] 2013-02-12 06:56:12 PST
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] 2013-02-12 07:36:13 PST
Created attachment 712909 [details] [diff] [review]
patch A v1: generalize test command in both copies of toolchain.m4
Comment 2 Felix S. Klock II [:pnkfelix, :fklock] 2013-02-12 07:38:30 PST
Created attachment 712911 [details] [diff] [review]
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] 2013-02-12 07:51:05 PST
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] 2013-02-12 08:11:55 PST
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] 2013-02-13 02:28:43 PST
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] 2013-02-13 02:31:31 PST
Created attachment 713336 [details] [diff] [review]
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] 2013-02-13 04:44:35 PST
Created attachment 713355 [details] [diff] [review]
patch A v4: generalize test command in both copies of toolchain.m4
Comment 8 Felix S. Klock II [:pnkfelix, :fklock] 2013-02-14 05:47:11 PST
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] 2013-02-14 06:14:14 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dc58b4dbeb3
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-02-14 14:23:44 PST
https://hg.mozilla.org/mozilla-central/rev/1dc58b4dbeb3

Note You need to log in before you can comment on or make changes to this bug.