901601 - Hook client.mk/toplevel make build up to mozbuild/mach Python logic

Status: NEW

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

Currently the code paths for building between |mach build| and client.mk/make are rather divergent. While client.mk has been feature stagnant, mach has added compiler warning collection, tracking of build progress, Finder CPU usage, and soon overall system resource usage (bug 883209).

I think the time has come to hook client.mk and a top-level |make| in the root Makefile up to the new Python code so they can realize these features.

I believe this can be accomplished without introducing some of the suboptimal bits of mach into the core build system, notably the undesirable prefixing of output lines with times and some issues around environment resolution.

I was thinking we could move some of the build logic from mach_commands.py into a standalone module/script then we could just modify client.mk, the root makefile, and mach to invoke that script/module.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Part 1: Move logic for invoking build backend into mozbuild package

This patch is 80% hoisting code from mozbuild's mach_commands.py into
mozbuild.controller.building. The other 20% is refactoring the interface
to invoking builds to make it more consumable outside of mach. A
mozbuild API to invoke the build backend has been added. This API allows
listeners to be registered and to receive callbacks at certain phases of
the build (e.g. compiler warning detected, tier traversal update, etc).

In part 2, I will hook up client.mk and the root Makefile's default
target to this new code.

The part of this patch I like the least is hackiness around mach's
logging. Someone (likely me) *really* needs to make the log manager suck
less. Maybe I can find someone to add desired functionality to mozlog
and we can port mach to use it...

Comment 2

[Mike Hommey [:glandium]]

I thought creating an incentive to use mach instead of client.mk was a motivation to keep client.mk stagnant.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Part 2: client.mk now invokes Python build wrapper

client.mk now invokes the Python code when building the tree.

I stopped short of updating the root Makefile.in because I'm having
second thoughts. To do it right would require us to inject ourselves
into every Makefile/target. I want to think we should do that with tools
like mach rather than in rules.mk.

https://tbpl.mozilla.org/?tree=Try&rev=cae302114896

Comment 4

[Gregory Szorc [:gps]]

(In reply to Mike Hommey [:glandium] from comment #2)
> I thought creating an incentive to use mach instead of client.mk was a
> motivation to keep client.mk stagnant.

It is. However, getting automation to use new awesomeness will require automation to not use client.mk. That's a huge effort and one I don't want to block rolling out resource monitoring to automation.

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 786053 [details] [diff] [review]
Part 2: client.mk now invokes Python build wrapper

And this broke aspects of |mach build|. The irony.

Comment 6

[Gregory Szorc [:gps]]

Attachment: Part 2: client.mk now invokes Python build wrapper

Unbusted |mach build|. The issue was |mach build| invokes |make -f
client.mk| which was invoking the Python build logic. The 2nd Python
build code was swallowing the BUILDSTATUS output. Furthermore, both
Python processes were analyzing output for compiler warnings, etc. This
was inefficient. So, I changed client.mk to look for an environment
variable indicating we're already in Python mode and to invoke make
directly if we are.

https://tbpl.mozilla.org/?tree=Try&rev=b98481407131

Comment 7

[Justin Wood (:Callek)]

(In reply to Gregory Szorc [:gps] from comment #4)
> (In reply to Mike Hommey [:glandium] from comment #2)
> > I thought creating an incentive to use mach instead of client.mk was a
> > motivation to keep client.mk stagnant.
> 
> It is. However, getting automation to use new awesomeness will require
> automation to not use client.mk. That's a huge effort and one I don't want
> to block rolling out resource monitoring to automation.

Personally I'm in favor of making automation use mach when 'we' feel its ready, in a proper rather than abstract weird wrapper way is a FAR better design.

Many of the prior eng-related concerns [I have had] with what mach was doing automatically/required/etc was brushed off with the "we can revisit when we are ready to switch automation to use it" so I'd _much_ rather have automation switching to mach be a deliberate and intentional thing tested with releng, than a wrapper landed into the build system and automatically enforced/hope-for-the-best.

(I note I don't remember half of the concerns I had at the moment, so if we are ready to switch to it, I think we can visit them ~now, but I do on a personal level think this approach is more complication for little gain)

Comment 8

[Gregory Szorc [:gps]]

I agree with the sentiment that we should just switch things to mach. I would love to do this instead of bastardizing client.mk to invoke a cheap mach knock-off.

However, that would be considerable effort for a number of people.

We'd have to reinvent client.mk in Python or similar. This is a lot of work.

We'd have to track down remaining bugs in mach and implement features people have been screaming for. Worth doing, sure. But these aren't pressing concerns and IMO not worth the effort at this juncture.

We'd have to coordinate with RelEng over changing core build actions at the time they are busy converting those same configs from buildbot to mozharness. I'd rather stay out of RelEng's way until their work is done.

We'd have to spend a lot of time post-land tracking conducting fire drills and explaining to people why the build system stopped working (because of a subtle difference between mach and client.mk - such as resolving objdirs or mozconfigs).

All this work would distract us from our Q3 goals. In the end, it doesn't buy us much. This patch, by moving most of the |mach build| logic into a standalone Python module, allows us to gain 80% of the benefits of mach with a client.mk build without being fully exposed to all of mach's known pitfalls and without requiring RelEng to change a thing. There is a chance we may break client.mk for some people. But from my experience the people complaining about mach's lack of features are "power users" who don't use client.mk.

(In reply to Justin Wood (:Callek) from comment #7)
> Personally I'm in favor of making automation use mach when 'we' feel its
> ready, in a proper rather than abstract weird wrapper way is a FAR better
> design.
> 
> Many of the prior eng-related concerns [I have had] with what mach was doing
> automatically/required/etc was brushed off with the "we can revisit when we
> are ready to switch automation to use it" so I'd _much_ rather have
> automation switching to mach be a deliberate and intentional thing tested
> with releng, than a wrapper landed into the build system and automatically
> enforced/hope-for-the-best.

This bug *is* about having automation use many of the previously mach-only Python bits of the build. That decision is implicit in an r+ of part 2. We test things by performing try pushes. If it works, it works. That's now the build configuration. If it breaks something down the line or developer workflows, we back out part 2 until things are fixed.

I view this bug as the initial step of "have automation builds use mach." The current patch just happens to bypass mach's command dispatch logic (along with things like the build progress terminal footer) and goes straight to the guts.

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 785994 [details] [diff] [review]
Part 1: Move logic for invoking build backend into mozbuild package

We're going to land bug 883209 before this.