Correctly handle --use-system-gcc

GuillaumeGomez · GuillaumeGomez · commit 588db24344dc · 2024-02-12T14:42:37.000+01:00
diff --git a/build_system/src/build.rs b/build_system/src/build.rs
@@ -207,7 +207,7 @@ fn build_codegen(args: &mut BuildArg) -> Result<(), String> {
     }
     run_command_with_output_and_env(&command, None, Some(&env))?;
 
-    args.config_info.setup(&mut env, None)?;
+    args.config_info.setup(&mut env, false)?;
 
     // We voluntarily ignore the error.
     let _ = fs::remove_dir_all("target/out");
@@ -229,7 +229,7 @@ pub fn run() -> Result<(), String> {
         Some(args) => args,
         None => return Ok(()),
     };
-    args.config_info.setup_gcc_path(None)?;
+    args.config_info.setup_gcc_path()?;
     build_codegen(&mut args)?;
     Ok(())
 }
diff --git a/build_system/src/cargo.rs b/build_system/src/cargo.rs
@@ -77,7 +77,7 @@ pub fn run() -> Result<(), String> {
     })?;
 
     let mut env: HashMap<String, String> = std::env::vars().collect();
-    ConfigInfo::default().setup(&mut env, None)?;
+    ConfigInfo::default().setup(&mut env, false)?;
     let toolchain = get_toolchain()?;
 
     let toolchain_version = rustc_toolchain_version_info(&toolchain)?;
diff --git a/build_system/src/config.rs b/build_system/src/config.rs
@@ -159,30 +159,17 @@ impl ConfigInfo {
         command
     }
 
