Bug 1853271 - Make `mach configure` prefer bootstrapped toolchains. r=firefox-build-system-reviewers,sergesanspaille,nalexander

This changes the semantics of bootstrapping substantially, but all for
the simpler.

- --disable-bootstrap now prevents bootstrapped toolchains from being used
  entirely, even if they are present.
- --enable-bootstrap still automatically downloads missing or
  out-of-date toolchains, and is still only the default when building off
  a VCS checkout of mozilla-central.
- When neither option is given on another tree than a VCS checkout of
  mozilla-central, already bootstrapped toolchains are prioritized, but
  missing toolchains are not downloaded, and outdated toolchains are not
  updated.
- --enable-bootstrap=no-update can now be used to replace the previous
  behavior of --disable-bootstrap, to avoid the automatic update of
  already bootstrapped toolchains, with the difference that missing
  toolchains are still automatically bootstrapped.

This has the downside of making the semantics of the per-toolchain
opt-in/opt-out mechanics introduced in bug 1828027 kind of confusing,
but I'm keeping reworking that, or entirely removing it for a followup.

Differential Revision: https://phabricator.services.mozilla.com/D188315

build/moz.configure/bootstrap.configure

@@ -36,10 +36,12 @@ option(
 
 
 @depends_if("--enable-bootstrap")
-def enable_bootstrap(bootstrap):
+def want_bootstrap(bootstrap):
     include = set()
     exclude = set()
     for item in bootstrap:
+        if item == "no-update":
+            continue
         if item.startswith("-"):
             exclude.add(item.lstrip("-"))
         else:
@@ -55,34 +57,6 @@ def enable_bootstrap(bootstrap):
     return match
 
 
-@depends(developer_options, "--enable-bootstrap", moz_fetches_dir)
-def bootstrap_search_path_order(developer_options, bootstrap, moz_fetches_dir):
-    if moz_fetches_dir:
-        log.debug("Prioritizing MOZ_FETCHES_DIR in toolchain path.")
-        return "prepend"
-
-    if bootstrap:
-        log.debug(
-            "Prioritizing mozbuild state dir in toolchain paths because "
-            "bootstrap mode is enabled."
-        )
-        return "maybe-prepend"
-
-    if developer_options:
-        log.debug(
-            "Prioritizing mozbuild state dir in toolchain paths because "
-            "you are not building in release mode."
-        )
-        return "prepend"
-
-    log.debug(
-        "Prioritizing system over mozbuild state dir in "
-        "toolchain paths because you are building in "
-        "release mode."
-    )
-    return "append"
-
-
 toolchains_base_dir = moz_fetches_dir | mozbuild_state_path
 
 
@@ -145,7 +119,8 @@ def bootstrap_path(path, **kwargs):
         )
 
     @depends(
-        enable_bootstrap,
+        "--enable-bootstrap",
+        want_bootstrap,
         toolchains_base_dir,
         moz_fetches_dir,
         bootstrap_toolchain_tasks,
@@ -163,7 +138,8 @@ def bootstrap_path(path, **kwargs):
     @imports(_from="__builtin__", _import="open")
     @imports(_from="__builtin__", _import="Exception")
     def bootstrap_path(
-        bootstrap,
+        enable_bootstrap,
+        want_bootstrap,
         toolchains_base_dir,
         moz_fetches_dir,
         tasks,
@@ -294,20 +270,26 @@ def bootstrap_path(path, **kwargs):
             return True
 
         path = os.path.join(toolchains_base_dir, path_prefix, *path_parts)
-        if bootstrap and bootstrap(path_parts[0]):
+        if enable_bootstrap and want_bootstrap(path_parts[0]):
+            exists = os.path.exists(path)
             try:
-                if not try_bootstrap(os.path.exists(path)):
+                # With --enable-bootstrap=no-update, we don't `try_bootstrap`, except
+                # when the toolchain can't be found.
+                if (
+                    "no-update" not in enable_bootstrap or not exists
+                ) and not try_bootstrap(exists):
                     # If there aren't toolchain artifacts to use for this build,
                     # don't return a path.
                     return None
             except Exception as e:
                 log.error("%s", e)
                 die("If you can't fix the above, retry with --disable-bootstrap.")
-        # We re-test whether the path exists because it may have been created by
+        if enable_bootstrap or enable_bootstrap.origin == "default":
-        # try_bootstrap. Automation will not have gone through the bootstrap
+            # We re-test whether the path exists because it may have been created by
-        # process, but we want to return the path if it exists.
+            # try_bootstrap. Automation will not have gone through the bootstrap
-        if os.path.exists(path):
+            # process, but we want to return the path if it exists.
-            return path
+            if os.path.exists(path):
+                return path
 
     return bootstrap_path
 
@@ -315,27 +297,15 @@ def bootstrap_path(path, **kwargs):
 @template
 def bootstrap_search_path(path, paths=original_path, **kwargs):
     @depends(
-        enable_bootstrap,
-        dependable(path),
         bootstrap_path(path, **kwargs),
-        bootstrap_search_path_order,
         paths,
         original_path,
     )
-    def bootstrap_search_path(
+    def bootstrap_search_path(path, paths, original_path):
-        bootstrap, path, bootstrap_path, order, paths, original_path
-    ):
         if paths is None:
             paths = original_path
-        if not bootstrap_path:
+        if not path:
             return paths
-        if order == "maybe-prepend":
+        return [path] + paths
-            if bootstrap(path.split("/")[0]):
-                order = "prepend"
-            else:
-                order = "append"
-        if order == "prepend":
-            return [bootstrap_path] + paths
-        return paths + [bootstrap_path]
 
     return bootstrap_search_path

build/moz.configure/toolchain.configure

@@ -691,12 +691,11 @@ clang_search_path = bootstrap_search_path("clang/bin")
 
 @depends(
     bootstrap_search_path("rustc/bin", when="MOZ_AUTOMATION"),
-    bootstrap_search_path_order,
     original_path,
 )
 @imports("os")
 @imports(_from="os", _import="environ")
-def rust_search_path(rust_path, search_order, original_path):
+def rust_search_path(rust_path, original_path):
     result = list(rust_path or original_path)
     # Also add the rustup install directory for cargo/rustc.
     cargo_home = environ.get("CARGO_HOME", "")
@@ -705,10 +704,7 @@ def rust_search_path(rust_path, search_order, original_path):
     else:
         cargo_home = os.path.expanduser(os.path.join("~", ".cargo"))
     rustup_path = os.path.join(cargo_home, "bin")
-    if search_order == "prepend":
+    result.insert(0, rustup_path)
-        result.insert(0, rustup_path)
-    else:
-        result.append(rustup_path)
     return result
 
 

python/mozbuild/mozbuild/bootstrap.py

@@ -37,9 +37,6 @@ def _bootstrap_sandbox():
         Path(__file__).parent.parent.parent.parent / "build" / "moz.configure"
     )
     sandbox.include_file(str(moz_configure / "init.configure"))
-    # bootstrap_search_path_order has a dependency on developer_options, which
-    # is not defined in init.configure. Its value doesn't matter for us, though.
-    sandbox["developer_options"] = sandbox["always"]
     sandbox.include_file(str(moz_configure / "bootstrap.configure"))
     return sandbox
 

python/mozbuild/mozbuild/test/configure/test_bootstrap.py

@@ -37,7 +37,6 @@ class TestBootstrap(BaseConfigureTest):
     # - `in_path` is a 3-tuple representing whether the path for each toolchain is
     # expected to have been added to the `bootstrap_search_path`. Valid values are:
     #   - `True`: the toolchain path was prepended to `bootstrap_search_path`.
-    #   - `"append"`: the toolchain path was appended to `bootstrap_search_path`.
     #   - `False`: the toolchain path is not in `bootstrap_search_path`.
     def assertBootstrap(self, arg, states, bootstrapped, in_path):
         called_for = []
@@ -119,7 +118,7 @@ class TestBootstrap(BaseConfigureTest):
                 "--disable-bootstrap",
                 (True, "old", False),
                 (False, False, False),
-                (True, True, False),
+                (False, False, False),
             )
             self.assertBootstrap(
                 None,
@@ -133,13 +132,13 @@ class TestBootstrap(BaseConfigureTest):
                 "--disable-bootstrap",
                 (True, "old", False),
                 (False, False, False),
-                ("append", "append", False),
+                (False, False, False),
             )
             self.assertBootstrap(
                 None,
                 (True, "old", False),
                 (False, False, False),
-                ("append", "append", False),
+                (True, True, False),
             )
 
         for milestone in ("124.0a1", "124.0"):
