Closed
Bug 1454771
Opened 6 years ago
Closed 6 years ago
Move tup invocation out of Makefile.in and into build mach command
Categories
(Firefox Build System :: General, enhancement)
Tracking
(firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: chmanchester, Assigned: chmanchester)
References
Details
Attachments
(2 files)
This will fix a few papercuts we've encountered so far
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8968704 -
Flags: review?(mshal)
Assignee | ||
Comment 2•6 years ago
|
||
The other thing Makefile.in is giving us is the build-backend rebuild detection, which is still quite useful.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8968704 [details] Bug 1454771 - Move tup invocation from Makefile.in to mach. https://reviewboard.mozilla.org/r/237402/#review245392 Looks great overall! Just a few small issues to address before landing, but I don't think they will require re-review. ::: python/mozbuild/mozbuild/backend/tup.py:228 (Diff revision 4) > > self._built_in_addons = set() > self._built_in_addons_file = 'dist/bin/browser/chrome/browser/content/browser/built_in_addons.json' > > + def build(self, config, output, jobs, verbose, what=None): > + if what is None: It seems 'what' is never None, but an empty list (maybe because it is an nargs='*' argument?), so we don't actually build the objdir by default. ::: python/mozbuild/mozbuild/backend/tup.py:235 (Diff revision 4) > + args = [self.environment.substs['TUP'], 'upd'] + what > + if self.environment.substs.get('MOZ_AUTOMATION'): > + args += ['--quiet'] > + if verbose: > + args += ['--verbose'] > + return config.run_process(args=args, We should also pass the jobs argument in (using '-j', just like make) with some logic like this: https://dxr.mozilla.org/mozilla-central/rev/26e53729a10976f52e75efa44e17b5e054969fec/python/mozbuild/mozbuild/base.py#616 ::: python/mozbuild/mozbuild/controller/building.py:1021 (Diff revision 4) > + > + if config is None: > - if what: > + if what: > - top_make = os.path.join(self.topobjdir, 'Makefile') > - if not os.path.exists(top_make): > print('Your tree has not been configured yet. Please run ' I think we can get rid of this error, since 'what' isn't used until after configure runs anyway. So if you have no objdir, './mach build foo' should work fine if the check is removed. ::: python/mozbuild/mozbuild/controller/building.py:1035 (Diff revision 4) > + config = self.config_environment > + > + status = maybe_invoke_backend(config.substs.get('BUILD_BACKENDS', > + [None])[0]) > > + if what and status is None: How hard would it be to move this whole if-block into the make backend's build() method? It can be done in a followup - just curious what the potential hazards are.
Attachment #8968704 -
Flags: review?(mshal) → review+
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8969670 [details] Bug 1454771 - Add mozconfig variables to the environment when running tup. https://reviewboard.mozilla.org/r/238472/#review245424
Attachment #8969670 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 9•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8968704 [details] Bug 1454771 - Move tup invocation from Makefile.in to mach. https://reviewboard.mozilla.org/r/237402/#review245392 > It seems 'what' is never None, but an empty list (maybe because it is an nargs='*' argument?), so we don't actually build the objdir by default. I'll change this to `if not what:` > How hard would it be to move this whole if-block into the make backend's build() method? It can be done in a followup - just curious what the potential hazards are. It would probably be relatively simple, but we'd have to figure out how to propogate errors correctly. This whole function is pretty big and messy and should probably be re-written at some point.
Assignee | ||
Comment 10•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8968704 [details] Bug 1454771 - Move tup invocation from Makefile.in to mach. https://reviewboard.mozilla.org/r/237402/#review245392 > I think we can get rid of this error, since 'what' isn't used until after configure runs anyway. So if you have no objdir, './mach build foo' should work fine if the check is removed. It's a slight behavior change, but yeah, I'll just get rid of it.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•6 years ago
|
||
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dee93aadc721 Move tup invocation from Makefile.in to mach. r=mshal https://hg.mozilla.org/integration/autoland/rev/fc793911d371 Add mozconfig variables to the environment when running tup. r=mshal
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dee93aadc721 https://hg.mozilla.org/mozilla-central/rev/fc793911d371
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
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
•