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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v1

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.

Comment 2

[(not currently active) Ted Mielczarek]

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

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v2

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

Comment 4

[(not currently active) Ted Mielczarek]

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).

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v3

Updated once again now with a new set of glasses. Thanks Ted for spotting this mistake.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 688916 [details] [diff] [review]
Patch v3

https://hg.mozilla.org/integration/mozilla-inbound/rev/582547d8070c

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/582547d8070c

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(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.