Transparently manage Mach virtualenv at run-time
Categories
(Firefox Build System :: Mach Core, enhancement, P2)
Tracking
(firefox96 fixed)
Tracking | Status | |
---|---|---|
firefox96 | --- | fixed |
People
(Reporter: mhentges, Assigned: mhentges)
References
(Blocks 1 open bug, Regressed 1 open bug)
Details
Attachments
(17 files, 3 obsolete files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
- There are pieces of the
mach
virtualenv which are needed for specific commands (zstandard
,psutil
). Move these into their associated command virtualenv. - Instead of having a separate
./mach create-mach-environment -f
, manage themach
virtualenv more transparently:- Bootstrap Mach by start with just vendored code, build the Mach virtualenv, install native dependencies, activate it, then continue running Mach.
- Automatically re-create the venv it if we're using a different Python version
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
The three removed paths don't exist in-repo, and after a cursory glance
they don't appear to be populated dynamically.
Note that the removal of the six
path for WPT is different: it's
technically just incorrect, and should amended to point to
$WPT/tools/third_party/six
. However, Python only allows a single
instance of a library to exist in import scope, and we're already
consuming six
from the Firefox-wide vendored 3rd-party libs.
Depends on D119687
Assignee | ||
Comment 2•3 years ago
|
||
1st-party and 3rd-party module references are two different concerns.
This is most clear when considering that, for any given revision, every
command is going to want the 1st-party modules contained in that
revision - they (shouldn't) ever want, say, mozversioncontrol
from
last year.
However, it's completely viable for two different commands to depend on
different (perhaps even conflicting) versions of a third_party module
(such as six
).
So, we centralize the definition of 1st-party modules accordingly.
Depends on D119825
Assignee | ||
Comment 3•3 years ago
|
||
_install_pip_package()
may be run from populate()
, which
is invoked from a child Python process that doesn't
have in-tree Python modules in its sys.path.
An alternate solution would be to add in-tree modules
to the sys.path, but that seemed more costly than
simply using tempfile
and shutil
.
Depends on D119826
Assignee | ||
Comment 4•3 years ago
|
||
After removing optional
in Bug 1712804, we need to add a variant back
here because there's fallible dependencies. However, I've tweaked the
re-introduction of the feature to require a specific repercussion
message as well. This seemed like a decent tradeoff - the developer
becomes aware that the failure is bad, it has repercussions, but it's
not a blocking issue. Additionally, since we're printing pip's output,
the developer will be able to see the underlying error causing the
warning.
I also added comment functionality to requirements definitions to allow
adjacent documentation of why some requirements are fallible. (Related:
I'm looking forward to mach_bootstrap
not needing to parse
requirements definitions. Almost there!)
Finally, in preparation for review: I didn't make
FalliblePypiSpecifier
extend PypiSpecifier
because I figured that
the benefit of flexibility (easier to allow implementations to diverge
without needing to untangle an inheritance relationship) was larger than
the cost of needing to add properties to both specifiers.
If we wanted re-use, I'd probably have FalliblePypiSpecifier
contain
a PypiSpecifier
, but then you have to reach deeper into the object to
get data, so shrug.
Depends on D119834
Assignee | ||
Comment 5•3 years ago
|
||
Assignee | ||
Comment 6•3 years ago
|
||
Assignee | ||
Comment 7•3 years ago
|
||
Attached some images (originally diagrammed for this patch) to help illustrate the target of this bug.
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 8•3 years ago
|
||
It was in nativecmds
to make finding pip3
easier.
However, since we're removing the distinction between "regular" and
"native" cmds (Mach will create and activate its venv naturally during
initialization), we need to start pruning the nativecmds
list.
Depends on D119835
Assignee | ||
Comment 9•3 years ago
|
||
It was originally added to "create-mach-environment" because that would
eventually be called by "./mach bootstrap".
However, as "create-mach-environment" is going away, this moves it
closer to the command that it's meant to impact.
Depends on D120398
Assignee | ||
Comment 10•3 years ago
|
||
We had logic in both mach_bootstrap
and the Mach Bootstrapper to
create the state_dir.
This joins them and has the added benefit of creating the state dir
earlier in the Mach lifecycle (as will be needed for early instantiation
of the Mach virtualenv).
Depends on D120399
Assignee | ||
Comment 11•3 years ago
|
||
TODO
Depends on D120400
Assignee | ||
Comment 12•3 years ago
|
||
Now that we no longer patch sys.path when using the Mach virtualenv,
the populate_local_paths
option is no longer ever False
. So, it can
be inlined as True
.
Depends on D120401
Assignee | ||
Comment 13•3 years ago
|
||
pkg_resources
parses the environment state when it's first imported.
However, when we activate an existing virtualenv, pkg_resources
isn't
automatically updated.
The current symptom of this is that the PyCharm debugger causes failures
when a virtualenv boundary is passed, because it imports pkg_resources
very early in the debugging lifecycle.
Depends on D120402
Assignee | ||
Comment 14•3 years ago
|
||
Warning: this solution may not be stable as more dependencies are added.
However, at the moment, it seems like the best set of tradeoffs.
Glean creates a child Python process that needs to be able to import glean
. So, it creates that process from sys.executable
with the
assumption that "sys.executable
has the current Glean module imported,
so it must be able to do it again."
However, sys.executable
is not changed when dynamically activating
virtualenvs from an existing Python process. So, without this change,
sys.executable
is the system Python, which isn't guaranteed to have
Glean.
Unfortunately, this solution only works because we currently only have
one dependency using sys.executable
. If we have this case in the
future with a dependency in a command virtualenv, then this solution
won't work anymore.
This leads us to an alternative solution (which isn't currently
adopted): we could install all Mach-level dependencies into every
command virtualenv. This has some benefits:
- Any dependency of Mach or any command can start subprocesses from
sys.executable
, and it'll always run from a virtualenv with that
dependency installed. - We're already going to have to validate that the dependencies of
commands don't conflict with Mach dependencies. The current planned
solution is to do that validation only in CI, but this would simplify
that situation. - The
virtualenv
developers have stated that dynamically activating a
virtualenv from an existing Python process isn't "switching"
virtualenvs, but rather adding its packages to the current
environment. Installing Mach dependencies into all command virtualenvs
would align with the additive nature here.
However, there's one big downside: performance. Installing modules,
especially native modules, can be slow. Having to set up glean
,
zstandard
and psutil
into all command virtualenvs is inefficient and
will waste developer time. So, until it's needed, the manual
sys.executable
management appears to be the more effective solution.
Depends on D120404
Comment 15•3 years ago
|
||
Comment 16•3 years ago
|
||
Backed out for busting the gecko decision task.
Failure log: https://treeherder.mozilla.org/logviewer?job_id=345811009&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/7b7a049fb3e3979546f707a50e12dc9aacf59f78
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 17•3 years ago
|
||
We've overloaded "bootstrap" to mean three different things:
- The "standalone bootstrap script":
python/mozboot/bin/bootstrap.py
.
This is to freshly clone a new repo, then run./mach bootstrap
. ./mach bootstrap
: Install necessary dependencies and set up the
system for development.- "Mach bootstrap": do the work Mach needs before it can run commands.
By using the term "initialize" instead, perhaps we can remove
ambiguity when discussing Mach.
I'm not attached to the name (or this change at all), but I'm interested
in reviewer thoughts :)
Depends on D120398
Comment 18•3 years ago
|
||
Comment 19•3 years ago
|
||
Backed out for causing win debug build bustages
Backout link: https://hg.mozilla.org/integration/autoland/rev/d716918916ac1a4993a616f07d501f9b64be467d
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 20•3 years ago
|
||
Comment 21•3 years ago
|
||
Backed out for causing gecko decision failures.
-
backout: https://hg.mozilla.org/integration/autoland/rev/eae583a154ad574dde8c4203f3402ae3f980c494
-
failure log: https://treeherder.mozilla.org/logviewer?job_id=346814802&repo=autoland&lineNumber=634
[task 2021-07-29T23:02:40.686Z] Writing docker-contexts/ubuntu1804-test.tar.gz for docker image ubuntu1804-test
[task 2021-07-29T23:02:40.686Z] Error loading tasks for kind docker-image:
[task 2021-07-29T23:02:40.687Z] Traceback (most recent call last):
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/mach_commands.py", line 354, in taskgraph_decision
[task 2021-07-29T23:02:40.687Z] ret = taskgraph.decision.taskgraph_decision(options)
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/decision.py", line 227, in taskgraph_decision
[task 2021-07-29T23:02:40.687Z] full_task_json = tgg.full_task_graph.to_json()
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 174, in full_task_graph
[task 2021-07-29T23:02:40.687Z] return self._run_until("full_task_graph")
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 435, in _run_until
[task 2021-07-29T23:02:40.687Z] k, v = next(self._run)
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 312, in _run
[task 2021-07-29T23:02:40.687Z] self._write_artifacts,
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 92, in load_tasks
[task 2021-07-29T23:02:40.687Z] for task_dict in transforms(trans_config, inputs)
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 80, in <listcomp>
[task 2021-07-29T23:02:40.687Z] Task(
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 2120, in check_run_task_caches
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 2028, in check_task_dependencies
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 2014, in check_task_identifiers
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1995, in chain_of_trust
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1786, in build_task
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1777, in try_task_config_routes
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1767, in try_task_config_chemspill_prio
[task 2021-07-29T23:02:40.687Z] for task in tasks:
[task 2021-07-29T23:02:40.687Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1757, in try_task_config_env
[task 2021-07-29T23:02:40.688Z] for task in tasks:
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1719, in add_index_routes
[task 2021-07-29T23:02:40.688Z] for task in tasks:
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1514, in validate
[task 2021-07-29T23:02:40.688Z] for task in tasks:
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1491, in task_name_from_label
[task 2021-07-29T23:02:40.688Z] for task in tasks:
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1446, in set_defaults
[task 2021-07-29T23:02:40.688Z] for task in tasks:
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1425, in set_implementation
[task 2021-07-29T23:02:40.688Z] for task in tasks:
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/cached_tasks.py", line 64, in cache_task
[task 2021-07-29T23:02:40.688Z] for task in order_tasks(config, tasks):
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/cached_tasks.py", line 19, in order_tasks
[task 2021-07-29T23:02:40.688Z] pending = deque(tasks)
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/docker_image.py", line 108, in fill_template
[task 2021-07-29T23:02:40.688Z] GECKO, context_path, context_file, image_name, args
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/util/docker.py", line 216, in create_context_tar
[task 2021-07-29T23:02:40.688Z] args=args,
[task 2021-07-29T23:02:40.688Z] File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/util/docker.py", line 252, in stream_context_tar
[task 2021-07-29T23:02:40.688Z] raise Exception("extra include path does not exist: %s" % p)
[task 2021-07-29T23:02:40.688Z] Exception: extra include path does not exist: build/psutil_requirements.txt
[taskcluster 2021-07-29 23:02:41.087Z] === Task Finished ===
[taskcluster 2021-07-29 23:02:41.994Z] Unsuccessful task run with exit code: 1 completed in 23.475 seconds
Assignee | ||
Updated•3 years ago
|
Comment 22•3 years ago
|
||
Comment 23•3 years ago
|
||
This was backout out for causing initial backout conflict, apologize for the inconvenience.
Backed out per glandium's request, 7 changesets (Bug 1720215, Bug 1717645, Bug 1717051) for causing build bustage and conflicting backout
Log: https://treeherder.mozilla.org/logviewer?job_id=346816196&repo=autoland&lineNumber=987
Backout: https://hg.mozilla.org/integration/autoland/rev/791ec1c34fb986a24cfcda7c4d98af416299a024
Assignee | ||
Updated•3 years ago
|
Comment 24•3 years ago
|
||
Comment 25•3 years ago
|
||
bugherder |
Assignee | ||
Comment 26•3 years ago
|
||
The Glean SDK was traditionally represented in-tree as glean_sdk
.
However, on PyPI and in pip list
, it appears as glean-sdk
(pip
was
doing the translation implicitly and internally).
This is breaking our virtualenv state-checking code, since it thinks
that glean_sdk
isn't installed.
Since glean_sdk
was marked as pypi-optional
, this hasn't affected us
yet, but it will when we bump the Glean SDK version, because developers'
virtualenvs won't be detected as out-of-date.
Comment 27•3 years ago
|
||
Comment 28•3 years ago
|
||
bugherder |
Assignee | ||
Comment 29•3 years ago
|
||
The "six"-related comment is no longer relevant, and here
isn't used.
Depends on D120402
Assignee | ||
Comment 30•3 years ago
|
||
Though it's acceptable for an optional dependency to not be installed,
it is a problem if it's installed to the wrong version.
Depends on D122885
Assignee | ||
Comment 31•3 years ago
|
||
Currently, developers don't have a way to have the Mach virtualenv
re-attempt to install optional dependencies (such as glean
).
As part of ./mach bootstrap
(the general catch-all "re-create my dev
environment" command), we should retry installing optional dependencies.
This also matches the "glean isn't installed" error message
recommendation.
Depends on D123241
Comment 32•3 years ago
|
||
Comment 33•3 years ago
|
||
bugherder |
Assignee | ||
Comment 34•3 years ago
|
||
Rather than re-implementing it as search_path()
, use the existing
MachEnvRequirements
tool to parse mach_virtualenv_requirements.txt
Depends on D125910
Updated•3 years ago
|
Comment 35•3 years ago
|
||
Comment 36•3 years ago
|
||
Comment 37•3 years ago
|
||
bugherder |
Comment 38•3 years ago
|
||
Comment 39•3 years ago
|
||
bugherder |
Comment 40•3 years ago
|
||
(In reply to Pulsebot from comment #38)
Pushed by mhentges@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/894fc83e19a0
Always populate Mach pths r=firefox-build-system-reviewers,glandium
With this patch landed I'm not able to run any mach
command anymore. The failure is:
➜ mach build
Traceback (most recent call last):
File "/Users/henrik/code/gecko/mach", line 167, in <module>
main(sys.argv[1:])
File "/Users/henrik/code/gecko/mach", line 159, in main
mach = check_and_get_mach(os.path.dirname(os.path.realpath(__file__)))
File "/Users/henrik/code/gecko/mach", line 146, in check_and_get_mach
return load_mach(dir_path, mach_path)
File "/Users/henrik/code/gecko/mach", line 134, in load_mach
return mach_initialize.initialize(dir_path)
File "/Users/henrik/code/gecko/build/mach_initialize.py", line 302, in initialize
import mach.main
File "/Users/henrik/code/gecko/python/mach/mach/main.py", line 33, in <module>
from .dispatcher import CommandAction
File "/Users/henrik/code/gecko/python/mach/mach/dispatcher.py", line 20, in <module>
from .decorators import SettingsProvider
File "/Users/henrik/code/gecko/python/mach/mach/decorators.py", line 13, in <module>
from mozbuild.base import MachCommandBase
ModuleNotFoundError: No module named 'mozbuild'
I requested a backout from the sheriffs.
Comment 41•3 years ago
|
||
Backed out for causing busting macth commands. a=backout
Backout link : https://hg.mozilla.org/mozilla-central/rev/26cd1746e43de0cf2cbcb9897cf19a0ee4a2da88
Assignee | ||
Comment 42•3 years ago
|
||
Hmm, your mach
virtualenv needed to be updated, but we don't have an up_to_date()
check happening unless you run ./mach create-mach-environment
or ./mach bootstrap
.
I'll add an up_to_date()
check on startup, then re-land this.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 43•3 years ago
|
||
Comment 44•3 years ago
|
||
bugherder |
Comment 45•3 years ago
|
||
(In reply to Mitchell Hentges [:mhentges] 🦀 from comment #42)
Hmm, your
mach
virtualenv needed to be updated, but we don't have anup_to_date()
check happening unless you run./mach create-mach-environment
or./mach bootstrap
.
I'll add anup_to_date()
check on startup, then re-land this.
Thanks for adding the extra check. It's working fine now and prompts me to run create-mach-environment
. Maybe this could be done automatically?
Assignee | ||
Comment 46•3 years ago
|
||
Good call, I'm working my way towards it. It's in the patch stack :)
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 49•3 years ago
|
||
Depends on D129694
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 50•3 years ago
|
||
Comment 51•3 years ago
|
||
bugherder |
Assignee | ||
Comment 52•3 years ago
|
||
Mission accomplished 😎
Updated•3 years ago
|
Updated•3 years ago
|
Comment 53•3 years ago
|
||
Comment on attachment 9237261 [details]
Bug 1717051: Attempt to reinstall optional dependencies during bootstrap
Revision D123242 was moved to bug 1747837. Setting attachment 9237261 [details] to obsolete.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 55•3 years ago
|
||
Comment on attachment 9237261 [details]
Bug 1717051: Attempt to reinstall optional dependencies during bootstrap
Revision D123242 was moved to bug 1747837. Setting attachment 9237261 [details] to obsolete.
Description
•