Rewrite standalone bootstrap.py script to clone before calling into `mach bootstrap`
Categories
(Firefox Build System :: Bootstrap Configuration, enhancement, P3)
Tracking
(firefox82 fixed)
Tracking | Status | |
---|---|---|
firefox82 | --- | fixed |
People
(Reporter: mhentges, Assigned: rstewart)
References
Details
(Keywords: in-triage)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
We have a bootstrap.py
script that, in theory, simplifies setting up a Firefox development environment. However, it has some downsides:
- The script likes to bitrot by having imports to in-tree modules that don't exist from a bootstrap context. This breaks the Firefox setup workflow until detected (Sentry can't catch this, a manual bug report is required) and fixed.
- We can add tests around this, but it's still added complexity that we need to keep an eye on.
- It complicates documentation, since there's two ways of setting up Firefox (manually fetch
bootstrap.py
and invoke it, or manually clone then bootstrap). The mac docs are especially confusing: it looks like you're supposed to download and run this script, and manually clone and bootstrap separately. - It's less straightforward than it looks. Before running the script, the user still has to install some dependencies manually:
- Mac: need homebrew, py3
- Linux: need py3
- Windows: need MozillaBuild (though, standalone
bootstrap.py
might not even work on Windows) - So, this isn't just a "wget and run" situation, other manual work is required.
- The only benefit that the standalone
bootstrap.py
script has over aclone && mach bootstrap
is that it installshg
/git
and clones the repo automatically. IMHO, this is a pretty insignificant benefit considering the costs.
Reporter | ||
Comment 1•4 years ago
|
||
A couple other notes:
- Is anyone depending on this standalone script? After some discussion with Ricky, it sounds like Linux distros are already cloning and bootstrapping, which is good
- When this is removed, the quick start guides will need to be updated. There's good potential for streamlining here
- There's code that will be able to be removed in-tree as well
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Note that when I refer to "standalone bootstrap.py
" here, I'm referring to the file python/mozboot/bin/bootstrap.py
and no other similarly-named file in-tree.
The current design, where standalone bootstrap.py
downloads a small portion of the mozilla-central
repo and then works through all the bootstrap
logic, performing a clone in the middle of the bootstrap
process, has some deficiencies, namely:
-
Refactoring code that is shared between the
bootstrap
logic and the mainlinemach
logic is, if not impossible, more difficult than it needs to be, since standalonebootstrap.py
needs to download a set of files that bootstraps a build environment perfectly with no other dependencies inmozilla-central
. As a result we have some duplicated or redundant code and some stuff that has been refactored into themozboot
directory irrespective of whether it actually makes sense to go there (see bug 1649850). -
Since
mach bootstrap
has access to the entiremozilla-central
environment, but the (much less frequently exercised) standalonebootstrap.py
script does not, this can lead people to write patches that work fine inmach bootstrap
but which regress standalonebootstrap.py
. Furthermore, testingbootstrap
patches with standalonebootstrap.py
is difficult. So this is a not infrequent source of regressions; bugs 1652736 and 1643158 are recent examples. Furthermore, typically these regressions are "fixed" by adding more code duplication or by replacing battle-tested frequently-used libraries (eitherm-c
-internal or third-party) with less robust bespoke code. -
Issue (2) is avoidable if people are sufficiently critical during code review, but at best, this is a weird extra level of mental overhead that we need to keep in mind only for
bootstrap
patches.
This patch preserves back-compatibility and the validity of existing documentation by factoring out all the logic to clone mozilla-central
into standalone bootstrap.py
directly, and cloning before calling into mach bootstrap
directly. This gives us only one official entry point into the bootstrapper, namely, mach bootstrap
.
There are a couple concrete implications of this change:
-
Now, the clone happens before any other interesting work happens, so people may have to wait ~an hour before actually beginning to engage with the
bootstrap
wizard. While it's arguably slightly less convenient, I'm not sure it matters enough that we should block this patch or a similar one on it. -
The
hg
/git
configuration step now happens after the clone rather than before it. Looking at what thehg
andgit
configuration wizards actually do today, I don't think this matters either (all of the configurations can easily happen after cloning the repo).
Another note: bootstrap
installs git-cinnabar
to the .mozbuild
state directory. We could have duplicated all of that logic over to standalone bootstrap.py
, but it's non-trivial and I didn't think that made any sense, so instead we have standalone bootstrap.py
download a temporary version and use it for the initial clone if necessary.
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Proposing an alternative approach in the patch.
Comment 4•4 years ago
|
||
For the record, I support deprecating this standalone-style mach bootstrap
invocation. It just hasn't played out very well.
Assignee | ||
Comment 5•4 years ago
|
||
In the interest of clarity I'm re-titling this bug to capture what the patch actually does.
The script has value inasmuch as it's a one-liner that does "everything" for you. The alternative would be that we make everyone clone into mozilla-unified
and then run mach bootstrap
by themselves, but having evaluated that option, that's a complicated enough request that it makes sense to provide code to do that for people. For example, consider the in-tree code to clone mozilla-unified
using either hg
or git
; each of these are about a paragraph of code, and git
specifically requires you to install an external tool git-cinnabar
. Otherwise, to preserve parity with the current state of standalone bootstrap.py
, we'd have to require people to copy-paste these blocks of code which doesn't make sense. Assuming that the existing code is useful, keeping it in a script is sensible.
The patch just flips the existing code around a little to avoid the existing issues with how bootstrap.py
has been historically constructed while having the same overall functionality.
Comment 7•4 years ago
|
||
bugherder |
Description
•