Closed
Bug 1402012
Opened 7 years ago
Closed 7 years ago
Don't rebuild world when configure output is unchanged
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox57 wontfix, firefox58 fixed)
RESOLVED
FIXED
mozilla58
People
(Reporter: mshal, Assigned: mshal)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
This is a subset of bug 902825, which I'm splitting out since this is the more straightforward part of the patchset and gives us the benefit of not rebuilding world when the configure output is unchanged. The rest of the work to only rebuild certain subsections of the code when a configure feature flag is changed will be finished in bug 902825.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
See https://bugzilla.mozilla.org/show_bug.cgi?id=902825#c104 and https://bugzilla.mozilla.org/show_bug.cgi?id=902825#c105 for some comments on the last round of reviews in that bug.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
Updated to avoid mentioning mozconfig_defines in a comment since those don't exist yet.
Updated•7 years ago
|
status-firefox57:
--- → wontfix
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8910814 [details] Bug 1402012 - Create config.statusd directory; https://reviewboard.mozilla.org/r/182290/#review189062 ::: python/mozbuild/mozbuild/backend/configenvironment.py:305 (Diff revision 2) > + try: > + return self[key] is not None > + except KeyError: > + return False > + > + def __iter__(self): it's weird that this does the equivalent of dict.iteritems, not dict.iter. Better name it iteritems. ::: python/mozbuild/mozbuild/backend/configenvironment.py:349 (Diff revision 2) > + substs = config['substs'].copy() > + defines = config['defines'].copy() > + > + global_defines = [ > + name for name in config['defines'] > + if name not in config['non_global_defines'] fwiw, I think we can kill non global defines. ::: python/mozbuild/mozbuild/backend/configenvironment.py:361 (Diff revision 2) > + self.substs._write_group(substs) > + self.defines._write_group(defines) maybe fill_group or fill would be better than write_group.
Attachment #8910814 -
Flags: review?(mh+mozilla) → review+
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8910815 [details] Bug 1402012 - Use PartialConfigEnvironment in process_define_files.py; https://reviewboard.mozilla.org/r/182292/#review189066
Attachment #8910815 -
Flags: review?(mh+mozilla) → review+
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8910816 [details] Bug 1402012 - Update buildconfig.py to use PartialConfigEnvironment; https://reviewboard.mozilla.org/r/182294/#review189072
Attachment #8910816 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 12•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8910814 [details] Bug 1402012 - Create config.statusd directory; https://reviewboard.mozilla.org/r/182290/#review189062 > it's weird that this does the equivalent of dict.iteritems, not dict.iter. Better name it iteritems. We discussed this in IRC, but I've renamed this to iteritems() and now call iteritems explicitly where buildconfig.defines / substs are used in dict.update() calls. Internally Python has some special-casing for how dicts are handled in these cases, and it seems we can't replicate it exactly by overriding methods, even if we subclass dict. I also noticed the function was using the full path as the key, which magically worked because __getitem__ still finds the file (the mozpath.join just keeps the full path). I've updated this to use the basename so we don't end up with weird keys in the dict.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
Pushed by mshal@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d1d46bec63ce Create config.statusd directory; r=glandium https://hg.mozilla.org/integration/autoland/rev/22ebbb5d30d4 Use PartialConfigEnvironment in process_define_files.py; r=glandium https://hg.mozilla.org/integration/autoland/rev/cb773c661e0c Update buildconfig.py to use PartialConfigEnvironment; r=glandium
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d1d46bec63ce https://hg.mozilla.org/mozilla-central/rev/22ebbb5d30d4 https://hg.mozilla.org/mozilla-central/rev/cb773c661e0c
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
•