Closed
Bug 1256604
Opened 8 years ago
Closed 7 years ago
Get rid of toolkit/content/Makefile.in
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: ted, Assigned: chmanchester)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
Details |
Right now toolkit/content/Makefile.in exists simply to set CXXFLAGS/CPPFLAGS in DEFINES for buildconfig.html: https://hg.mozilla.org/mozilla-central/file/5e14887312d4523ab59c3f6c6c94a679cf42b496/toolkit/content/Makefile.in I guess this is stuck here because the full value of $CXXFLAGS isn't computable without config.mk currently.
Reporter | ||
Comment 1•7 years ago
|
||
This is not super critical, in that it is just for about:buildconfig, but chmanchester: can we make this work more sensibly now with your CFLAGS work?
Assignee | ||
Comment 2•7 years ago
|
||
Yep, after bug 1403346 we should be able to do this from moz.build.
Depends on: 1403346
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → cmanchester
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8926238 [details] Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build https://reviewboard.mozilla.org/r/197494/#review202932 No flag change just to revisit this, or get someone who knows more to look at it. Technically this looks fine. But I don't understand a few things: - Why are the original `CXXFLAGS` and `CPPFLAGS` not in the new set? - We have some weird things happening in the tree, like http://searchfox.org/mozilla-central/source/browser/app/moz.build#118. Does this global setting even make sense in the wild? ::: toolkit/content/moz.build:14 (Diff revision 1) > for var in ('target', 'MOZ_CONFIGURE_OPTIONS', 'CC', 'CC_VERSION', 'CXX'): > DEFINES[var] = CONFIG[var] > > DEFINES['CFLAGS'] = ' '.join(CONFIG['OS_CFLAGS']) > > +cxx_flags = [] How do we make sure this set doesn't bit-rot as new places to stash flags are added?
Assignee | ||
Comment 5•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8926238 [details] Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build https://reviewboard.mozilla.org/r/197494/#review202932 The build backend hasn't actually set `CPPFLAGS` for a while, configure takes `CPPFLAGS` and exposes them as `OS_CPPFLAGS`. `CXXFLAGS` isn't used by the backend anymore (it was a little weird, it never contained all the flags, but imposed some mostly irellevant order between flags sets), the flags added in this patch reflect what used to be in `CXXFLAGS`. > How do we make sure this set doesn't bit-rot as new places to stash flags are added? We can either expose all the flags, potentially a larger set than was here originally, or make a case-by-case decision, and do our best not to add categories of flags, what we have already is probably more than necessary.
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8926238 [details] Bug 1256604 - Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build https://reviewboard.mozilla.org/r/197494/#review203406 ::: toolkit/content/buildconfig.html:53 (Diff revision 1) > <td>@CFLAGS@</td> > </tr> > <tr> > <td>@CXX@</td> > <td>@CC_VERSION@</td> > #ifndef BUILD_FASTER You can remove this #ifndef (and fixup this comment: https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/config/faster/rules.mk#63) since this now builds in the faster make backend.
Attachment #8926238 -
Flags: review+
Updated•7 years ago
|
Attachment #8926238 -
Flags: review?(core-build-config-reviews)
Comment hidden (mozreview-request) |
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7908ec3fc4af Remove toolkit/content/Makefile.in and move setting CXXFLAGS for buildconfig.html to moz.build r=mshal
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7908ec3fc4af
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•