@@ -151,6 +150,12 @@ class TestBootstrap(BaseConfigureTest):
                     (False, True, True),
                     (True, True, True),
                 )
+                self.assertBootstrap(
+                    "--enable-bootstrap=no-update",
+                    (True, "old", False),
+                    (False, False, True),
+                    (True, True, True),
+                )
 
                 # With `--enable-bootstrap=foo,bar`, only foo and bar are bootstrappable
                 self.assertBootstrap(
@@ -163,7 +168,19 @@ class TestBootstrap(BaseConfigureTest):
                     "--enable-bootstrap=foo",
                     (True, "old", True),
                     (False, False, False),
-                    (True, "append", "append"),
+                    (True, True, True),
+                )
+                self.assertBootstrap(
+                    "--enable-bootstrap=no-update,foo,bar",
+                    (False, "old", False),
+                    (True, False, False),
+                    (True, True, False),
+                )
+                self.assertBootstrap(
+                    "--enable-bootstrap=no-update,foo",
+                    (True, "old", True),
+                    (False, False, False),
+                    (True, True, True),
                 )
 
                 # With `--enable-bootstrap=-foo`, anything is bootstrappable, except foo
@@ -171,7 +188,7 @@ class TestBootstrap(BaseConfigureTest):
                     "--enable-bootstrap=-foo",
                     (True, False, "old"),
                     (False, True, True),
