diff --git a/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs b/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs index 8d4c50842..b9a52c1b1 100644 --- a/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs +++ b/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs @@ -97,9 +97,17 @@ fn load_metadata_from_rustwide( workspace: &Workspace, toolchain: &Toolchain, source_dir: &Path, + host_unstable_flags: &[String], ) -> Result { + let mut args = vec!["metadata", "--format-version", "1"]; + // Add whitelisted unstable flags (currently `bindeps`) for host-side `cargo metadata`. + // See https://github.com/rust-lang/docs.rs/issues/2710 + let host_unstable_flag_refs: Vec<&str> = + host_unstable_flags.iter().map(|s| s.as_str()).collect(); + args.extend(host_unstable_flag_refs); + let res = Command::new(workspace, toolchain.cargo()) - .args(&["metadata", "--format-version", "1"]) + .args(&args) .cd(source_dir) .log_output(false) .run_capture()?; @@ -487,10 +495,17 @@ impl RustwideBuilder { } pub fn build_local_package(&mut self, path: &Path) -> Result { - let metadata = load_metadata_from_rustwide(&self.workspace, &self.toolchain, path) - .map_err(|err| { - err.context(format!("failed to load local package {}", path.display())) - })?; + // Read docs.rs metadata first to get host-side unstable cargo flags (e.g., `-Z bindeps`). + let docsrs_metadata = Metadata::from_crate_root(path).unwrap_or_default(); + let host_unstable_flags = docsrs_metadata.unstable_cargo_flags(); + + let metadata = load_metadata_from_rustwide( + &self.workspace, + &self.toolchain, + path, + &host_unstable_flags, + ) + .map_err(|err| err.context(format!("failed to load local package {}", path.display())))?; let package = metadata.root(); self.build_package( &package @@ -686,18 +701,34 @@ impl RustwideBuilder { if !res.successful() && cargo_lock.exists() { info!("removing lockfile and reattempting build"); std::fs::remove_file(cargo_lock)?; + + // Get host-side unstable cargo flags for host-side cargo commands. + let host_unstable_flags = metadata.unstable_cargo_flags(); + { let _span = info_span!("cargo_generate_lockfile").entered(); + let mut args = vec!["generate-lockfile"]; + let flags_refs: Vec<&str> = host_unstable_flags + .iter() + .map(|s| s.as_str()) + .collect(); + args.extend(flags_refs.iter()); Command::new(&self.workspace, self.toolchain.cargo()) .cd(build.host_source_dir()) - .args(&["generate-lockfile"]) + .args(&args) .run_capture()?; } { let _span = info_span!("cargo fetch --locked").entered(); + let mut args = vec!["fetch", "--locked"]; + let flags_refs: Vec<&str> = host_unstable_flags + .iter() + .map(|s| s.as_str()) + .collect(); + args.extend(flags_refs.iter()); Command::new(&self.workspace, self.toolchain.cargo()) .cd(build.host_source_dir()) - .args(&["fetch", "--locked"]) + .args(&args) .run_capture()?; } res = @@ -1114,6 +1145,7 @@ impl RustwideBuilder { &self.workspace, &self.toolchain, &build.host_source_dir(), + &metadata.unstable_cargo_flags(), )?; let mut rustdoc_flags = vec![ @@ -1824,7 +1856,6 @@ mod tests { #[ignore] fn test_proc_macro(crate_: &'static str, version: Version) -> Result<()> { let env = TestEnvironment::new()?; - let crate_ = KrateName::from_static(crate_); let mut builder = env.build_builder()?; @@ -2079,7 +2110,6 @@ mod tests { #[ignore] fn test_no_implicit_features_for_optional_dependencies_with_dep_syntax() -> Result<()> { let env = TestEnvironment::new()?; - let krate = KrateName::from_static("optional-dep"); let mut builder = env.build_builder()?; @@ -2263,6 +2293,67 @@ mod tests { Ok(()) } + #[test] + #[ignore] // Requires full build environment + fn test_bindeps_metadata_with_unstable_flags() -> Result<()> { + let env = TestEnvironment::new()?; + let mut builder = env.build_builder()?; + builder.update_toolchain()?; + let crate_path = Path::new("tests/crates/bindeps-test"); + let metadata = Metadata::from_crate_root(crate_path)?; + let unstable_flags = metadata.unstable_cargo_flags(); + + // The test crate contains a real `artifact = "bin"` dependency, so metadata + // must fail without `-Zbindeps`. + assert!( + load_metadata_from_rustwide(&builder.workspace, &builder.toolchain, crate_path, &[]) + .is_err(), + "cargo metadata should fail without -Zbindeps", + ); + + // With the fix, host-side cargo metadata receives the whitelisted unstable flag. + assert!( + load_metadata_from_rustwide( + &builder.workspace, + &builder.toolchain, + crate_path, + &unstable_flags, + ) + .is_ok(), + "cargo metadata should succeed with -Zbindeps", + ); + + Ok(()) + } + + #[test] + #[ignore] // Requires full build environment + fn test_bindeps_crate_full_build() -> Result<()> { + // Full build path for a crate using artifact dependencies (`-Zbindeps`). + // + // This currently fails because rustwide's `Prepare::prepare()` runs + // `cargo metadata`, `cargo generate-lockfile`, and `cargo fetch` + // without the `-Zbindeps` flag, causing `InvalidCargoTomlSyntax` at + // the `validate_manifest` step — before our build closure even runs. + // + // Fixing this requires rustwide to accept extra cargo args for its + // prepare phase (see https://github.com/rust-lang/rustwide/pull/119). + // + // TODO: flip this assertion to `assert!(... .successful)` once rustwide + // is updated with extra_cargo_args support. + let env = TestEnvironment::new()?; + let mut builder = env.build_builder()?; + builder.update_toolchain()?; + + assert!( + !builder + .build_local_package(Path::new("tests/crates/bindeps-test"))? + .successful + ); + + Ok(()) + } + #[test] #[ignore] fn test_build_with_cpu_limit() -> Result<()> { diff --git a/crates/bin/docs_rs_builder/tests/crates/bindeps-test/Cargo.toml b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/Cargo.toml new file mode 100644 index 000000000..f2642b3df --- /dev/null +++ b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "bindeps-test" +version = "0.1.0" +edition = "2021" + +[package.metadata.docs.rs] +cargo-args = ["-Zbindeps"] + +[build-dependencies] +# This requires `-Z bindeps` to be accepted by cargo metadata/fetch. +bindeps-helper = { path = "bindeps-helper", artifact = "bin" } + +[lib] +name = "bindeps_test" +path = "src/lib.rs" diff --git a/crates/bin/docs_rs_builder/tests/crates/bindeps-test/bindeps-helper/Cargo.toml b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/bindeps-helper/Cargo.toml new file mode 100644 index 000000000..fd016b705 --- /dev/null +++ b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/bindeps-helper/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "bindeps-helper" +version = "0.1.0" +edition = "2021" + +[[bin]] +name = "bindeps-helper" +path = "src/main.rs" diff --git a/crates/bin/docs_rs_builder/tests/crates/bindeps-test/bindeps-helper/src/main.rs b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/bindeps-helper/src/main.rs new file mode 100644 index 000000000..f328e4d9d --- /dev/null +++ b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/bindeps-helper/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/crates/bin/docs_rs_builder/tests/crates/bindeps-test/src/lib.rs b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/src/lib.rs new file mode 100644 index 000000000..a3002880e --- /dev/null +++ b/crates/bin/docs_rs_builder/tests/crates/bindeps-test/src/lib.rs @@ -0,0 +1,9 @@ +//! Test crate for bindeps support +//! +//! This crate uses unstable cargo feature `bindeps` (artifact dependencies). +//! It should build on docs.rs when the fix for #2710 is applied. + +pub fn hello() -> &'static str { + "Hello from bindeps-test!" +} + diff --git a/crates/bin/docs_rs_web/templates/core/Cargo.toml.example b/crates/bin/docs_rs_web/templates/core/Cargo.toml.example index 9bc4706d9..60d2edaf9 100644 --- a/crates/bin/docs_rs_web/templates/core/Cargo.toml.example +++ b/crates/bin/docs_rs_web/templates/core/Cargo.toml.example @@ -48,4 +48,7 @@ rustdoc-args = ["--example-rustdoc-arg"] # List of command line arguments for `cargo`. # # These cannot be a subcommand, they may only be options. +# For security reasons, docs.rs only forwards a whitelist of unstable flags to +# host-side cargo commands (`metadata`, `fetch`, `generate-lockfile`). +# Currently that whitelist contains only `bindeps` (`-Zbindeps` or `-Z bindeps`). cargo-args = ["-Z", "build-std"] diff --git a/crates/bin/docs_rs_web/templates/core/about/metadata.html b/crates/bin/docs_rs_web/templates/core/about/metadata.html index 9660ea9ff..555ebc958 100644 --- a/crates/bin/docs_rs_web/templates/core/about/metadata.html +++ b/crates/bin/docs_rs_web/templates/core/about/metadata.html @@ -17,6 +17,13 @@

Metadata for custom builds

{% filter highlight("toml") %} {%- include "core/Cargo.toml.example" -%} {% endfilter %} + +

+ For security reasons, docs.rs only forwards a whitelist of unstable cargo flags to + host-side cargo commands (cargo metadata, cargo fetch, + cargo generate-lockfile). Currently, only bindeps is + supported (as -Zbindeps or -Z bindeps). +

{%- endblock body %} diff --git a/crates/lib/metadata/lib.rs b/crates/lib/metadata/lib.rs index bdc29c2cb..1024e7058 100644 --- a/crates/lib/metadata/lib.rs +++ b/crates/lib/metadata/lib.rs @@ -146,6 +146,37 @@ pub struct Metadata { additional_targets: Vec, } +impl Metadata { + /// Return unstable cargo flags from `cargo_args` that are allowed on host commands. + /// + /// Most host-side cargo invocations on docs.rs are not sandboxed, so only whitelisted + /// unstable flags are allowed here. + /// + /// Currently this includes only `bindeps`, accepted as either `-Zbindeps` or + /// `-Z bindeps`. + pub fn unstable_cargo_flags(&self) -> Vec { + let mut flags = Vec::new(); + let mut iter = self.cargo_args.iter(); + while let Some(arg) = iter.next() { + if arg == "-Z" { + // `-Z bindeps` style (two separate args) + if let Some(value) = iter.next() { + if value == "bindeps" { + flags.push("-Z".to_string()); + flags.push(value.clone()); + } + } + } else if let Some(value) = arg.strip_prefix("-Z") { + if value == "bindeps" { + // `-Zbindeps` style (single arg) + flags.push(arg.clone()); + } + } + } + flags + } +} + /// The targets that should be built for a crate. /// /// The `default_target` is the target to be used as the home page for that crate. @@ -530,6 +561,53 @@ mod test_parsing { let metadata = Metadata::from_str(manifest).unwrap(); assert!(!metadata.proc_macro); } + + #[test] + fn test_unstable_cargo_flags() { + // Test `-Zbindeps` style (single arg) + let manifest = r#" + [package] + name = "test" + [package.metadata.docs.rs] + cargo-args = ["-Zbindeps", "--some-other-arg"] + "#; + let metadata = Metadata::from_str(manifest).unwrap(); + assert_eq!(metadata.unstable_cargo_flags(), vec!["-Zbindeps"]); + + // Test `-Z bindeps` style (two separate args) + let manifest = r#" + [package] + name = "test" + [package.metadata.docs.rs] + cargo-args = ["-Z", "bindeps", "--other"] + "#; + let metadata = Metadata::from_str(manifest).unwrap(); + assert_eq!(metadata.unstable_cargo_flags(), vec!["-Z", "bindeps"]); + + // Test that only whitelisted flags are returned. + let manifest = r#" + [package] + name = "test" + [package.metadata.docs.rs] + cargo-args = ["-Zbindeps", "-Z", "build-std", "--offline"] + "#; + let metadata = Metadata::from_str(manifest).unwrap(); + assert_eq!(metadata.unstable_cargo_flags(), vec!["-Zbindeps"]); + + // Test no unstable flags + let manifest = r#" + [package] + name = "test" + [package.metadata.docs.rs] + cargo-args = ["--offline", "--locked"] + "#; + let metadata = Metadata::from_str(manifest).unwrap(); + assert!(metadata.unstable_cargo_flags().is_empty()); + + // Test empty cargo-args + let metadata = Metadata::default(); + assert!(metadata.unstable_cargo_flags().is_empty()); + } } #[cfg(test)] @@ -852,4 +930,40 @@ mod test_calculations { ]; assert_eq!(metadata.cargo_args(&[], &[]), expected_args); } + + #[test] + fn test_bindeps_cargo_args_parsing() { + use std::str::FromStr; + // Test that cargo-args with -Zbindeps is correctly parsed. + // This test demonstrates the issue: these flags are parsed but not + // passed to cargo metadata/fetch commands (see #2710). + let manifest = r#" + [package] + name = "test" + [package.metadata.docs.rs] + cargo-args = ["-Zbindeps"] + "#; + let metadata = Metadata::from_str(manifest).unwrap(); + assert_eq!(metadata.cargo_args, vec!["-Zbindeps"]); + + // After fix: unstable_cargo_flags() correctly extracts and returns the flags + assert_eq!(metadata.unstable_cargo_flags(), vec!["-Zbindeps"]); + } + + #[test] + fn test_bindeps_separate_args_parsing() { + use std::str::FromStr; + // Test -Z bindeps style (two separate args) + let manifest = r#" + [package] + name = "test" + [package.metadata.docs.rs] + cargo-args = ["-Z", "bindeps"] + "#; + let metadata = Metadata::from_str(manifest).unwrap(); + assert_eq!(metadata.cargo_args, vec!["-Z", "bindeps"]); + + // After fix: unstable_cargo_flags() correctly extracts and returns the flags + assert_eq!(metadata.unstable_cargo_flags(), vec!["-Z", "bindeps"]); + } }