Skip to content

Commit a0e8861

Browse files
cscheidclaude
andcommitted
fix(xtask): stop nested cargo from rebuilding the TLS stack every run (bd-awchm8w7)
Every xtask subcommand that shells out to a nested cargo (stage-doc-examples, verify, build-all, test) inherits the outer `cargo xtask` process's package env vars — most importantly CARGO_MANIFEST_DIR=.../crates/xtask. The `ring` build script (base of q2's rustls/reqwest TLS stack) fingerprints CARGO_MANIFEST_DIR, so the inherited value marks it dirty and forces a full multi-minute rebuild of that dependency closure on every nested invocation whenever the prior build came from a plain shell or `cargo build --workspace`. Diagnosed while investigating a "stage-doc-examples hangs" report: the hang was this rebuild, hidden by the `--quiet` flag on the nested `cargo run`. Fingerprint log: EnvVarChanged { name: "CARGO_MANIFEST_DIR", old_value: None, new_value: Some(".../crates/xtask") }. - Add util::strip_inherited_cargo_env / util::nested_command, which mark the package-scoped CARGO_* vars (CARGO_MANIFEST_DIR, CARGO_PKG_*, OUT_DIR, …) for removal so a nested cargo fingerprints as a fresh shell would. PATH, CARGO_HOME, RUSTFLAGS, etc. are left untouched. - Route stage_doc_examples, verify (run_command), build_all (run_command), and test through it. Drop `--quiet` from the stage-doc render so a genuine compile or lock-wait is visible instead of a silent hang. Verified end-to-end with CARGO_LOG fingerprint logging: after a clean-shell `cargo build --workspace`, the nested xtask builds show 0 TLS-stack rebuilds, stable in both directions. 80/80 xtask tests pass; clean -D warnings build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 49b3d14 commit a0e8861

5 files changed

Lines changed: 145 additions & 5 deletions

File tree

