Skip to content

Commit 8203f08

Browse files
redsun82Copilot
andcommitted
Merge branch 'main' into redsun82-rust-analyzer-update
Resolve the `qltest.rs` conflict by adopting main's `cargo_check` helper. Preserve our `--cap-lints=allow` by folding it into the unified `RUSTFLAGS` in `get_extra_env`, so the initial check and extraction stay cache-consistent and deny-by-default lints don't fail extraction of valid test sources. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 890cb55d-0437-4bd7-9268-87b8897ada01
2 parents 15c543e + 95c30d3 commit 8203f08

3 files changed

Lines changed: 54 additions & 22 deletions

File tree

rust/codeql-extractor.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ options:
3636
This value is an optional path to use as `CARGO_TARGET_DIR` for the internal
3737
cargo commands the extractor uses. Pointing it to a persistent directory may
3838
reduce execution time of consecutive extractor runs. By default, a new scratch
39-
directory is used for each run.
39+
directory is used for each extraction, while qltests use the test's `target`
40+
directory so artifacts can be reused across runs.
4041
type: string
4142
cargo_target:
4243
title: Target architecture

rust/extractor/src/config.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,21 @@ pub struct Config {
7777
}
7878

7979
impl Config {
80+
/// Returns the directory where Cargo should place its build cache.
81+
pub(crate) fn cargo_target_dir(&self) -> PathBuf {
82+
self.cargo_target_dir.clone().unwrap_or_else(|| {
83+
// When the `target` directory is not explicitly set, we default to
84+
// the relative `target` directory (cargo's default) when running
85+
// qltests. This directory is preserved, so subsequent builds
86+
// benefit from the cache.
87+
if self.qltest {
88+
PathBuf::from("target")
89+
} else {
90+
self.scratch_dir.join("target")
91+
}
92+
})
93+
}
94+
8095
pub fn extract() -> anyhow::Result<Config> {
8196
let args = argfile::expand_args(argfile::parse_fromfile, argfile::PREFIX)
8297
.context("expanding parameter files")?;
@@ -109,11 +124,21 @@ impl Config {
109124
figment.extract().context("loading configuration")
110125
}
111126

112-
fn get_extra_env(&self) -> FxHashMap<String, Option<String>> {
127+
pub(crate) fn get_extra_env(&self) -> FxHashMap<String, Option<String>> {
113128
let mut extra_env = FxHashMap::default();
114129
// RUSTUP_AUTO_INSTALL is set to 0 by rust-analyzer (https://github.com/rust-lang/rust-analyzer/issues/20719),
115130
// but we do want to allow rustup to auto-install toolchains if needed, so we set it to 1 here.
116131
extra_env.insert("RUSTUP_AUTO_INSTALL".to_owned(), Some("1".to_owned()));
132+
if self.qltest_cargo_check {
133+
// Match the `cargo check` invocation in `cargo_check` so Cargo reuses its
134+
// cache (it does not when `RUSTFLAGS` differ). `--cap-lints=allow` keeps
135+
// deny-by-default lints (e.g. `dangerous_implicit_autorefs` on recent
136+
// toolchains) from failing extraction of otherwise valid test sources.
137+
extra_env.insert(
138+
"RUSTFLAGS".to_owned(),
139+
Some("-Awarnings --cap-lints=allow".to_owned()),
140+
);
141+
}
117142
extra_env.extend(self.cargo_extra_env.clone());
118143
extra_env
119144
}
@@ -183,12 +208,7 @@ impl Config {
183208
.iter()
184209
.map(|p| join_path_buf(dir, p))
185210
.collect(),
186-
target_dir_config: Utf8PathBuf::from_path_buf(
187-
self.cargo_target_dir
188-
.clone()
189-
.unwrap_or_else(|| self.scratch_dir.join("target")),
190-
)
191-
.map_or(
211+
target_dir_config: Utf8PathBuf::from_path_buf(self.cargo_target_dir()).map_or(
192212
TargetDirectoryConfig::None,
193213
TargetDirectoryConfig::Directory,
194214
),

rust/extractor/src/qltest.rs

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,29 @@ fn set_sources(config: &mut Config) -> anyhow::Result<()> {
9090
Ok(())
9191
}
9292

93+
fn cargo_check(config: &Config) -> anyhow::Result<()> {
94+
let mut command = Command::new("cargo");
95+
command.env("CARGO_TARGET_DIR", config.cargo_target_dir());
96+
// Pass the extra environment variables to the initial `cargo check`.
97+
for (key, value) in config.get_extra_env() {
98+
match value {
99+
Some(value) => command.env(key, value),
100+
None => command.env_remove(key),
101+
};
102+
}
103+
let status = command
104+
.arg("check")
105+
.arg("-q")
106+
.status()
107+
.context("spawning cargo check")?;
108+
if status.success() {
109+
info!("cargo check successful");
110+
Ok(())
111+
} else {
112+
anyhow::bail!("requested cargo check failed");
113+
}
114+
}
115+
93116
pub(crate) fn prepare(config: &mut Config) -> anyhow::Result<()> {
94117
dump_lib()?;
95118
set_sources(config)?;
@@ -99,19 +122,7 @@ pub(crate) fn prepare(config: &mut Config) -> anyhow::Result<()> {
99122
dump_nightly_toolchain()?;
100123
}
101124
if config.qltest_cargo_check {
102-
// `--cap-lints=allow` keeps deny-by-default lints (e.g. `dangerous_implicit_autorefs`
103-
// on recent toolchains) from failing extraction of otherwise valid test sources.
104-
let status = Command::new("cargo")
105-
.env("RUSTFLAGS", "-Awarnings --cap-lints=allow")
106-
.arg("check")
107-
.arg("-q")
108-
.status()
109-
.context("spawning cargo check")?;
110-
if status.success() {
111-
info!("cargo check successful");
112-
} else {
113-
anyhow::bail!("requested cargo check failed");
114-
}
115-
};
125+
cargo_check(config)?;
126+
}
116127
Ok(())
117128
}

0 commit comments

Comments
 (0)