Test cargo arguments passed by bootstrap.py

This moves a lot of code around, but the logic itself is not too terribly complicated.

- Move almost all logic in `def bootstrap` to the `RustBuild` class, to avoid mixing setting configuration with running commands
- Update various doctests to the new (more complete) RustBuild config. In particular, don't pretend that `bin_root` supports `build` being unset.
- Change `parse_args` not to use a global, to allow testing it
- Set BUILD_DIR appropriately so bootstrap.py doesn't panic because cargo isn't found

3 files changed, 118 insertions, 88 deletions

diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py
index 5714613cdf5..9b8fdd38d22 100644
--- a/src/bootstrap/bootstrap.py
+++ b/src/bootstrap/bootstrap.py
@@ -458,23 +458,52 @@ def unpack_component(download_info):
         verbose=download_info.verbose,
     )
 
-class RustBuild(object):
-    """Provide all the methods required to build Rust"""
+class FakeArgs:
+    """Used for unit tests to avoid updating all call sites"""
     def __init__(self):
-        self.checksums_sha256 = {}
-        self.stage0_compiler = None
-        self.download_url = ''
         self.build = ''
         self.build_dir = ''
         self.clean = False
-        self.config_toml = ''
-        self.rust_root = ''
-        self.use_locked_deps = False
-        self.use_vendored_sources = False
         self.verbose = False
+        self.json_output = False
+
+class RustBuild(object):
+    """Provide all the methods required to build Rust"""
+    def __init__(self, config_toml="", args=FakeArgs()):
         self.git_version = None
         self.nix_deps_dir = None
         self._should_fix_bins_and_dylibs = None
+        self.rust_root = os.path.abspath(os.path.join(__file__, '../../..'))
+
+        self.config_toml = config_toml
+
+        self.verbose = args.verbose != 0
+        self.clean = args.clean
+        self.json_output = args.json_output
+
+        profile = self.get_toml('profile')
+        if profile is not None:
+            include_file = 'config.{}.toml'.format(profile)
+            include_dir = os.path.join(self.rust_root, 'src', 'bootstrap', 'defaults')
+            include_path = os.path.join(include_dir, include_file)
+            # HACK: This works because `self.get_toml()` returns the first match it finds for a
+            # specific key, so appending our defaults at the end allows the user to override them
+            with open(include_path) as included_toml:
+                self.config_toml += os.linesep + included_toml.read()
+
+        self.use_vendored_sources = self.get_toml('vendor', 'build') == 'true'
+        self.use_locked_deps = self.get_toml('locked-deps', 'build') == 'true'
+
+        build_dir = args.build_dir or self.get_toml('build-dir', 'build') or 'build'
+        self.build_dir = os.path.abspath(build_dir)
+
+        with open(os.path.join(self.rust_root, "src", "stage0.json")) as f:
+            data = json.load(f)
+        self.checksums_sha256 = data["checksums_sha256"]
+        self.stage0_compiler = Stage0Toolchain(data["compiler"])
+        self.download_url = os.getenv("RUSTUP_DIST_SERVER") or data["config"]["dist_server"]
+
+        self.build = args.build or self.build_triple()
 
     def download_toolchain(self):
         """Fetch the build system for Rust, written in Rust
@@ -704,9 +733,10 @@ class RustBuild(object):
         """Return the path for .rustc-stamp at the given stage
 
         >>> rb = RustBuild()
+        >>> rb.build = "host"
         >>> rb.build_dir = "build"
-        >>> rb.rustc_stamp() == os.path.join("build", "stage0", ".rustc-stamp")
-        True
+        >>> expected = os.path.join("build", "host", "stage0", ".rustc-stamp")
+        >>> assert rb.rustc_stamp() == expected, rb.rustc_stamp()
         """
         return os.path.join(self.bin_root(), '.rustc-stamp')
 
@@ -721,15 +751,9 @@ class RustBuild(object):
         """Return the binary root directory for the given stage
 
         >>> rb = RustBuild()
