Closed
Bug 1457162
Opened 6 years ago
Closed 6 years ago
EXPAND_LIBS_LIST_STYLE checks do not include CFLAGS
Categories
(Firefox Build System :: General, enhancement)
Tracking
(firefox62 fixed)
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: tjr, Assigned: tjr)
References
Details
Attachments
(2 files, 1 obsolete file)
In the initial conftest.c compilation, we include $CFLAGS: https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/build/autoconf/expandlibs.m4#17 However in the subsequent list checks we do not include CFLAGS (e.g. https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/build/autoconf/expandlibs.m4#19) If CFLAGS contains something that would check the object file output (like, say -fuse-ld=lld) - then all of the list checks will fail.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8971219 -
Flags: review?(ted)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8971219 -
Attachment is obsolete: true
Attachment #8971219 -
Flags: review?(ted)
Updated•6 years ago
|
Attachment #8971665 -
Flags: review?(ted) → review?(nfroyd)
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8971665 [details] Bug 1457162 Include CFLAGS and CPPFLAGS during the EXPAND_LIBS_LIST_STYLE check https://reviewboard.mozilla.org/r/240434/#review246698
Attachment #8971665 -
Flags: review?(nfroyd) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b51c2e39a12a Include CFLAGS and CPPFLAGS during the EXPAND_LIBS_LIST_STYLE check r=froydnj
Keywords: checkin-needed
Comment 6•6 years ago
|
||
Backed out for build bustages Pus that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b51c2e39a12a52fe7310f9dfa21e51e61fa48105 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=178771796&repo=autoland&lineNumber=30080 Backout: https://hg.mozilla.org/integration/autoland/rev/54d57adb53fb89d574f515f97a1b024c8fc3641f
Flags: needinfo?(tom)
Assignee | ||
Comment 7•6 years ago
|
||
Hm. So I'm not sure why; but this patch breaks the _Windows_ builds and causes them to select the list file type 'none' instead of their normal 'list'. I'll have to investigate this.
Flags: needinfo?(tom)
Assignee | ||
Comment 8•6 years ago
|
||
So I traced this problem down to the fact that on Windows we add -TC to CFLAGS which forces the object file to be treated as a C file.
Assignee | ||
Comment 9•6 years ago
|
||
I have a successfull full try run just by removing -TC which doesn't seem to be required (at least based on TaskCluster output)...
Assignee | ||
Comment 10•6 years ago
|
||
Forgot link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7cc5c8f0de93e4be716a22c143363af85b22ec1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•6 years ago
|
||
mozreview-review |
Comment on attachment 8979089 [details] Bug 1457162 Remove -TC from CFLAGS https://reviewboard.mozilla.org/r/245342/#review251490 ::: js/src/old-configure.in:147 (Diff revision 1) > if test "$GCC" != "yes"; then > # Check to see if we are really running in a msvc environemnt > _WIN32_MSVC=1 > > # Make sure compilers are valid > - CFLAGS="$CFLAGS -TC -nologo" > + CFLAGS="$CFLAGS -nologo" I doubt this will harm anything (as evidenced by your try push), but it does give us a weird asymmetry with `CXXFLAGS` still including `-TP` below.
Attachment #8979089 -
Flags: review+
Updated•6 years ago
|
Attachment #8979089 -
Flags: review?(core-build-config-reviews)
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8971665 [details] Bug 1457162 Include CFLAGS and CPPFLAGS during the EXPAND_LIBS_LIST_STYLE check https://reviewboard.mozilla.org/r/240434/#review251488 I really think at this point we should just drop all these configure tests and hardcode the list style based on the linker we know is in use, but this is fine to unblock the work you're doing.
Attachment #8971665 -
Flags: review?(ted) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 15•6 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/bf7ac536eb94 Remove -TC from CFLAGS r=ted https://hg.mozilla.org/integration/autoland/rev/a77d15466672 Include CFLAGS and CPPFLAGS during the EXPAND_LIBS_LIST_STYLE check r=froydnj,ted
Keywords: checkin-needed
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bf7ac536eb94 https://hg.mozilla.org/mozilla-central/rev/a77d15466672
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•5 years ago
|
Version: Version 3 → 3 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•