Closed
Bug 1372987
Opened 7 years ago
Closed 7 years ago
move library/object prefix/suffix configuration to moz.configure
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
For parts of configuring Stylo, we need information about the library
extensions on all of our platforms, and this change is a reasonable way
to get at that information without duplicating it in two places. Plus
moving more things to moz.configure is more better.
Assignee | ||
Comment 1•7 years ago
|
||
This builds on all platforms we test in automation.
Attachment #8877694 -
Flags: review?(mshal)
Comment 2•7 years ago
|
||
Comment on attachment 8877694 [details] [diff] [review]
move library/object prefix/suffix configuration to moz.configure
DLL_PREFIX / DLL_SUFFIX are still used in an old-configure script: https://dxr.mozilla.org/mozilla-central/rev/da66c4a05fda49d457d9411a7092fed87cf9e53a/build/autoconf/expandlibs.m4#56
They don't appear to actually be needed. Can we just remove them from that test? If not, we probably need to add old configure assignments for them as well.
Attachment #8877694 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 3•7 years ago
|
||
Thanks for the review.
(In reply to Michael Shal [:mshal] from comment #2)
> DLL_PREFIX / DLL_SUFFIX are still used in an old-configure script:
> https://dxr.mozilla.org/mozilla-central/rev/
> da66c4a05fda49d457d9411a7092fed87cf9e53a/build/autoconf/expandlibs.m4#56
>
> They don't appear to actually be needed. Can we just remove them from that
> test? If not, we probably need to add old configure assignments for them as
> well.
I think we can remove them. Alternatively, I think we can just delete the testing for EXPAND_LIBS_ORDER_STYLE; while it is used extensively:
https://dxr.mozilla.org/mozilla-central/search?q=EXPAND_LIBS_ORDER_STYLE&redirect=true
It only comes into play if --symbol-order is passed to expandlibs.py:
https://dxr.mozilla.org/mozilla-central/source/config/expandlibs_exec.py#322
https://dxr.mozilla.org/mozilla-central/source/config/expandlibs_exec.py#330
But the only way --symbol-order gets passed is here:
https://dxr.mozilla.org/mozilla-central/source/config/config.mk#551-553
And SYMBOL_ORDER appears to be unused:
https://dxr.mozilla.org/mozilla-central/search?q=SYMBOL_ORDER&redirect=false
So I guess we can go with the tiny amount of code deletion, or we can go for the more extensive removal. WDYT?
Flags: needinfo?(mshal)
Comment 4•7 years ago
|
||
I'm not sure - it doesn't look like SYMBOL_ORDER was set anywhere in tree by the patch from bug 603370 - maybe it was intended to be set externally for testing, or by buildbot or something? glandium, can you comment on #c3?
Flags: needinfo?(mshal)
Updated•7 years ago
|
Flags: needinfo?(mh+mozilla)
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1d521fcbb0d
move library/object prefix/suffix configuration to moz.configure; r=mshal
Assignee | ||
Comment 6•7 years ago
|
||
I pushed the patch with the (simple) expandlibs.m4 changes; removing EXPAND_LIBS_ORDER_STYLE and associated code can be done in a follow-up bug. Would still be interested in glandium's thoughts, though.
Comment 7•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 8•7 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #4)
> I'm not sure - it doesn't look like SYMBOL_ORDER was set anywhere in tree by
> the patch from bug 603370 - maybe it was intended to be set externally for
> testing, or by buildbot or something? glandium, can you comment on #c3?
FWIW, it was meant to be set from the results of profiling, but that never really happened.
Flags: needinfo?(mh+mozilla)
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•