1439767 - configure should look in ~/.mozbuild/clang/bin for clang-cl and friends

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[(Away)]

In bug 1428349 comment 9, ted wrote:

> dmajor and I discussed briefly on IRC, but as an additional nicety we could fix
> configure to look in ~/.mozbuild/clang/bin when looking for compilers, so that
> developers could simply add `CC=clang-cl CXX=clang-cl` to their mozconfigs and
> have it Just Work with the toolchain that `mach bootstrap` installs.

(I assume this would also cover lld-link.exe?)

Ted, are you able to take on this work?

Comment 1

[Mike Hommey [:glandium]]

For lld-link, you'd have to set LINKER too. Which I think is reasonable.

Comment 2

[(Away)]

(In reply to Mike Hommey [:glandium] from comment #1)
> For lld-link, you'd have to set LINKER too. Which I think is reasonable.

Yes, of course. What I meant was, if you set LINKER, then we should look for it in ~/.mozbuild.

Comment 3

[(not currently active) Ted Mielczarek]

I'm probably not going to get around to this, but I don't think it ought to be a lot of work.

It's also just a "nice to have" to make it easier for people to use clang-cl. It should be already possible for people to simply set:
CC=~/.mozbuild/clang/bin/clang-cl.exe
CXX=~/.mozbuild/clang/bin/clang-cl.exe

in their mozconfigs.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: add .mozbuild clang path to the toolchain search path on Windows

Doing this means compiling with clang-cl is just `export CC=clang-cl` in
somebody's mozconfig, rather than something more elaborate.

This is untested at the moment; David, would you test this out?

Comment 5

[(Away)]

Comment on attachment 8954397 [details] [diff] [review]
add .mozbuild clang path to the toolchain search path on Windows

Works great, thank you!

I removed my PATH hacks from start-shell.bat, confirmed I could no longer find clang-cl nor lld-link, applied the patch, and found them both.

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to David Major [:dmajor] from comment #5)
> Works great, thank you!
> 
> I removed my PATH hacks from start-shell.bat, confirmed I could no longer
> find clang-cl nor lld-link, applied the patch, and found them both.

\o/

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8954397 [details] [diff] [review]
add .mozbuild clang path to the toolchain search path on Windows

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

lgtm.

I wonder if we could be more systematic.  What if we added $MOZ_STATE_PATH/$PROG and $MOZ_STATE_PATH/$PROG/bin to the search path for every moz.configure `check_path` (https://searchfox.org/mozilla-central/source/build/moz.configure/checks.configure#101)?  Maybe we could make this work more generally.

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to Nick Alexander :nalexander from comment #7)
> I wonder if we could be more systematic.  What if we added
> $MOZ_STATE_PATH/$PROG and $MOZ_STATE_PATH/$PROG/bin to the search path for
> every moz.configure `check_path`
> (https://searchfox.org/mozilla-central/source/build/moz.configure/checks.
> configure#101)?  Maybe we could make this work more generally.

I like this idea.  Offhand, we'd have to teach this mechanism that `clang-cl`, `lld-link` (or just `lld`?), and `llvm-config` need to be looked up in the PROG=clang directory.

We already have this mozbuild_state_path dance in (at least!) two places, so it'd be nice to avoid introducing any more!

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Nathan Froyd [:froydnj] from comment #8)
> (In reply to Nick Alexander :nalexander from comment #7)
> > I wonder if we could be more systematic.  What if we added
> > $MOZ_STATE_PATH/$PROG and $MOZ_STATE_PATH/$PROG/bin to the search path for
> > every moz.configure `check_path`
> > (https://searchfox.org/mozilla-central/source/build/moz.configure/checks.
> > configure#101)?  Maybe we could make this work more generally.
> 
> I like this idea.  Offhand, we'd have to teach this mechanism that
> `clang-cl`, `lld-link` (or just `lld`?), and `llvm-config` need to be looked
> up in the PROG=clang directory.

I was a little worried about that.  Follow-up, for sure.

To me, it's connected to $topsrcdir/{clang,android-sdk,etc...} in the source directory, so we should try to address them together.  Maybe.

> We already have this mozbuild_state_path dance in (at least!) two places, so
> it'd be nice to avoid introducing any more!

Indeed.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94094c6e6f87
add .mozbuild clang path to the toolchain search path on Windows; r=nalexander

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/94094c6e6f87