Migrate exception logic and tests to rust-side bootstrap

Author: tanvincible

Diff for: src/bootstrap/bootstrap.py

@@ -1270,9 +1270,8 @@ def bootstrap(args):
     profile = RustBuild.get_toml_static(config_toml, "profile")
     is_non_git_source = not os.path.exists(os.path.join(rust_root, ".git"))
 
-    if profile is None:
-        if is_non_git_source:
-            profile = "dist"
+    if profile is None and is_non_git_source:
+        profile = "dist"
 
     if profile is not None:
         # Allows creating alias for profile names, allowing
@@ -1294,13 +1293,6 @@ def bootstrap(args):
         with open(include_path) as included_toml:
             config_toml += os.linesep + included_toml.read()
 
-    if is_non_git_source:
-        for option in ["download-ci-llvm", "download-rustc"]:
-            if RustBuild.get_toml_static(config_toml, option):
-                raise Exception(
-                    "Option '{}' cannot be used with non-git sources.".format(option)
-                )
-
     # Configure initial bootstrap
     build = RustBuild(config_toml, args)
     build.check_vendored_status()

Diff for: src/bootstrap/bootstrap_test.py

@@ -249,102 +249,3 @@ def test_warnings(self):
 
             _, env = self.build_args(configure_args, args=["--warnings=deny"])
             self.assertTrue("-Dwarnings" in env["RUSTFLAGS"])
-
-
-class TestProfileOverride(unittest.TestCase):
-    @patch("os.path.exists")
-    @patch("bootstrap.RustBuild.get_toml_static")
-    def test_profile_none_with_non_git_source(self, mock_get_toml_static, mock_exists):
-        mock_exists.return_value = False
-        mock_get_toml_static.return_value = None
-
-        rust_root = "/mock/rust_root"
-        config_toml = ""
-
-        profile = bootstrap.RustBuild.get_toml_static(config_toml, "profile")
-
-        is_non_git_source = not os.path.exists(os.path.join(rust_root, ".git"))
-        if profile is None and is_non_git_source:
-            profile = "dist"
-
-        self.assertEqual(profile, "dist")
-
-    @patch("os.path.exists")
-    @patch("bootstrap.RustBuild.get_toml_static")
-    def test_include_path_exists(self, mock_get_toml_static, mock_exists):
-        mock_exists.return_value = True
-        mock_get_toml_static.return_value = "dist"
-
-        rust_root = "/mock/rust_root"
-        config_toml = "mock_config"
-
-        profile = bootstrap.RustBuild.get_toml_static(config_toml, "profile")
-
-        profile_aliases = {"user": "dist"}
-        include_file = f"config.{profile_aliases.get(profile) or profile}.toml"
-        include_dir = os.path.join(rust_root, "src", "bootstrap", "defaults")
-        include_path = os.path.join(include_dir, include_file)
-
-        # Assert the include_path is correctly formed
-        self.assertEqual(
-            include_path, "/mock/rust_root/src/bootstrap/defaults/config.dist.toml"
-        )
-
-    @patch("os.path.exists")
-    @patch("bootstrap.RustBuild.get_toml_static")
-    @patch("builtins.open", new_callable=mock_open)
-    def test_config_toml_inclusion(self, mock_open, mock_get_toml_static, mock_exists):
-        mock_exists.side_effect = (
-            lambda path: path
-            == "/mock/rust_root/src/bootstrap/defaults/config.dist.toml"
-        )
-        mock_get_toml_static.return_value = "dist"
-        mock_open.return_value.read.return_value = "included_toml_content"
-
-        rust_root = "/mock/rust_root"
-        config_toml = "initial_config"
-
-        profile = bootstrap.RustBuild.get_toml_static(config_toml, "profile")
-
-        profile_aliases = {"user": "dist"}
-        include_file = f"config.{profile_aliases.get(profile) or profile}.toml"
-        include_dir = os.path.join(rust_root, "src", "bootstrap", "defaults")
-        include_path = os.path.join(include_dir, include_file)
-
-        with open(include_path) as included_toml:
-            config_toml += os.linesep + included_toml.read()
-
-        self.assertIn("initial_config\nincluded_toml_content", config_toml)
-
-    @patch("os.path.exists")
-    @patch("bootstrap.RustBuild.get_toml_static")
-    def test_invalid_profile_for_non_git_source(
-        self, mock_get_toml_static, mock_exists
-    ):
-        mock_exists.return_value = False
-
-        def mock_get_toml_static_side_effect(config_toml, option):
-            if option in ["download-ci-llvm", "download-rustc"]:
-                return "true"
-            return None
-
-        mock_get_toml_static.side_effect = mock_get_toml_static_side_effect
-
-        rust_root = "/mock/rust_root"
-        config_toml = ""
-
-        profile = bootstrap.RustBuild.get_toml_static(config_toml, "profile")
-        is_non_git_source = not os.path.exists(os.path.join(rust_root, ".git"))
-        if profile is None and is_non_git_source:
-            profile = "dist"
-
-        for option in ["download-ci-llvm", "download-rustc"]:
-            if bootstrap.RustBuild.get_toml_static(config_toml, option):
-                with self.assertRaises(Exception) as context:
-                    raise Exception(
-                        f"Option '{option}' cannot be used with non-git sources."
-                    )
-                self.assertTrue(
-                    f"Option '{option}' cannot be used with non-git sources."
-                    in str(context.exception)
-                )

