Rust: Reuse Cargo target dir when running QL tests

Author: paldepind

rust/codeql-extractor.yml

@@ -36,7 +36,8 @@ options:
       This value is an optional path to use as `CARGO_TARGET_DIR` for the internal
       cargo commands the extractor uses. Pointing it to a persistent directory may
       reduce execution time of consecutive extractor runs. By default, a new scratch
-      directory is used for each run.
+      directory is used for each extraction, while qltests use the test's `target`
+      directory so artifacts can be reused across runs.
     type: string
   cargo_target:
     title: Target architecture

rust/extractor/src/config.rs

@@ -76,6 +76,21 @@ pub struct Config {
 }
 
 impl Config {
+    /// Returns the directory where Cargo should place its build cache.
+    pub(crate) fn cargo_target_dir(&self) -> PathBuf {
+        self.cargo_target_dir.clone().unwrap_or_else(|| {
+            // When the `target` directory is not explicitly set, we default to
+            // the relative `target` directory (cargo's default) when running
+            // qltests. This directory is preserved, so subsequent builds
+            // benefit from the cache.
+            if self.qltest {
+                PathBuf::from("target")
+            } else {
+                self.scratch_dir.join("target")
+            }
+        })
+    }
+
     pub fn extract() -> anyhow::Result<Config> {
         let args = argfile::expand_args(argfile::parse_fromfile, argfile::PREFIX)
             .context("expanding parameter files")?;
@@ -108,11 +123,17 @@ impl Config {
         figment.extract().context("loading configuration")
     }
 
-    fn get_extra_env(&self) -> FxHashMap<String, Option<String>> {
+    pub(crate) fn get_extra_env(&self) -> FxHashMap<String, Option<String>> {
         let mut extra_env = FxHashMap::default();
         // RUSTUP_AUTO_INSTALL is set to 0 by rust-analyzer (https://github.com/rust-lang/rust-analyzer/issues/20719),
         // but we do want to allow rustup to auto-install toolchains if needed, so we set it to 1 here.
         extra_env.insert("RUSTUP_AUTO_INSTALL".to_owned(), Some("1".to_owned()));
+        if self.qltest_cargo_check {
+            // When running qltests we add this flag to match the `cargo check`
+            // invocation in the `cargo_check` function. This is neccessary as
+            // Cargo does not re-use the cache when `RUSTFLAGS` differ.
+            extra_env.insert("RUSTFLAGS".to_owned(), Some("-Awarnings".to_owned()));
+        }
         extra_env.extend(self.cargo_extra_env.clone());
         extra_env
     }
@@ -182,12 +203,7 @@ impl Config {
                     .iter()
                     .map(|p| join_path_buf(dir, p))
                     .collect(),
-                target_dir_config: Utf8PathBuf::from_path_buf(
-                    self.cargo_target_dir
-                        .clone()
-                        .unwrap_or_else(|| self.scratch_dir.join("target")),
-                )
-                .map_or(
+                target_dir_config: Utf8PathBuf::from_path_buf(self.cargo_target_dir()).map_or(
                     TargetDirectoryConfig::None,
                     TargetDirectoryConfig::Directory,
                 ),

rust/extractor/src/qltest.rs

@@ -90,6 +90,29 @@ fn set_sources(config: &mut Config) -> anyhow::Result<()> {
     Ok(())
 }
 
+fn cargo_check(config: &Config) -> anyhow::Result<()> {
+    let mut command = Command::new("cargo");
+    command.env("CARGO_TARGET_DIR", config.cargo_target_dir());
+    // Pass the extra environment variables to the initial `cargo check`.
+    for (key, value) in config.get_extra_env() {
+        match value {
+            Some(value) => command.env(key, value),
+            None => command.env_remove(key),
+        };
+    }
+    let status = command
+        .arg("check")
+        .arg("-q")
+        .status()
+        .context("spawning cargo check")?;
+    if status.success() {
+        info!("cargo check successful");
+        Ok(())
+    } else {
+        anyhow::bail!("requested cargo check failed");
+    }
+}
+
 pub(crate) fn prepare(config: &mut Config) -> anyhow::Result<()> {
     dump_lib()?;
     set_sources(config)?;
@@ -98,17 +121,7 @@ pub(crate) fn prepare(config: &mut Config) -> anyhow::Result<()> {
         dump_nightly_toolchain()?;
     }
     if config.qltest_cargo_check {
-        let status = Command::new("cargo")
-            .env("RUSTFLAGS", "-Awarnings")
-            .arg("check")
-            .arg("-q")
-            .status()
-            .context("spawning cargo check")?;
-        if status.success() {
-            info!("cargo check successful");
-        } else {
-            anyhow::bail!("requested cargo check failed");
-        }
-    };
+        cargo_check(config)?;
+    }
     Ok(())
 }