crates/xtask/src/build_all.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ fn run_command(
201201
) -> Result<()> {
202202
let mut cmd = Command::new(program);
203203
cmd.args(args).current_dir(dir);
204+
// Nested cargo/npm: strip the outer `cargo xtask`'s package env vars so a
205+
// child cargo doesn't spuriously rebuild q2's TLS-stack closure (an
206+
// inherited CARGO_MANIFEST_DIR flips the `ring` build script dirty). See
207+
// `util::strip_inherited_cargo_env` (bd-awchm8w7).
208+
crate::util::strip_inherited_cargo_env(&mut cmd);
204209

205210
if let Some(flags) = rustflags {
206211
cmd.env("RUSTFLAGS", flags);

crates/xtask/src/stage_doc_examples.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ use anyhow::{Context, Result};
2929
use serde::Deserialize;
3030
use std::fs;
3131
use std::path::Path;
32-
use std::process::Command;
3332

3433
use crate::create_worktree::repo_root;
3534

@@ -116,10 +115,16 @@ fn stage_one(
116115

117116
/// Render a single example project in place with `q2`.
118117
fn render_project(root: &Path, project: &Path) -> Result<()> {
119-
let status = Command::new("cargo")
120-
.current_dir(root)
121-
.args(["run", "--quiet", "--bin", "q2", "--", "render"])
122-
.arg(project)
118+
// We are a nested `cargo` invocation; strip the outer `cargo xtask`'s
119+
// package env vars so the child cargo fingerprints q2 as a fresh shell
120+
// would. Without this, an inherited `CARGO_MANIFEST_DIR` forces a full
121+
// rebuild of q2's TLS-stack dependency closure on every run. See
122+
// `util::strip_inherited_cargo_env` (bd-awchm8w7).
123+
let mut cmd = crate::util::nested_command("cargo");
124+
cmd.current_dir(root)
125+
.args(["run", "--bin", "q2", "--", "render"])
126+
.arg(project);
127+
let status = cmd
123128
.status()
124129
.context("spawning `cargo run --bin q2 -- render`")?;
125130
if !status.success() {

crates/xtask/src/test.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ pub fn run(extra_args: &[String], rustflags: Option<&str>) -> Result<()> {
3232

3333
let mut cmd = Command::new("cargo");
3434
cmd.args(&args_refs).current_dir(&project_root);
35+
// Nested cargo: strip the outer `cargo xtask`'s package env vars so a child
36+
// cargo doesn't spuriously rebuild q2's TLS-stack closure (an inherited
37+
// CARGO_MANIFEST_DIR flips the `ring` build script dirty). See
38+
// `crate::util::strip_inherited_cargo_env` (bd-awchm8w7).
39+
crate::util::strip_inherited_cargo_env(&mut cmd);
3540

3641
if let Some(flags) = rustflags {
3742
cmd.env("RUSTFLAGS", flags);

crates/xtask/src/util.rs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Small shared utilities for xtask subcommands.
22
33
use std::path::{Path, PathBuf};
4+
use std::process::Command;
45

56
/// Return a copy of `path` with forward slashes replaced by the platform's
67
/// main separator. No-op on POSIX where `MAIN_SEPARATOR == '/'`.
@@ -18,9 +19,128 @@ pub fn with_native_separators(path: &Path) -> PathBuf {
1819
PathBuf::from(s.replace('/', std::path::MAIN_SEPARATOR_STR))
1920
}
2021

22+
/// The package-scoped environment variables that `cargo` injects into a
23+
/// process it runs — including `cargo xtask` itself. When an xtask subcommand
24+
/// shells out to a *nested* `cargo`, these leak from the outer invocation into
25+
/// the child unless stripped.
26+
///
27+
/// This matters because some build scripts fingerprint these vars. Most
28+
/// importantly, `ring` (the base of q2's rustls/reqwest TLS stack) fingerprints
29+
/// `CARGO_MANIFEST_DIR`: when the nested cargo inherits
30+
/// `CARGO_MANIFEST_DIR=.../crates/xtask` from the outer `cargo xtask`, the
31+
/// build script reads as dirty (`EnvVarChanged { name: "CARGO_MANIFEST_DIR",
32+
/// old_value: None, new_value: Some(".../crates/xtask") }`) and the entire
33+
/// dependency closure rebuilds — a multi-minute compile on *every* nested
34+
/// invocation whenever the previous build came from a plain shell or
35+
/// `cargo build --workspace`. Stripping these vars makes the nested cargo
36+
/// fingerprint exactly as a fresh shell would. Cargo sets the correct
37+
/// per-crate values for the crates it actually builds. See bd-awchm8w7.
38+
///
39+
/// Only package-scoped vars are listed; environment essentials the child still
40+
/// needs (`PATH`, `CARGO_HOME`, `RUSTUP_*`, `RUSTFLAGS`, …) are deliberately
41+
/// left untouched.
42+
const INHERITED_CARGO_PKG_VARS: &[&str] = &[
43+
"CARGO_MANIFEST_DIR",
44+
"CARGO_MANIFEST_PATH",
45+
"CARGO_PKG_NAME",
46+
"CARGO_PKG_VERSION",
47+
"CARGO_PKG_VERSION_MAJOR",
48+
"CARGO_PKG_VERSION_MINOR",
49+
"CARGO_PKG_VERSION_PATCH",
50+
"CARGO_PKG_VERSION_PRE",
51+
"CARGO_PKG_AUTHORS",
52+
"CARGO_PKG_DESCRIPTION",
53+
"CARGO_PKG_REPOSITORY",
54+
"CARGO_PKG_HOMEPAGE",
55+
"CARGO_PKG_LICENSE",
56+
"CARGO_PKG_LICENSE_FILE",
57+
"CARGO_PKG_RUST_VERSION",
58+
"CARGO_PKG_README",
59+
"CARGO_CRATE_NAME",
60+
"CARGO_BIN_NAME",
61+
"CARGO_PRIMARY_PACKAGE",
62+
"OUT_DIR",
63+
];
64+
65+
/// Mark the inherited cargo package env vars (see [`INHERITED_CARGO_PKG_VARS`])
66+
/// for removal on `cmd`, so a *nested* `cargo`/`npm` invocation spawned from an
67+
/// xtask subcommand fingerprints exactly as a fresh shell would — avoiding the
68+
/// spurious full rebuild of the TLS-stack dependency closure that an inherited
69+
/// `CARGO_MANIFEST_DIR` triggers.
70+
///
71+
/// Use this for any nested tool invocation from xtask; prefer
72+
/// [`nested_command`] when constructing the `Command` from scratch.
73+
pub fn strip_inherited_cargo_env(cmd: &mut Command) -> &mut Command {
74+
for var in INHERITED_CARGO_PKG_VARS {
75+
cmd.env_remove(var);
76+
}
77+
cmd
78+
}
79+
80+
/// Construct a [`Command`] for `program` with the inherited cargo package env
81+
/// vars already stripped (see [`strip_inherited_cargo_env`]). This is the
82+
/// preferred constructor for any nested `cargo`/`npm` invocation from an xtask
83+
/// subcommand.
84+
pub fn nested_command(program: &str) -> Command {
85+
let mut cmd = Command::new(program);
86+
strip_inherited_cargo_env(&mut cmd);
87+
cmd
88+
}
89+
2190
#[cfg(test)]
2291
mod tests {
2392
use super::*;
93+
use std::collections::HashMap;
94+
95+
/// Collect a `Command`'s env overrides into a map of name -> Option<value>,
96+
/// where `None` means the variable is marked for removal in the child.
97+
fn env_overrides(cmd: &Command) -> HashMap<String, Option<String>> {
98+
cmd.get_envs()
99+
.map(|(k, v)| {
100+
(
101+
k.to_string_lossy().into_owned(),
102+
v.map(|v| v.to_string_lossy().into_owned()),
103+
)
104+
})
105+
.collect()
106+
}
107+
108+
#[test]
109+
fn strip_marks_inherited_cargo_pkg_vars_for_removal() {
110+
let mut cmd = Command::new("cargo");
111+
strip_inherited_cargo_env(&mut cmd);
112+
let env = env_overrides(&cmd);
113+
// The variable that actually caused the ring/rustls rebuild thrash.
114+
assert_eq!(env.get("CARGO_MANIFEST_DIR"), Some(&None));
115+
// A representative sample of the rest of the package-scoped vars.
116+
for var in ["CARGO_PKG_NAME", "CARGO_PKG_VERSION", "OUT_DIR"] {
117+
assert_eq!(
118+
env.get(var),
119+
Some(&None),
120+
"{var} should be marked for removal"
121+
);
122+
}
123+
}
124+
125+
#[test]
126+
fn strip_does_not_touch_unrelated_or_cargo_home() {
127+
// CARGO_HOME / PATH must survive — only package-scoped vars are stripped.
128+
let mut cmd = Command::new("cargo");
129+
strip_inherited_cargo_env(&mut cmd);
130+
let env = env_overrides(&cmd);
131+
assert!(
132+
!env.contains_key("CARGO_HOME"),
133+
"CARGO_HOME must not be stripped"
134+
);
135+
assert!(!env.contains_key("PATH"), "PATH must not be stripped");
136+
}
137+
138+
#[test]
139+
fn nested_command_strips_pkg_vars() {
140+
let cmd = nested_command("cargo");
141+
let env = env_overrides(&cmd);
142+
assert_eq!(env.get("CARGO_MANIFEST_DIR"), Some(&None));
143+
}
24144

25145
#[test]
26146
fn with_native_separators_is_noop_on_posix_like_paths() {

crates/xtask/src/verify.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,11 @@ fn run_command(
470470
) -> Result<()> {
471471
let mut cmd = Command::new(program);
472472
cmd.args(args).current_dir(dir);
473+
// Nested cargo/npm: strip the outer `cargo xtask`'s package env vars so a
474+
// child cargo doesn't spuriously rebuild q2's TLS-stack closure (an
475+
// inherited CARGO_MANIFEST_DIR flips the `ring` build script dirty). See
476+
// `util::strip_inherited_cargo_env` (bd-awchm8w7).
477+
crate::util::strip_inherited_cargo_env(&mut cmd);
473478

474479
if let Some(flags) = rustflags {
475480
cmd.env("RUSTFLAGS", flags);

0 commit comments

Comments
 (0)