Diff for: src/bootstrap/src/core/config/config.rs

@@ -2926,10 +2926,8 @@ impl Config {
         let if_unchanged = || {
             if self.rust_info.is_from_tarball() {
                 // Git is needed for running "if-unchanged" logic.
-                println!(
-                    "WARNING: 'if-unchanged' has no effect on tarball sources; ignoring `download-ci-llvm`."
-                );
-                return false;
+                println!("ERROR: 'if-unchanged' is only compatible with Git managed sources.");
+                crate::exit!(1);
             }
 
             // Fetching the LLVM submodule is unnecessary for self-tests.

Diff for: src/bootstrap/src/core/config/tests.rs

@@ -454,3 +454,53 @@ fn check_rustc_if_unchanged_paths() {
         assert!(config.src.join(p).exists(), "{p} doesn't exist.");
     }
 }
+
+#[test]
+fn override_profile_for_non_git_sources() {
+    let source_path = "path/to/repo.tarball";
+    let is_git_source = source_path.contains(".git");
+
+    if !is_git_source {
+        let mut config = parse(&format!("rust_info.source_path = \"{}\"", source_path));
+
+        if config.profile.is_none() {
+            config.profile = Some("dist".to_string());
+        }
+        assert_eq!(config.profile, Some("dist".to_string()));
+
+        if !is_git_source {
+            assert_eq!(config.profile, Some("dist".to_string()));
+        }
+    }
+}
+
+#[test]
+fn check_if_unchanged_on_non_git_sources() {
+    let source_types = ["tarball", "other-source-type", "path/to/repo.git"];
+
+    for &source_type in source_types.iter() {
+        let is_git_source = source_type.contains(".git");
+
+        if !is_git_source {
+            let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
+
+            if !is_git_source {
+                println!(
+                    "ERROR: 'if-unchanged' is not compatible with {} sources. Aborting `download-ci-llvm`.",
+                    source_type
+                );
+                crate::exit!(1);
+            }
+
+            let rustc_if_unchanged_config = parse("rust.download-rustc = \"if-unchanged\"");
+
+            if !is_git_source {
+                println!(
+                    "ERROR: 'if-unchanged' is not compatible with {} sources. Aborting `download-rustc`.",
+                    source_type
+                );
+                crate::exit!(1);
+            }
+        }
+    }
+}