Closed Bug 818061 Opened 8 years ago Closed 8 years ago

Fallback to systems default clang if CC/CXX is not set

Categories

(Firefox Build System :: General, defect)

20 Branch
All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla20

People

(Reporter: whimboo, Assigned: whimboo)

Details

Attachments

(1 file, 2 obsolete files)

If we can't find clang with the current logic in build/macosx/mozconfig.common we currently fail. Instead we should fallback to the systems default clang if CC and CXX haven't been set. 

http://mxr.mozilla.org/mozilla-central/source/build/macosx/mozconfig.common

We will also have to update the documentation on MDN.
Attached patch Patch v1 (obsolete) — Splinter Review
Falls back to the default clang version if environment variables haven't been declared or clang hasn't been found by the logic in the lines before.
Attachment #688275 - Flags: review?(ted)
Comment on attachment 688275 [details] [diff] [review]
Patch v1

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

::: build/macosx/mozconfig.common
@@ +21,5 @@
> +
> +# If not set use the system default clang++
> +if [ -n CXX ]; then
> +    export CXX=clang++
> +fi

These tests aren't correct, I think you want:
if [ -z "$CC" ]; then
Attachment #688275 - Flags: review?(ted) → review-
Attached patch Patch v2 (obsolete) — Splinter Review
You are right. My bad. Fixed that and also triggered a tryserver build just to be sure we do not break anything.

https://tbpl.mozilla.org/?tree=Try&rev=46d26ddcaae0
Attachment #688275 - Attachment is obsolete: true
Attachment #688770 - Flags: review?(ted)
Comment on attachment 688770 [details] [diff] [review]
Patch v2

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

::: build/macosx/mozconfig.common
@@ +14,5 @@
>      export CXX=$topsrcdir/../clang/bin/clang++
>  fi
> +
> +# If not set use the system default clang
> +if [ -z CC ]; then

I think you actually want [ -z "$CC" ] here (and below).
Attachment #688770 - Flags: review?(ted) → review+
Attached patch Patch v3Splinter Review
Updated once again now with a new set of glasses. Thanks Ted for spotting this mistake.
Attachment #688770 - Attachment is obsolete: true
Attachment #688916 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/582547d8070c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
(In reply to Henrik Skupin (:whimboo) from comment #0)
> We will also have to update the documentation on MDN.

Given that we fixed it in mozconfig.common no updates for MDN have to be made.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.