-        >>> rb.build_dir = "build"
-        >>> rb.bin_root() == os.path.join("build", "stage0")
-        True
-
-        When the 'build' property is given should be a nested directory:
-
         >>> rb.build = "devel"
-        >>> rb.bin_root() == os.path.join("build", "devel", "stage0")
-        True
+        >>> expected = os.path.abspath(os.path.join("build", "devel", "stage0"))
+        >>> assert rb.bin_root() == expected, rb.bin_root()
         """
         subdir = "stage0"
         return os.path.join(self.build_dir, self.build, subdir)
@@ -842,6 +866,16 @@ class RustBuild(object):
             print("::group::Building bootstrap")
         else:
             print("Building bootstrap", file=sys.stderr)
+
+        args = self.build_bootstrap_cmd(env, color, warnings, verbose_count)
+        # Run this from the source directory so cargo finds .cargo/config
+        run(args, env=env, verbose=self.verbose, cwd=self.rust_root)
+
+        if "GITHUB_ACTIONS" in env:
+            print("::endgroup::")
+
+    def build_bootstrap_cmd(self, env, color, warnings, verbose_count):
+        """For tests."""
         build_dir = os.path.join(self.build_dir, "bootstrap")
         if self.clean and os.path.exists(build_dir):
             shutil.rmtree(build_dir)
@@ -927,11 +961,7 @@ class RustBuild(object):
         except KeyError:
             pass
 
-        # Run this from the source directory so cargo finds .cargo/config
-        run(args, env=env, verbose=self.verbose, cwd=self.rust_root)
-
-        if "GITHUB_ACTIONS" in env:
-            print("::endgroup::")
+        return args
 
     def build_triple(self):
         """Build triple as in LLVM
@@ -981,7 +1011,7 @@ class RustBuild(object):
             if os.path.exists(cargo_dir):
                 shutil.rmtree(cargo_dir)
 
-def parse_args():
+def parse_args(args):
     """Parse the command line arguments that the python script needs."""
     parser = argparse.ArgumentParser(add_help=False)
     parser.add_argument('-h', '--help', action='store_true')
@@ -994,16 +1024,11 @@ def parse_args():
     parser.add_argument('--warnings', choices=['deny', 'warn', 'default'], default='default')
     parser.add_argument('-v', '--verbose', action='count', default=0)
 
-    return parser.parse_known_args(sys.argv)[0]
+    return parser.parse_known_args(args)[0]
 
 def bootstrap(args):
     """Configure, fetch, build and run the initial bootstrap"""
-    # Configure initial bootstrap
-    build = RustBuild()
-    build.rust_root = os.path.abspath(os.path.join(__file__, '../../..'))
-    build.verbose = args.verbose != 0
-    build.clean = args.clean
-    build.json_output = args.json_output
+    rust_root = os.path.abspath(os.path.join(__file__, '../../..'))
 
     # Read from `--config`, then `RUST_BOOTSTRAP_CONFIG`, then `./config.toml`,
     # then `config.toml` in the root directory.
@@ -1012,45 +1037,23 @@ def bootstrap(args):
     if using_default_path:
         toml_path = 'config.toml'
         if not os.path.exists(toml_path):
-            toml_path = os.path.join(build.rust_root, toml_path)
+            toml_path = os.path.join(rust_root, toml_path)
 
     # Give a hard error if `--config` or `RUST_BOOTSTRAP_CONFIG` are set to a missing path,
     # but not if `config.toml` hasn't been created.
     if not using_default_path or os.path.exists(toml_path):
         with open(toml_path) as config:
-            build.config_toml = config.read()
-
-    profile = build.get_toml('profile')
-    if profile is not None:
-        include_file = 'config.{}.toml'.format(profile)
-        include_dir = os.path.join(build.rust_root, 'src', 'bootstrap', 'defaults')
-        include_path = os.path.join(include_dir, include_file)
-        # HACK: This works because `build.get_toml()` returns the first match it finds for a
-        # specific key, so appending our defaults at the end allows the user to override them
-        with open(include_path) as included_toml:
-            build.config_toml += os.linesep + included_toml.read()
+            config_toml = config.read()
+
+    # Configure initial bootstrap
+    build = RustBuild(config_toml, args)
+    build.check_vendored_status()
 
     verbose_count = args.verbose
     config_verbose_count = build.get_toml('verbose', 'build')
     if config_verbose_count is not None:
         verbose_count = max(args.verbose, int(config_verbose_count))
 
-    build.use_vendored_sources = build.get_toml('vendor', 'build') == 'true'
-    build.use_locked_deps = build.get_toml('locked-deps', 'build') == 'true'
-
-    build.check_vendored_status()
-
-    build_dir = args.build_dir or build.get_toml('build-dir', 'build') or 'build'
-    build.build_dir = os.path.abspath(build_dir)
-
-    with open(os.path.join(build.rust_root, "src", "stage0.json")) as f:
-        data = json.load(f)
-    build.checksums_sha256 = data["checksums_sha256"]
-    build.stage0_compiler = Stage0Toolchain(data["compiler"])
-    build.download_url = os.getenv("RUSTUP_DIST_SERVER") or data["config"]["dist_server"]
-
-    build.build = args.build or build.build_triple()
-
     if not os.path.exists(build.build_dir):
         os.makedirs(build.build_dir)
 
@@ -1077,7 +1080,7 @@ def main():
     if len(sys.argv) > 1 and sys.argv[1] == 'help':
         sys.argv[1] = '-h'
 
-    args = parse_args()
+    args = parse_args(sys.argv)
     help_triggered = args.help or len(sys.argv) == 1
 
     # If the user is asking for help, let them know that the whole download-and-build
diff --git a/src/bootstrap/bootstrap_test.py b/src/bootstrap/bootstrap_test.py
index a0c296dc19c..6854aafa1c0 100644
--- a/src/bootstrap/bootstrap_test.py
+++ b/src/bootstrap/bootstrap_test.py
@@ -15,6 +15,23 @@ from shutil import rmtree
 import bootstrap
 import configure
 
+def serialize_and_parse(args):
+    from io import StringIO
+
+    section_order, sections, targets = configure.parse_args(args)
+    buffer = StringIO()
+    configure.write_config_toml(buffer, section_order, targets, sections)
+    build = bootstrap.RustBuild()
+    build.config_toml = buffer.getvalue()
+
+    try:
+        import tomllib
+        # Verify this is actually valid TOML.
+        tomllib.loads(build.config_toml)
+    except ImportError:
+        print("warning: skipping TOML validation, need at least python 3.11", file=sys.stderr)
+    return build
+
 
 class VerifyTestCase(unittest.TestCase):
     """Test Case for verify"""
@@ -79,46 +96,57 @@ class ProgramOutOfDate(unittest.TestCase):
 
 class GenerateAndParseConfig(unittest.TestCase):
     """Test that we can serialize and deserialize a config.toml file"""
-    def serialize_and_parse(self, args):
-        from io import StringIO
-
-        section_order, sections, targets = configure.parse_args(args)
-        buffer = StringIO()
-        configure.write_config_toml(buffer, section_order, targets, sections)
-        build = bootstrap.RustBuild()
-        build.config_toml = buffer.getvalue()
-
-        try:
-            import tomllib
-            # Verify this is actually valid TOML.
-            tomllib.loads(build.config_toml)
-        except ImportError:
-            print("warning: skipping TOML validation, need at least python 3.11", file=sys.stderr)
-        return build
-
     def test_no_args(self):