-                    ("append", True, True),
+                    (True, True, True),
                 )
                 self.assertBootstrap(
                     "--enable-bootstrap=-foo",
@@ -179,13 +196,25 @@ class TestBootstrap(BaseConfigureTest):
                     (False, True, True),
                     (False, True, True),
                 )
+                self.assertBootstrap(
+                    "--enable-bootstrap=no-update,-foo",
+                    (True, False, "old"),
+                    (False, True, False),
+                    (True, True, True),
+                )
+                self.assertBootstrap(
+                    "--enable-bootstrap=no-update,-foo",
+                    (False, False, "old"),
+                    (False, True, False),
+                    (False, True, True),
+                )
 
                 # Corner case.
                 self.assertBootstrap(
                     "--enable-bootstrap=-foo,foo,bar",
                     (False, False, "old"),
                     (False, True, False),
-                    (False, True, "append"),
+                    (False, True, True),
                 )
 
 

tools/rb/fix_stacks.py

@@ -35,9 +35,6 @@ def autobootstrap():
     )
     moz_configure = os.path.join(buildconfig.topsrcdir, "build", "moz.configure")
     sandbox.include_file(os.path.join(moz_configure, "init.configure"))
-    # bootstrap_search_path_order has a dependency on developer_options, which
-    # is not defined in init.configure. Its value doesn't matter for us, though.
-    sandbox["developer_options"] = sandbox["always"]
     sandbox.include_file(os.path.join(moz_configure, "bootstrap.configure"))
     # Expand the `bootstrap_path` template for "fix-stacks", and execute the
     # expanded function via `_value_for`, which will trigger autobootstrap.