1717104 - Only make vendored Python packages available if requested by the virtualenv

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Mitchell Hentges [:mhentges] 🦀]

Currently, all third-party Python modules are placed into the sys.path, even if they're not wanted by the virtualenv. This can cause collisions with pypi modules, which we want to guard against.

Unfortunately, one of the pieces that will make this trickier is that we aren't lazy-loading mach commands yet. So, existing mach commands will need to be lazy-loading (do imports at the function level, rather than file) at the command level, which is going to be some up-front busywork. This is required so that mach commands don't try to import 3rd-party libraries even though the commands aren't being run yet.

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1717104: Remove obsolete "set-variable" notes from virtualenv docs

Since https://phabricator.services.mozilla.com/D115921, "set-variable"
is no longer a valid action. Looks like that patch forgot to remove the
associated docs

Comment 2

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1717104: Update "pth" docs for virtualenvs

The "pth" action no longer has a customizable filename, and the docs
should be updated accordingly.

Depends on D118609

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4708c9518cf7
Remove obsolete "set-variable" notes from virtualenv docs r=ahal
https://hg.mozilla.org/integration/autoland/rev/ff3541181f8a
Update "pth" docs for virtualenvs r=ahal

Comment 4

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/4708c9518cf7
https://hg.mozilla.org/mozilla-central/rev/ff3541181f8a

Comment 5

[Mitchell Hentges [:mhentges] 🦀]

We should prune the mach virtualenv to remove dependencies only used in commands (and not at the global Mach level) once this work is done.

Comment 6

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1717104: Align virtualenv_name with requirements definition

Maps virtualenvs one-to-one with their associated requirements
definition.

For example:

Name: "docs"
Virtualenv location: <snip>/_virtualenvs/docs
Requirements location: $topsrcdir/build/docs_virtualenv_packages.txt

An issue to be resolved in the future is that it's tricky to know that,
when you define a new virtualenv, you have to know that a requirements
definition needs to exist in "build/".

As part of this change, the default virtualenv changed from
build to common. Since some python-tests depend on glean_parser,
the python-test virtualenv was modified to still include it.
This addition to the python-test virtualenv is temporary and
will be removed when bug 1724273 is resolved.

Depends on D122890

Comment 7

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1717104: Activate virtualenv before running command

All commands now have their virtualenv activated before they're invoked.
If they haven't explicitly specified a virtualenv, then they use the one
named "common".

Removes all now-redundant manual activations of virtualenvs.

Tweaks some tests that include Mach command invocations to
skip virtualenv activation to avoid parallel process
race conditions.

Depends on D122891

Comment 8

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1717104: `VirtualenvManager` should not allow mixing pythons

Tweak the VirtualenvManager API to accept a single base "python"
executable during initialization, then to consistently use it for
up-to-date checking, construction, etc.

This constraint allows future simplification for behaviour involving th
"base" python.

Depends on D122893

Comment 9

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1717104: Encode `virtualenv_name` into venv's directory

Adds a MozVirtualenvMetadata file abstraction that allows us to:

• Statically determine if a virtualenv is managed by Mach.
• Determine the virtualenv's name.

Note: I called this file moz_virtualenv_metadata.json instead of
mach_virtualenv_metadata.json because I didn't want to be ambiguous
between "the Mach virtualenv" and "a Mach-managed virtualenv".

Depends on D124516

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ec8241c2e1a
Align virtualenv_name with requirements definition r=ahal
https://hg.mozilla.org/integration/autoland/rev/66d5610bfb64
`VirtualenvManager` should not allow mixing pythons r=ahal
https://hg.mozilla.org/integration/autoland/rev/abdc8408e9e6
Encode `virtualenv_name` into venv's directory r=ahal

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9ec8241c2e1a
https://hg.mozilla.org/mozilla-central/rev/66d5610bfb64
https://hg.mozilla.org/mozilla-central/rev/abdc8408e9e6

Comment 12

[Steven Newbury]

This seems to have broken MACH_USE_SYSTEM_PYTHON=1 on Linux, at least when building on Gentoo.

The "common" virtualenv doesn't get set up, so it fails similar to 1731024 but for "./mach build --verbose"

Error running mach:

['build', '--verbose']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file build| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

FileNotFoundError: [Errno 2] No such file or directory: '/var/tmp/portage/www-client/firefox-9999/work/firefox_build/instrumented/_virtualenvs/common/bin/python'

Comment 13

[Mitchell Hentges [:mhentges] 🦀]

Steven, I can't reproduce that issue, can you share the associated stack trace?
When I do a ./mach build --verbose, the common virtualenv isn't used.

Comment 14

[Steven Newbury]

Curious. Could that be affected by using PGO? I'm building my own nightly from a modified portage ebuild.

Specifically, what gets called is "virtx ./mach build --verbose"

It was working fine up until a few days ago. It's definitely trying to use the common virtualenv as you can see above.

Comment 15

[Mitchell Hentges [:mhentges] 🦀]

Perhaps it's related to PGO - can you create a new bug, mark this one as "regressing" it, and attach your full build logs? I need to see the stack trace to continue investigation here.

Comment 16

[Steven Newbury]

Regressions: 1732177

Comment 17

[Steven Newbury]

Mitchell, I don't see how to mark the bug a regressed by this bug. Obviously just writing it via a comment isn't the way...

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/720a528d4868
Activate virtualenv before running command r=perftest-reviewers,ahal,AlexandruIonescu

Comment 19

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/720a528d4868

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:ahochheiden, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.

Comment 21

[Alex Hochheiden [:ahochheiden]]

As far as I can tell, everything that needed to be done for this has landed. I'll close this now.

If something was overlooked we can always raise a new bug.