From ba28962c5e199d6bb189ae743df9600ad1fa6682 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Wed, 28 Feb 2024 19:53:27 +0000 Subject: [PATCH] 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 | 76 ++++++------------- build/moz.configure/toolchain.configure | 8 +- python/mozbuild/mozbuild/bootstrap.py | 3 - .../mozbuild/test/configure/test_bootstrap.py | 43 +++++++++-- tools/rb/fix_stacks.py | 3 - 5 files changed, 61 insertions(+), 72 deletions(-) diff --git a/build/moz.configure/bootstrap.configure b/build/moz.configure/bootstrap.configure index daaff7cdaa2c..d8deddbb9e20 100644 --- a/build/moz.configure/bootstrap.configure +++ b/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 - # try_bootstrap. Automation will not have gone through the bootstrap - # process, but we want to return the path if it exists. - if os.path.exists(path): - return path + if enable_bootstrap or enable_bootstrap.origin == "default": + # We re-test whether the path exists because it may have been created by + # try_bootstrap. Automation will not have gone through the bootstrap + # process, but we want to return the path if it exists. + 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( - bootstrap, path, bootstrap_path, order, paths, original_path - ): + def bootstrap_search_path(path, paths, original_path): if paths is None: paths = original_path - if not bootstrap_path: + if not path: return paths - if order == "maybe-prepend": - if bootstrap(path.split("/")[0]): - order = "prepend" - else: - order = "append" - if order == "prepend": - return [bootstrap_path] + paths - return paths + [bootstrap_path] + return [path] + paths return bootstrap_search_path diff --git a/build/moz.configure/toolchain.configure b/build/moz.configure/toolchain.configure index 823995edd6a6..c39fec861b7c 100644 --- a/build/moz.configure/toolchain.configure +++ b/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) - else: - result.append(rustup_path) + result.insert(0, rustup_path) return result diff --git a/python/mozbuild/mozbuild/bootstrap.py b/python/mozbuild/mozbuild/bootstrap.py index 60a307145cca..a21ad75ee4cf 100644 --- a/python/mozbuild/mozbuild/bootstrap.py +++ b/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 diff --git a/python/mozbuild/mozbuild/test/configure/test_bootstrap.py b/python/mozbuild/mozbuild/test/configure/test_bootstrap.py index 758ddd563226..e3e3d3c744dd 100644 --- a/python/mozbuild/mozbuild/test/configure/test_bootstrap.py +++ b/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), ) diff --git a/tools/rb/fix_stacks.py b/tools/rb/fix_stacks.py index fb30ad319d0f..29aa396219e7 100755 --- a/tools/rb/fix_stacks.py +++ b/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.