Rollup merge of #112281 - jyn514:test-bootstrap-py, r=albertlarsan68

Test the cargo args generated by bootstrap.py

I recommend reviewing this commit-by-commit using the instructions in https://rustc-dev-guide.rust-lang.org/git.html#moving-large-sections-of-code.

- 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

- Allow passing arguments to `bootstrap_test.py`

    Previous, it used the built-in test runner, which doesn't support options unless they're manually passed in the script.

- Fix progress messages for configure in bootstrap_test.py

    Before it would unconditionally print `configure-args = []`.

r? `@albertlarsan68` cc https://github.com/rust-lang/rust/pull/112089 https://github.com/rust-lang/rust/pull/111979#issuecomment-1568525699

4 files changed, 144 insertions, 107 deletions

diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py
index 5714613cdf5..53a762cd0a8 100644
--- a/src/bootstrap/bootstrap.py
+++ b/src/bootstrap/bootstrap.py
@@ -458,23 +458,51 @@ 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
+        self.color = 'auto'
+        self.warnings = 'default'
+
+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.clean = args.clean
+        self.json_output = args.json_output
+        self.verbose = args.verbose
+        self.color = args.color
+        self.warnings = args.warnings
+
+        config_verbose_count = self.get_toml('verbose', 'build')
+        if config_verbose_count is not None:
+            self.verbose = max(self.verbose, int(config_verbose_count))
+
+        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 +732,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 +750,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)
@@ -761,9 +784,12 @@ class RustBuild(object):
         >>> rb.get_toml("key1")
         'true'
         """
+        return RustBuild.get_toml_static(self.config_toml, key, section)
 
+    @staticmethod
+    def get_toml_static(config_toml, key, section=None):
         cur_section = None
-        for line in self.config_toml.splitlines():
+        for line in config_toml.splitlines():
             section_match = re.match(r'^\s*\[(.*)\]\s*$', line)
             if section_match is not None:
                 cur_section = section_match.group(1)
@@ -772,7 +798,7 @@ class RustBuild(object):
             if match is not None:
                 value = match.group(1)
                 if section is None or section == cur_section:
-                    return self.get_string(value) or value.strip()
+                    return RustBuild.get_string(value) or value.strip()
         return None
 
     def cargo(self):
@@ -835,13 +861,23 @@ class RustBuild(object):
         """
         return os.path.join(self.build_dir, "bootstrap", "debug", "bootstrap")
 
-    def build_bootstrap(self, color, warnings, verbose_count):
+    def build_bootstrap(self):
         """Build bootstrap"""
         env = os.environ.copy()
         if "GITHUB_ACTIONS" in env:
             print("::group::Building bootstrap")
         else:
             print("Building bootstrap", file=sys.stderr)
+
+        args = self.build_bootstrap_cmd(env)
+        # 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):
+        """For tests."""
         build_dir = os.path.join(self.build_dir, "bootstrap")
         if self.clean and os.path.exists(build_dir):
             shutil.rmtree(build_dir)
@@ -894,10 +930,10 @@ class RustBuild(object):
         if target_linker is not None:
             env["RUSTFLAGS"] += " -C linker=" + target_linker
         env["RUSTFLAGS"] += " -Wrust_2018_idioms -Wunused_lifetimes"
-        if warnings == "default":
+        if self.warnings == "default":
             deny_warnings = self.get_toml("deny-warnings", "rust") != "false"
         else:
-            deny_warnings = warnings == "deny"
+            deny_warnings = self.warnings == "deny"
         if deny_warnings:
             env["RUSTFLAGS"] += " -Dwarnings"
 
@@ -908,7 +944,7 @@ class RustBuild(object):
                 self.cargo()))
         args = [self.cargo(), "build", "--manifest-path",
                 os.path.join(self.rust_root, "src/bootstrap/Cargo.toml")]
-        args.extend("--verbose" for _ in range(verbose_count))
+        args.extend("--verbose" for _ in range(self.verbose))
         if self.use_locked_deps:
             args.append("--locked")
         if self.use_vendored_sources:
@@ -918,20 +954,16 @@ class RustBuild(object):
             args.append("build-metrics")
         if self.json_output:
             args.append("--message-format=json")
-        if color == "always":
+        if self.color == "always":
             args.append("--color=always")
-        elif color == "never":
+        elif self.color == "never":
             args.append("--color=never")
         try:
             args += env["CARGOFLAGS"].split()
         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 +1013,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 +1026,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,52 +1039,37 @@ 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()