-    pub fn setup_gcc_path(&mut self, override_gcc_path: Option<&str>) -> Result<(), String> {
+    pub fn setup_gcc_path(&mut self) -> Result<(), String> {
         let ConfigFile { gcc_path, .. } = ConfigFile::new(self.config_file.as_deref())?;
 
-        self.gcc_path = match override_gcc_path {
-            Some(path) => {
-                if gcc_path.is_some() {
-                    println!(
-                        "overriding setting from `{}`",
-                        self.config_file.as_deref().unwrap_or("config.toml")
-                    );
-                }
-                path.to_string()
-            }
+        self.gcc_path = match gcc_path {
+            Some(path) => path,
+            // FIXME: Once we support "download", rewrite this.
             None => {
-                match gcc_path {
-                    Some(path) => path,
-                    // FIXME: Once we support "download", rewrite this.
-                    None => {
-                        return Err(format!(
-                            "missing `gcc-path` value from `{}`",
-                            self.config_file.as_deref().unwrap_or("config.toml"),
-                        ))
-                    }
-                }
+                return Err(format!(
+                    "missing `gcc-path` value from `{}`",
+                    self.config_file.as_deref().unwrap_or("config.toml"),
+                ))
             }
         };
         Ok(())
@@ -191,12 +178,12 @@ impl ConfigInfo {
     pub fn setup(
         &mut self,
         env: &mut HashMap<String, String>,
-        override_gcc_path: Option<&str>,
+        use_system_gcc: bool,
     ) -> Result<(), String> {
         env.insert("CARGO_INCREMENTAL".to_string(), "0".to_string());
 
-        if self.gcc_path.is_empty() || override_gcc_path.is_some() {
-            self.setup_gcc_path(override_gcc_path)?;
+        if self.gcc_path.is_empty() && !use_system_gcc {
+            self.setup_gcc_path()?;
         }
         env.insert("GCC_PATH".to_string(), self.gcc_path.clone());
 
diff --git a/build_system/src/test.rs b/build_system/src/test.rs
@@ -31,7 +31,10 @@ fn get_runners() -> Runners {
         "--test-failing-rustc",
         ("Run failing rustc tests", test_failing_rustc),
     );
-    runners.insert("--projects", ("Run the tests of popular crates", test_projects));
+    runners.insert(
+        "--projects",
+        ("Run the tests of popular crates", test_projects),
+    );
     runners.insert("--test-libcore", ("Run libcore tests", test_libcore));
     runners.insert("--clean", ("Empty cargo target directory", clean));
     runners.insert("--build-sysroot", ("Build sysroot", build_sysroot));
@@ -109,7 +112,7 @@ fn show_usage() {
 struct TestArg {
     no_default_features: bool,
     build_only: bool,
-    gcc_path: Option<String>,
+    use_system_gcc: bool,
     runners: BTreeSet<String>,
     flags: Vec<String>,
     backend: Option<String>,
@@ -121,7 +124,6 @@ struct TestArg {
 
 impl TestArg {
     fn new() -> Result<Option<Self>, String> {
-        let mut use_system_gcc = false;
         let mut test_arg = Self::default();
 
         // We skip binary name and the `test` command.
@@ -147,7 +149,10 @@ impl TestArg {
                         return Err("Expected an argument after `--features`, found nothing".into())
                     }
                 },
-                "--use-system-gcc" => use_system_gcc = true,
+                "--use-system-gcc" => {
+                    println!("Using system GCC");
+                    test_arg.use_system_gcc = true;
+                }
                 "--build-only" => test_arg.build_only = true,
                 "--use-backend" => match args.next() {
                     Some(backend) if !backend.is_empty() => test_arg.backend = Some(backend),
@@ -180,11 +185,6 @@ impl TestArg {
                     }
                 }
             }
-
-            if use_system_gcc {
-                println!("Using system GCC");
-                test_arg.gcc_path = Some("gcc".to_string());
-            }
         }
         match (test_arg.current_part, test_arg.nb_parts) {
             (Some(_), Some(_)) | (None, None) => {}
@@ -703,7 +703,7 @@ fn test_projects(env: &Env, args: &TestArg) -> Result<(), String> {
         //"https://github.com/rust-lang/cargo", // TODO: very slow, only run on master?
     ];
 
-    let run_tests = |projects_path, iter: &mut dyn Iterator<Item=&&str>| -> Result<(), String> {
+    let run_tests = |projects_path, iter: &mut dyn Iterator<Item = &&str>| -> Result<(), String> {
         for project in iter {
             let clone_result = git_clone(project, Some(projects_path), true)?;
             let repo_path = Path::new(&clone_result.repo_dir);
@@ -727,8 +727,7 @@ fn test_projects(env: &Env, args: &TestArg) -> Result<(), String> {
         let start = current_part * count;
         // We remove the projects we don't want to test.
         run_tests(projects_path, &mut projects.iter().skip(start).take(count))?;
-    }
-    else {
+    } else {
         run_tests(projects_path, &mut projects.iter())?;
     }
 
@@ -1166,23 +1165,25 @@ pub fn run() -> Result<(), String> {
     };
     let mut env: HashMap<String, String> = std::env::vars().collect();
 
-    args.config_info.setup_gcc_path(None)?;
-    env.insert(
-        "LIBRARY_PATH".to_string(),
-        args.config_info.gcc_path.clone(),
-    );
-    env.insert(
-        "LD_LIBRARY_PATH".to_string(),
-        args.config_info.gcc_path.clone(),
-    );
+    if !args.use_system_gcc {
+        args.config_info.setup_gcc_path()?;
+        env.insert(
+            "LIBRARY_PATH".to_string(),
+            args.config_info.gcc_path.clone(),
+        );
+        env.insert(
+            "LD_LIBRARY_PATH".to_string(),
+            args.config_info.gcc_path.clone(),
+        );
+    }
 
     build_if_no_backend(&env, &args)?;
     if args.build_only {
         println!("Since it's build only, exiting...");
         return Ok(());
     }
 
-    args.config_info.setup(&mut env, args.gcc_path.as_deref())?;
+    args.config_info.setup(&mut env, args.use_system_gcc)?;
 
     if args.runners.is_empty() {
         run_all(&env, &args)?;
diff --git a/build_system/src/utils.rs b/build_system/src/utils.rs
@@ -254,7 +254,11 @@ pub struct CloneResult {
     pub repo_dir: String,
 }
 
-pub fn git_clone(to_clone: &str, dest: Option<&Path>, shallow_clone: bool) -> Result<CloneResult, String> {
+pub fn git_clone(
+    to_clone: &str,
+    dest: Option<&Path>,
+    shallow_clone: bool,
+) -> Result<CloneResult, String> {
     let repo_name = to_clone.split('/').last().unwrap();
     let repo_name = match repo_name.strip_suffix(".git") {
         Some(n) => n.to_string(),
