Skip to content

Commit f2f2850

Browse files
committed
fix: Pass unstable cargo flags to all cargo commands (#2710)
This commit fixes issue #2710 by ensuring that unstable cargo flags (e.g., `-Zbindeps`) from `cargo-args` are passed to ALL cargo commands, not just `cargo rustdoc`. Changes: - Add `Metadata::unstable_cargo_flags()` method to extract `-Z*` flags from `cargo-args` - Update `load_metadata_from_rustwide()` to accept `unstable_flags` parameter and pass them to `cargo metadata` - Update `build_local_package()` to read metadata and pass unstable flags - Update `execute_build()` to pass unstable flags to `cargo metadata` - Update fallback path (`cargo generate-lockfile`, `cargo fetch`) to pass unstable flags - Remove unused `.peekable()` in `unstable_cargo_flags()` method - Apply rustfmt formatting - Update test to expect success (with fix)
1 parent 5888c44 commit f2f2850

2 files changed

Lines changed: 130 additions & 53 deletions

File tree

crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs

Lines changed: 48 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,16 @@ fn load_metadata_from_rustwide(
114114
workspace: &Workspace,
115115
toolchain: &Toolchain,
116116
source_dir: &Path,
117+
unstable_flags: &[String],
117118
) -> Result<CargoMetadata> {
119+
let mut args = vec!["metadata", "--format-version", "1"];
120+
// Add unstable flags (e.g., `-Z bindeps`) to support crates using unstable cargo features.
121+
// See https://github.com/rust-lang/docs.rs/issues/2710
122+
let unstable_flags_refs: Vec<&str> = unstable_flags.iter().map(|s| s.as_str()).collect();
123+
args.extend(unstable_flags_refs);
124+
118125
let res = Command::new(workspace, toolchain.cargo())
119-
.args(&["metadata", "--format-version", "1"])
126+
.args(&args)
120127
.cd(source_dir)
121128
.log_output(false)
122129
.run_capture()?;
@@ -500,10 +507,15 @@ impl RustwideBuilder {
500507
}
501508

502509
pub fn build_local_package(&mut self, path: &Path) -> Result<BuildPackageSummary> {
503-
let metadata = load_metadata_from_rustwide(&self.workspace, &self.toolchain, path)
504-
.map_err(|err| {
505-
err.context(format!("failed to load local package {}", path.display()))
506-
})?;
510+
// Read docsrs metadata first to get unstable cargo flags (e.g., `-Z bindeps`)
511+
let docsrs_metadata = Metadata::from_crate_root(path).unwrap_or_default();
512+
let unstable_flags = docsrs_metadata.unstable_cargo_flags();
513+
514+
let metadata =
515+
load_metadata_from_rustwide(&self.workspace, &self.toolchain, path, &unstable_flags)
516+
.map_err(|err| {
517+
err.context(format!("failed to load local package {}", path.display()))
518+
})?;
507519
let package = metadata.root();
508520
self.build_package(
509521
&package.name,
@@ -686,18 +698,28 @@ impl RustwideBuilder {
686698
if !res.result.successful && cargo_lock.exists() {
687699
info!("removing lockfile and reattempting build");
688700
std::fs::remove_file(cargo_lock)?;
701+
702+
// Get unstable cargo flags for commands that need them (e.g., `-Z bindeps`)
703+
let unstable_flags = metadata.unstable_cargo_flags();
704+
689705
{
690706
let _span = info_span!("cargo_generate_lockfile").entered();
707+
let mut args = vec!["generate-lockfile"];
708+
let flags_refs: Vec<&str> = unstable_flags.iter().map(|s| s.as_str()).collect();
709+
args.extend(flags_refs.iter());
691710
Command::new(&self.workspace, self.toolchain.cargo())
692711
.cd(build.host_source_dir())
693-
.args(&["generate-lockfile"])
712+
.args(&args)
694713
.run_capture()?;
695714
}
696715
{
697716
let _span = info_span!("cargo fetch --locked").entered();
717+
let mut args = vec!["fetch", "--locked"];
718+
let flags_refs: Vec<&str> = unstable_flags.iter().map(|s| s.as_str()).collect();
719+
args.extend(flags_refs.iter());
698720
Command::new(&self.workspace, self.toolchain.cargo())
699721
.cd(build.host_source_dir())
700-
.args(&["fetch", "--locked"])
722+
.args(&args)
701723
.run_capture()?;
702724
}
703725
res =
@@ -1108,6 +1130,7 @@ impl RustwideBuilder {
11081130
&self.workspace,
11091131
&self.toolchain,
11101132
&build.host_source_dir(),
1133+
&metadata.unstable_cargo_flags(),
11111134
)?;
11121135

11131136
let mut rustdoc_flags = vec![
@@ -2229,54 +2252,33 @@ mod tests {
22292252
}
22302253

22312254
#[test]
2232-
#[ignore] // This test demonstrates the issue - it will fail without the fix
2233-
fn test_bindeps_crate_fails_without_unstable_flags() -> Result<()> {
2234-
// This test demonstrates issue #2710: crates using unstable cargo features
2235-
// like `bindeps` fail to build because the unstable flags from `cargo-args`
2236-
// are not passed to `cargo metadata` and `cargo fetch` commands.
2255+
#[ignore] // Requires full build environment
2256+
fn test_bindeps_crate_builds_with_unstable_flags() -> Result<()> {
2257+
// This test verifies that the fix for issue #2710 works: crates using
2258+
// unstable cargo features like `bindeps` can now build because the unstable
2259+
// flags from `cargo-args` are correctly passed to `cargo metadata` and
2260+
// `cargo fetch` commands.
22372261
//
2238-
// The test crate has `cargo-args = ["-Zbindeps"]` in its Cargo.toml,
2239-
// but `load_metadata_from_rustwide` doesn't accept these flags,
2240-
// causing `cargo metadata` to fail.
2241-
//
2242-
// Without the fix: This test will fail because cargo metadata fails
2243-
// With the fix: This test should pass
2262+
// The test crate has `cargo-args = ["-Zbindeps"]` in its Cargo.toml.
2263+
// With the fix, `load_metadata_from_rustwide` accepts unstable flags and
2264+
// passes them to `cargo metadata`, allowing the build to succeed.
22442265

22452266
let env = TestEnvironment::new_with_runtime()?;
22462267
let mut builder = env.build_builder()?;
22472268
builder.update_toolchain()?;
22482269

2249-
// This should fail without the fix because cargo metadata is called
2250-
// without the `-Zbindeps` flag, even though it's in cargo-args
2270+
// With the fix, cargo metadata is called with the `-Zbindeps` flag
2271+
// from cargo-args, allowing the build to succeed
22512272
let result = builder.build_local_package(Path::new("tests/crates/bindeps-test"));
22522273

2253-
// Without fix: this will fail (demonstrating the issue)
2254-
// With fix: this should succeed
2255-
assert!(
2256-
result.is_err(),
2257-
"build should fail without unstable flags fix - this demonstrates issue #2710"
2258-
);
2274+
assert!(result.is_ok(), "build should succeed with bindeps fix");
2275+
if let Ok(summary) = result {
2276+
assert!(
2277+
summary.successful,
2278+
"build should be successful with bindeps fix"
2279+
);
2280+
}
22592281

22602282
Ok(())
22612283
}
2262-
2263-
#[test]
2264-
fn test_load_metadata_signature_doesnt_accept_unstable_flags() {
2265-
// This test demonstrates the problem: `load_metadata_from_rustwide`
2266-
// doesn't accept unstable flags parameter, so flags from `cargo-args`
2267-
// cannot be passed to `cargo metadata`.
2268-
//
2269-
// Current signature: load_metadata_from_rustwide(workspace, toolchain, source_dir)
2270-
// Needed signature: load_metadata_from_rustwide(workspace, toolchain, source_dir, unstable_flags)
2271-
//
2272-
// This is a compile-time check - the function signature doesn't allow
2273-
// passing unstable flags, which is the root cause of issue #2710.
2274-
//
2275-
// FIXME: After fix, this test should be updated to verify that
2276-
// unstable flags ARE passed to cargo metadata.
2277-
2278-
// This test just documents the problem - the actual fix requires
2279-
// changing the function signature to accept unstable_flags parameter
2280-
assert!(true, "This test documents the problem - see issue #2710");
2281-
}
22822284
}

crates/lib/metadata/lib.rs

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,34 @@ pub struct Metadata {
146146
additional_targets: Vec<String>,
147147
}
148148

149+
impl Metadata {
150+
/// Return unstable cargo flags (`-Z*`) from `cargo_args`.
151+
///
152+
/// These flags need to be passed to all cargo invocations (e.g., `cargo metadata`,
153+
/// `cargo fetch`) to support unstable features like `bindeps`.
154+
///
155+
/// See <https://github.com/rust-lang/docs.rs/issues/2710>.
156+
pub fn unstable_cargo_flags(&self) -> Vec<String> {
157+
let mut flags = Vec::new();
158+
let mut iter = self.cargo_args.iter();
159+
while let Some(arg) = iter.next() {
160+
if arg.starts_with("-Z") {
161+
if arg == "-Z" {
162+
// `-Z bindeps` style (two separate args)
163+
if let Some(value) = iter.next() {
164+
flags.push("-Z".to_string());
165+
flags.push(value.clone());
166+
}
167+
} else {
168+
// `-Zbindeps` style (single arg)
169+
flags.push(arg.clone());
170+
}
171+
}
172+
}
173+
flags
174+
}
175+
}
176+
149177
/// The targets that should be built for a crate.
150178
///
151179
/// The `default_target` is the target to be used as the home page for that crate.
@@ -521,6 +549,56 @@ mod test_parsing {
521549
let metadata = Metadata::from_str(manifest).unwrap();
522550
assert!(!metadata.proc_macro);
523551
}
552+
553+
#[test]
554+
fn test_unstable_cargo_flags() {
555+
// Test `-Zbindeps` style (single arg)
556+
let manifest = r#"
557+
[package]
558+
name = "test"
559+
[package.metadata.docs.rs]
560+
cargo-args = ["-Zbindeps", "--some-other-arg"]
561+
"#;
562+
let metadata = Metadata::from_str(manifest).unwrap();
563+
assert_eq!(metadata.unstable_cargo_flags(), vec!["-Zbindeps"]);
564+
565+
// Test `-Z bindeps` style (two separate args)
566+
let manifest = r#"
567+
[package]
568+
name = "test"
569+
[package.metadata.docs.rs]
570+
cargo-args = ["-Z", "bindeps", "--other"]
571+
"#;
572+
let metadata = Metadata::from_str(manifest).unwrap();
573+
assert_eq!(metadata.unstable_cargo_flags(), vec!["-Z", "bindeps"]);
574+
575+
// Test multiple unstable flags
576+
let manifest = r#"
577+
[package]
578+
name = "test"
579+
[package.metadata.docs.rs]
580+
cargo-args = ["-Zbindeps", "-Z", "build-std", "--offline"]
581+
"#;
582+
let metadata = Metadata::from_str(manifest).unwrap();
583+
assert_eq!(
584+
metadata.unstable_cargo_flags(),
585+
vec!["-Zbindeps", "-Z", "build-std"]
586+
);
587+
588+
// Test no unstable flags
589+
let manifest = r#"
590+
[package]
591+
name = "test"
592+
[package.metadata.docs.rs]
593+
cargo-args = ["--offline", "--locked"]
594+
"#;
595+
let metadata = Metadata::from_str(manifest).unwrap();
596+
assert!(metadata.unstable_cargo_flags().is_empty());
597+
598+
// Test empty cargo-args
599+
let metadata = Metadata::default();
600+
assert!(metadata.unstable_cargo_flags().is_empty());
601+
}
524602
}
525603

526604
#[cfg(test)]
@@ -859,11 +937,8 @@ mod test_calculations {
859937
let metadata = Metadata::from_str(manifest).unwrap();
860938
assert_eq!(metadata.cargo_args, vec!["-Zbindeps"]);
861939

862-
// The problem: these unstable flags should be extracted and passed
863-
// to cargo metadata/fetch, but currently they are only used in cargo rustdoc.
864-
// This causes builds to fail for crates using bindeps.
865-
// FIXME: After fix, unstable_cargo_flags() should return ["-Zbindeps"]
866-
// assert_eq!(metadata.unstable_cargo_flags(), vec!["-Zbindeps"]);
940+
// After fix: unstable_cargo_flags() correctly extracts and returns the flags
941+
assert_eq!(metadata.unstable_cargo_flags(), vec!["-Zbindeps"]);
867942
}
868943

869944
#[test]
@@ -879,7 +954,7 @@ mod test_calculations {
879954
let metadata = Metadata::from_str(manifest).unwrap();
880955
assert_eq!(metadata.cargo_args, vec!["-Z", "bindeps"]);
881956

882-
// FIXME: After fix, unstable_cargo_flags() should return ["-Z", "bindeps"]
883-
// assert_eq!(metadata.unstable_cargo_flags(), vec!["-Z", "bindeps"]);
957+
// After fix: unstable_cargo_flags() correctly extracts and returns the flags
958+
assert_eq!(metadata.unstable_cargo_flags(), vec!["-Z", "bindeps"]);
884959
}
885960
}

0 commit comments

Comments
 (0)