+            config_toml = config.read()
+    else:
+        config_toml = ''
 
-    profile = build.get_toml('profile')
+    profile = RustBuild.get_toml_static(config_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_dir = os.path.join(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
+        # 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:
-            build.config_toml += os.linesep + included_toml.read()
-
-    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'
+            config_toml += os.linesep + included_toml.read()
 
+    # Configure initial bootstrap
+    build = RustBuild(config_toml, args)
     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)
 
     # Fetch/build the bootstrap
     build.download_toolchain()
     sys.stdout.flush()
-    build.build_bootstrap(args.color, args.warnings, verbose_count)
+    build.build_bootstrap()
     sys.stdout.flush()
 
     # Run the bootstrap
@@ -1077,7 +1089,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 5ecda83ee66..815b32eb991 100644
--- a/src/bootstrap/bootstrap_test.py
+++ b/src/bootstrap/bootstrap_test.py
@@ -1,4 +1,6 @@
-"""Bootstrap tests"""
+"""Bootstrap tests
+
+Run these with `x test bootstrap`, or `python -m unittest bootstrap_test.py`."""
 
 from __future__ import absolute_import, division, print_function
 import os
@@ -13,6 +15,22 @@ from shutil import rmtree
 import bootstrap
 import configure
 
+def serialize_and_parse(configure_args, bootstrap_args=bootstrap.FakeArgs()):
+    from io import StringIO
+
+    section_order, sections, targets = configure.parse_args(configure_args)
+    buffer = StringIO()
+    configure.write_config_toml(buffer, section_order, targets, sections)
+    build = bootstrap.RustBuild(config_toml=buffer.getvalue(), args=bootstrap_args)
+
+    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"""
@@ -77,58 +95,58 @@ 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)
 
-if __name__ == '__main__':
-    SUITE = unittest.TestSuite()
-    TEST_LOADER = unittest.TestLoader()
-    SUITE.addTest(doctest.DocTestSuite(bootstrap))
-    SUITE.addTests([
-        TEST_LOADER.loadTestsFromTestCase(VerifyTestCase),
-        TEST_LOADER.loadTestsFromTestCase(GenerateAndParseConfig),
-        TEST_LOADER.loadTestsFromTestCase(ProgramOutOfDate)])
-
-    RUNNER = unittest.TextTestRunner(stream=sys.stdout, verbosity=2)
-    result = RUNNER.run(SUITE)
-    sys.exit(0 if result.wasSuccessful() else 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"]
+
+        parsed = bootstrap.parse_args(args)
+        build = serialize_and_parse(configure_args, parsed)
+        build.build_dir = os.environ["BUILD_DIR"]
+        build.build = os.environ["BUILD_PLATFORM"]
+        return build.build_bootstrap_cmd(env), 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/configure.py b/src/bootstrap/configure.py
index a16f77317c8..a5a1385dc0d 100755
--- a/src/bootstrap/configure.py
+++ b/src/bootstrap/configure.py
@@ -280,7 +280,7 @@ def parse_args(args):
 
     config = {}
 
-    set('build.configure-args', sys.argv[1:], config)
+    set('build.configure-args', args, config)
     apply_args(known_args, option_checking, config)
     return parse_example_config(known_args, config)
 
@@ -400,7 +400,9 @@ def parse_example_config(known_args, config):
     targets = {}
     top_level_keys = []
 
-    for line in open(rust_dir + '/config.example.toml').read().split("\n"):
+    with open(rust_dir + '/config.example.toml') as example_config:
+        example_lines = example_config.read().split("\n")
+    for line in example_lines:
         if cur_section is None:
             if line.count('=') == 1:
                 top_level_key = line.split('=')[0]
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 873ed61daf3..bdc6b4de6cd 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -2664,7 +2664,12 @@ impl Step for Bootstrap {
     /// Tests the build system itself.
     fn run(self, builder: &Builder<'_>) {
         let mut check_bootstrap = Command::new(&builder.python());
-        check_bootstrap.arg("bootstrap_test.py").current_dir(builder.src.join("src/bootstrap/"));
+        check_bootstrap
+            .args(["-m", "unittest", "bootstrap_test.py"])
+            .env("BUILD_DIR", &builder.out)
+            .env("BUILD_PLATFORM", &builder.build.build.triple)
+            .current_dir(builder.src.join("src/bootstrap/"))
+            .args(builder.config.test_args());
         try_run(builder, &mut check_bootstrap).unwrap();
 
         let host = builder.config.build;