-        build = self.serialize_and_parse([])
+        build = serialize_and_parse([])
         self.assertEqual(build.get_toml("changelog-seen"), '2')
         self.assertEqual(build.get_toml("profile"), 'user')
         self.assertIsNone(build.get_toml("llvm.download-ci-llvm"))
 
     def test_set_section(self):
-        build = self.serialize_and_parse(["--set", "llvm.download-ci-llvm"])
+        build = serialize_and_parse(["--set", "llvm.download-ci-llvm"])
         self.assertEqual(build.get_toml("download-ci-llvm", section="llvm"), 'true')
 
     def test_set_target(self):
-        build = self.serialize_and_parse(["--set", "target.x86_64-unknown-linux-gnu.cc=gcc"])
+        build = serialize_and_parse(["--set", "target.x86_64-unknown-linux-gnu.cc=gcc"])
         self.assertEqual(build.get_toml("cc", section="target.x86_64-unknown-linux-gnu"), 'gcc')
 
     def test_set_top_level(self):
-        build = self.serialize_and_parse(["--set", "profile=compiler"])
+        build = serialize_and_parse(["--set", "profile=compiler"])
         self.assertEqual(build.get_toml("profile"), 'compiler')
 
     def test_set_codegen_backends(self):
-        build = self.serialize_and_parse(["--set", "rust.codegen-backends=cranelift"])
+        build = serialize_and_parse(["--set", "rust.codegen-backends=cranelift"])
         self.assertNotEqual(build.config_toml.find("codegen-backends = ['cranelift']"), -1)
-        build = self.serialize_and_parse(["--set", "rust.codegen-backends=cranelift,llvm"])
+        build = serialize_and_parse(["--set", "rust.codegen-backends=cranelift,llvm"])
         self.assertNotEqual(build.config_toml.find("codegen-backends = ['cranelift', 'llvm']"), -1)
-        build = self.serialize_and_parse(["--enable-full-tools"])
+        build = serialize_and_parse(["--enable-full-tools"])
         self.assertNotEqual(build.config_toml.find("codegen-backends = ['llvm']"), -1)
 
+
+class BuildBootstrap(unittest.TestCase):
+    """Test that we generate the appropriate arguments when building bootstrap"""
+
+    def build_args(self, configure_args=[], args=[], env={}):
+        env = env.copy()
+        env["PATH"] = os.environ["PATH"]
+
+        build = serialize_and_parse(configure_args)
+        build.build_dir = os.environ["BUILD_DIR"]
+        parsed = bootstrap.parse_args(args)
+        return build.build_bootstrap_cmd(env, parsed.color, parsed.warnings, parsed.verbose), env
+
+    def test_cargoflags(self):
+        args, _ = self.build_args(env={"CARGOFLAGS": "--timings"})
+        self.assertTrue("--timings" in args)
+
+    def test_warnings(self):
+        for toml_warnings in ['false', 'true', None]:
+            configure_args = []
+            if toml_warnings is not None:
+                configure_args = ["--set", "rust.deny-warnings=" + toml_warnings]
+
+            _, env = self.build_args(configure_args, args=["--warnings=warn"])
+            self.assertFalse("-Dwarnings" in env["RUSTFLAGS"])
+
+            _, env = self.build_args(configure_args, args=["--warnings=deny"])
+            self.assertTrue("-Dwarnings" in env["RUSTFLAGS"])
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 9cc5a69b920..9f0f24e0f7b 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -2665,9 +2665,8 @@ impl Step for Bootstrap {
     fn run(self, builder: &Builder<'_>) {
         let mut check_bootstrap = Command::new(&builder.python());
         check_bootstrap
-            .arg("-m")
-            .arg("unittest")
-            .arg("bootstrap_test.py")
+            .args(["-m", "unittest", "bootstrap_test.py"])
+            .env("BUILD_DIR", &builder.out)
             .current_dir(builder.src.join("src/bootstrap/"))
             .args(builder.config.test_args());
         try_run(builder, &mut check_bootstrap).unwrap();