1808732 - Move vendored packages out of the `mach` site

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, task, P3)

Description

[Andrew Halberstadt [:ahal]]

Background

There's a newish feature wherein mach commands can define a custom virtualenv, e.g like this. This allows them to define custom dependencies in the corresponding file under python/sites, for example. These dependencies can be from pypi, the vendored dir or even just add a directory to the PYTHONPATH.

All commands inherit the packages in the mach.txt site. This is supposed to be the set of packages that are needed for Mach core itself. However it also includes all of the vendored packages under third_party/python. I am pretty sure we did this because most Mach commands just assume these packages are available in their import scope, and it was easier to maintain a backwards compatible migration path if this was still the case in the new system.

However! The problem now is that all sites must remain compatible with the mach site. For example, if a site wants to use the latest version of requests, it can't because the vendored requests is pinned to an older version and listed in python/sites/mach.txt.

So to allow commands to use newer versions of vendored packages and reduce the number of compatibility errors in general, we should remove as much as possible from the mach.txt site, such that it contains only the packages necessary to run mach itself.

There are two approaches we could take here, the easy one and the one that will minimize conflicts even further.

The Easy Approach

1. First we move all vendor and pth dependencies out of the mach site (unless it is actually needed by mach), and into the common site (which is currently empty).
2. Then, for every command that needs to access the vendored packages, we ensure that it either has virtualenv_name=common in the Mach command definition, OR if it already is using a site, we add:

packages.txt:python/sites/common.txt

to it.

The Fewer Conflicts Approach

The easy approach should work, but might still result in conflicts. Commands need to either be compatible with all vendors, or not use them at all with no in-between. So instead of putting everything in the common site, we could go through each command one by one, determine exactly which vendors (if any) are needed, and then create a custom site just for that command which lists exactly the vendored libraries that it needs. Of course, related commands could still share a site if their set of dependencies are very similar.

In addition to having fewer conflicts, this approach would force us to explicitly register which packages are being used. While it might be a bit more work, I think this is very valuable when trying to determine which vendors are being used by which commands, and what needs to be tested when updating them!

Recommendation

Given all of the above, I'd recommend we first implement the easy approach just to get the ball rolling and get all commands onto the new "sites" system. Once we have that, I'd recommend we spend the effort going through all commands and figuring out exactly what is needed for each one. I think this will pay big dividends over the long run.

Comment 1

[Andrew Halberstadt [:ahal]]

I took a stab at this and it might be blocked on bug 1695312. I think the list of packages in mach.txt might actually be what it currently needs. This is because when you invoke mach, it'll import every mach_commands.py file which means that each of the imports at the top of those files must be importable from the mach scope.

So we need to get to the point where we can run mach without importing every single mach_commands.py file. Note that there are excellent reasons for us to do this anyway, e.g much less of a performance hit when running mach.

Comment 2

[Alex Hochheiden [:ahochheiden] (needinfo? me)]

(In reply to Andrew Halberstadt [:ahal] from comment #1)

I think the list of packages in mach.txt might actually be what it currently needs.

That's my understanding. Though, we don't have anything that proves that the list of dependencies in there is the current 'minimal' set. At one point or another, all of the dependencies listed in there were required for baseline mach, but there's no easy way to tell if something changed and any particular dependency is or is no longer required.

(In reply to Andrew Halberstadt [:ahal] from comment #0)

However it also includes all of the vendored packages under third_party/python.

I don't think this is 100% correct. Most things from third_party/python are included in mach.txt, but not all. Some of the packages specified in build.txt and python-tests.txt are in third_party/python, but are not included in mach.txt.

Comment 3

[Andrew Halberstadt [:ahal]]

Thanks for the clarification, that's my new understanding as well. To be extra clear, I don't think those things are needed by Mach core, but are only there due to the fact that we import the world on every Mach invocation.

Comment 4

[Alex Hochheiden [:ahochheiden] (needinfo? me)]

Attachment: Bug 1808732 - Move various dependencies out of the `mach` site to either the specific site they're needed in (like `build, or `lint`), and/or to `common` r?#build

Now that we selectively load command modules, and activate the command
virtualenv much earlier in the mach process, a lot of the module
dependencies specified in mach.txt are no longer necessary there. With
their removal from mach.txt they will no longer be automatically
inherited by every site, which reduces potential dependency conflicts
for specific sites.

The common site still effectively has the same set of dependencies.
This is the default site that all commands use unless otherwise
specified. Most commands use this site, and going through every command
and seeing if a dependency is or isn't needed, then deciding if or if
not to create a new site for that command made sense was too time
consuming to do here.

Essentially the idea here going forward is that if you're trying to
add/update a new dependency to a command that is currently defaulting to
the common site and there is a conflict with one of the dependencies
in common you can move your command to a new site specifically for
your command, and you will have the minimal possible set of dependencies
a mach command can have, improving the odds that you can add the
module(s) you need for your command.

Depends on D180500

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0413a48d4f03
Move various dependencies out of the `mach` site to either the specific site they're needed in (like `build, or `lint`), and/or to `common` r=firefox-build-system-reviewers,glandium

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing lint failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/3ae2f464a14e6453ce1d7402edfe00be45d2c781

Push with failures

Failure log -> Exception: Cannot use MACH_BUILD_PYTHON_NATIVE_PACKAGE_SOURCE="system" for any sites other than ('mach', 'build', 'common'). The current attempted site is "vendor"

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49a85b3b07e0
Move various dependencies out of the `mach` site to either the specific site they're needed in (like `build, or `lint`), and/or to `common` r=firefox-build-system-reviewers,glandium

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/49a85b3b07e0

Comment 9

[Natalia Csoregi [:nataliaCs]]

Backed out as per request on Element, #sheriffs.

• backout: https://hg.mozilla.org/mozilla-central/rev/7d797f3003c61fdc3b0528c2ba54518f531144a3

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1084bcb2a255
Move various dependencies out of the `mach` site to either the specific site they're needed in (like `build, or `lint`), and/or to `common` r=firefox-build-system-reviewers,glandium

Comment 11

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/1084bcb2a255

Comment 12

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/1084bcb2a255