Move tup invocation out of Makefile.in and into build mach command

RESOLVED FIXED in Firefox 61

Status

enhancement
RESOLVED FIXED
Last year
8 months ago

People

(Reporter: cmanchester, Assigned: cmanchester)

Tracking

(Blocks 1 bug)

3 Branch
mozilla61
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(2 attachments)

This will fix a few papercuts we've encountered so far
Attachment #8968704 - Flags: review?(mshal)
The other thing Makefile.in is giving us is the build-backend rebuild detection, which is still quite useful.
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 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+
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.
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.
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
https://hg.mozilla.org/mozilla-central/rev/dee93aadc721
https://hg.mozilla.org/mozilla-central/rev/fc793911d371
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.