Closed Bug 1390640 Opened 7 years ago Closed 7 years ago

make NDK clang automatically discoverable by moz.configure

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox57 affected)

RESOLVED INCOMPLETE
Tracking Status
firefox57 --- affected

People

(Reporter: froydnj, Unassigned)

References

Details

Configure knows how to find the GCC inside of the NDK automatically, because android sets up --with-toolchain-prefix automatically, and configure's searching for default compilers understands how to use that to locate gcc.  I would like to do the same thing for the clang in the NDK, preferably without adding android-specific logic or options to toolchain.configure.

The problem is that clang is located in an entirely different directory and doesn't follow the ${TARGET}- prefix scheme that GCC does.  So I'm not sure there's a good way to do it without adding some android-specific logic/option.

I attempted to return a namespace from android_toolchain_prefix:

http://dxr.mozilla.org/mozilla-central/source/build/moz.configure/android-ndk.configure#195

but that didn't work so well.  I guess I could return some sort of sekret format in the string (comma-separated values?), but then I'd have to modify all the places that looked at --with-toolchain-prefix, and it'd be a bit of a footgun.

Do you have any ideas, glandium?
Flags: needinfo?(mh+mozilla)
If clang is /path/to/clang, --with-toolchain-prefix=/path/to/ should work.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #1)
> If clang is /path/to/clang, --with-toolchain-prefix=/path/to/ should work.

I should have mentioned upfront that has its own set of issues.  Many places depend on a) --with-toolchain-prefix not even being an actual path, but something to be used with string concatenation (e.g. not os.path.join'd):

http://dxr.mozilla.org/mozilla-central/source/build/moz.configure/android-ndk.configure#195

and b) depend on --with-toolchain-prefix, or at least TOOLCHAIN_PREFIX, which is derived from --with-toolchain-prefix, pointing at binutils, e.g.:

http://dxr.mozilla.org/mozilla-central/source/build/autoconf/toolchain.m4#78

and the NDK clang is not smart enough to locate things like the NDK's ld unassisted, so even the NDK clang needs to be pointed at the NDK binutils.

So we could, AFAICS:

1) Keep --with-toolchain-prefix the same, but derive the location of clang from it here:

http://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#513-534

2) Change --with-toolchain-prefix as you suggest, but then we'd have to derive the location of the NDK's GCC installation:

https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#189-205

to make everything else in configure happy.

3) Add a separate option (--with-clang-toolchain-prefix) that shouldn't normally be needed, but that Android could automatically set in a similar fashion to --with-toolchain-prefix.

4) Something else that I haven't thought of.

Is that a clearer summary of the situation?  What do you think we should do?
Flags: needinfo?(mh+mozilla)
(In reply to Nathan Froyd [:froydnj] from comment #2)
> (In reply to Mike Hommey [:glandium] from comment #1)
> > If clang is /path/to/clang, --with-toolchain-prefix=/path/to/ should work.
> 
> I should have mentioned upfront that has its own set of issues.  Many places
> depend on a) --with-toolchain-prefix not even being an actual path, but
> something to be used with string concatenation (e.g. not os.path.join'd):
> 
> http://dxr.mozilla.org/mozilla-central/source/build/moz.configure/android-
> ndk.configure#195

which works equally for /path/to/ and /path/to/i686-linux-android-. But I guess the problem is that ld and other tools are not /path/to/ld, etc.

> and b) depend on --with-toolchain-prefix, or at least TOOLCHAIN_PREFIX,
> which is derived from --with-toolchain-prefix, pointing at binutils, e.g.:
> 
> http://dxr.mozilla.org/mozilla-central/source/build/autoconf/toolchain.m4#78
> 
> and the NDK clang is not smart enough to locate things like the NDK's ld
> unassisted, so even the NDK clang needs to be pointed at the NDK binutils.
> 
> So we could, AFAICS:
> 
> 1) Keep --with-toolchain-prefix the same, but derive the location of clang
> from it here:
> 
> http://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.
> configure#513-534
> 
> 2) Change --with-toolchain-prefix as you suggest, but then we'd have to
> derive the location of the NDK's GCC installation:
> 
> https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.
> configure#189-205
> 
> to make everything else in configure happy.
> 
> 3) Add a separate option (--with-clang-toolchain-prefix) that shouldn't
> normally be needed, but that Android could automatically set in a similar
> fashion to --with-toolchain-prefix.
> 
> 4) Something else that I haven't thought of.
> 
> Is that a clearer summary of the situation?  What do you think we should do?

alter PATH? make ndk.configure imply_option() CC and CXX?
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #3)
> (In reply to Nathan Froyd [:froydnj] from comment #2)
> > 4) Something else that I haven't thought of.
> > 
> > Is that a clearer summary of the situation?  What do you think we should do?
> 
> alter PATH? make ndk.configure imply_option() CC and CXX?

I am not super excited about altering PATH.  I guess we already do it for Windows (!), but I am a little worried about HOST_CC etc. getting pointed at the clang in the NDK and havoc resulting from that.  It's not clear to me that the NDK clang would necessarily find all the host headers and such that the clang originally in PATH would.

imply_option'ing CC seems to run into problems with old-configure.in re-exporting CC as something different (clang -std=gnu99 --target=...) for the js/src/ subconfigure and then moz.configure complaining about mismatches between the found value in the environment and the imply_option'd value.

But the imply_option suggestion suggests another possibility: we define a function in android-ndk.configure that returns the appropriate CC/CXX pair from the NDK, and then default_c_compilers can use those in its list of possibilities.  This is similar to the extra_toolchain_flags trick we already use for the NDK.
I am going to take care of this as part of bug 1163171, since it'd be a bit confusing to introduce it while we're still using GCC.  I'm going to use the strategy outlined in comment 4.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.