fix mozconfig suggestions for Windows builds

RESOLVED FIXED in Firefox 55

Status

RESOLVED FIXED
a year ago
6 months ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

Trunk
mozilla55
All
Windows

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

a year ago
I think the following need to be fixed:

* We should suggest the correct thing for --with-libclang-path
* Arguments should be quoted, since the embedded paths can have spaces in them (maybe?)
* Our tooltool packages have llvm-config.exe, we should just suggest using that if it's available so there's less stuff to put in the mozconfig.
(Assignee)

Comment 1

a year ago
(In reply to Nathan Froyd [:froydnj] from comment #0)
> * Our tooltool packages have llvm-config.exe, we should just suggest using
> that if it's available so there's less stuff to put in the mozconfig.

This turns out to be a dicey proposition, because llvm-config gets an 8.3 path due to...issues...in the build system, and it then provides 8.3 paths for the paths we ask it for, which I'm not certain will actually work with bindgen.  Let's leave this one alone for the time being.
(Assignee)

Comment 2

a year ago
Created attachment 8871901 [details] [diff] [review]
part 1 - provide the correct --with-libclang-path suggestions

libclang.dll lives in the bin/ directory, not the lib/ directory.

(These patches are on top of bug 1364588, because it's convenient.)
Attachment #8871901 - Flags: review?(giles)
(Assignee)

Comment 3

a year ago
Created attachment 8871902 [details] [diff] [review]
part 2- quote suggested Stylo paths on Windows

We install the tooltool packages to ${HOME}/.mozbuild by default, and
${HOME} may have spaces on Windows.  The arguments to the command-line
options need to be quoted appropriately.
Attachment #8871902 - Flags: review?(giles)
(In reply to Nathan Froyd [:froydnj] from comment #2)
> Created attachment 8871901 [details] [diff] [review]
> part 1 - provide the correct --with-libclang-path suggestions
> 
> libclang.dll lives in the bin/ directory, not the lib/ directory.
> 
> (These patches are on top of bug 1364588, because it's convenient.)

Wait, I set --with-libclang-path="D:/Program\ Files/LLVM/lib" and the build system correctly finds libclang.dll under "D:/Program Files/LLVM/bin".
(Assignee)

Comment 5

a year ago
(In reply to Masatoshi Kimura [:emk] from comment #4)
> Wait, I set --with-libclang-path="D:/Program\ Files/LLVM/lib" and the build
> system correctly finds libclang.dll under "D:/Program Files/LLVM/bin".

bindgen will fix things up, but it seems better to just point things at the correct place from the start, to avoid confusion later.
Comment on attachment 8871901 [details] [diff] [review]
part 1 - provide the correct --with-libclang-path suggestions

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

Wow, confusing. The base and windows versions were _both_ wrong?

::: python/mozboot/mozboot/mozillabuild.py
@@ +16,5 @@
>  
>  <<<
>  ac_add_options --enable-stylo
>  
> +ac_add_options --with-libclang-path={state_dir}/clang/bin

The help string for --with-libclang-path mentions 'libraries' I like "directory containing a libclang shared library" from https://github.com/KyleMayes/clang-sys#environment-variables a bit better. More obvious what goes here.
Attachment #8871901 - Flags: review?(giles) → review+
Comment on attachment 8871902 [details] [diff] [review]
part 2- quote suggested Stylo paths on Windows

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

I'll assume this quoting both makes it into the message correctly, and works in mozconfig. :)
Attachment #8871902 - Flags: review?(giles) → review+

Comment 8

a year ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d25c3ea9b76
part 1 - provide the correct --with-libclang-path suggestions; r=rillian
https://hg.mozilla.org/integration/mozilla-inbound/rev/9299b9448718
part 2- quote suggested Stylo paths on Windows; r=rillian
https://hg.mozilla.org/mozilla-central/rev/3d25c3ea9b76
https://hg.mozilla.org/mozilla-central/rev/9299b9448718
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

6 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.