Closed Bug 840512 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla21

People

(Reporter: pnkfelix, Assigned: pnkfelix)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
Took out ansi color annotations that corrupted patch output.  Otherwise same as patch A v1.
Attachment #712909 - Attachment is obsolete: true
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.)
Attachment #712911 - Flags: review?(mh+mozilla)
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.
Attachment #712911 - Flags: review?(mh+mozilla) → review-
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.
(Removed backslash-escape in response to review comments.)
Attachment #712911 - Attachment is obsolete: true
Attachment #713336 - Flags: review?(mh+mozilla)
Attachment #713336 - Flags: review?(mh+mozilla) → review+
Attachment #713355 - Attachment description: revised version of patch made using hg instead of git (for proper formatting) → patch A v4: generalize test command in both copies of toolchain.m4
Assignee: nobody → pnkfelix
Status: NEW → ASSIGNED
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.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1dc58b4dbeb3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: 842131
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.