From 68c195163a0fdc9b29c55df7b307892fd28f9473 Mon Sep 17 00:00:00 2001 From: joshlf's Agent Date: Tue, 25 Aug 2026 02:13:08 +0000 Subject: [PATCH] [ci] Read docs.rs arguments from Cargo metadata The nightly documentation command must include the exact Rustdoc arguments that docs.rs uses. Keeping another list in the command model would let the two copies drift. Read the ordered arguments from the canonical Zerocopy package selected by CI policy. Reject missing or malformed metadata, ownership by the wrong package, and values whose argument boundaries the current RUSTDOCFLAGS transport cannot preserve. Expose only checked arguments to later planning code. Tests: cargo test -p zc --all-targets cargo clippy -p zc --all-targets --offline -- -D warnings gherrit-pr-id: Getgur4i3su6l2xcpgewl2bbvcskszm5r --- tools/zc/src/inventory.rs | 192 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/tools/zc/src/inventory.rs b/tools/zc/src/inventory.rs index c2fc24327f..47b680ddba 100644 --- a/tools/zc/src/inventory.rs +++ b/tools/zc/src/inventory.rs @@ -147,6 +147,10 @@ pub struct CargoPackage { // compiler than the package default, and even an otherwise unused target // must still be understood while Cargo loads the manifest. editions: BTreeSet, + // Retain Cargo's structured package metadata so validation can select the + // docs.rs command contract from the exact policy-owned Zerocopy package. + // Raw metadata is deliberately not exposed from the checked inventory. + metadata: serde_json::Value, features: BTreeMap>, dependencies: BTreeMap, // Keep package identity separate from the renamed dependency keys above. @@ -289,6 +293,7 @@ pub struct RepositoryInventory { policy_packages: BTreeMap, cargo_targets: BTreeSet, toolchain_versions: BTreeMap, + zerocopy_docs_rs_rustdoc_args: Vec, } impl RepositoryInventory { @@ -318,6 +323,17 @@ impl RepositoryInventory { pub fn toolchain_versions(&self) -> &BTreeMap { &self.toolchain_versions } + + /// Returns the ordered docs.rs Rustdoc arguments owned by Zerocopy. + /// + /// These are read from the canonical package selected by + /// `ci/zc.toml`, not from a package-name search or another workspace + /// member which happens to declare similar metadata. Inventory validation + /// has already rejected missing, empty, non-string, control-bearing, or + /// whitespace-bearing arguments. + pub fn zerocopy_docs_rs_rustdoc_args(&self) -> &[String] { + &self.zerocopy_docs_rs_rustdoc_args + } } /// A collection failure or an aggregated validation failure. @@ -1006,6 +1022,7 @@ impl CollectedRepository { manifest: manifest.clone(), rust_version: package.rust_version.as_ref().map(ToString::to_string), editions: cargo_package_editions(package), + metadata: package.metadata.clone(), features: package .features .iter() @@ -1079,6 +1096,8 @@ impl CollectedRepository { ); validate_workspace_package_classification(policy, &self.packages, &mut errors); validate_cargo_target_classification(&self.packages, &mut errors); + let zerocopy_docs_rs_rustdoc_args = + validate_zerocopy_docs_rs_rustdoc_args(policy, &self.packages, &mut errors); let mut policy_packages = BTreeMap::new(); for (id, package_policy) in policy.packages() { @@ -1146,6 +1165,7 @@ impl CollectedRepository { policy_packages, cargo_targets, toolchain_versions, + zerocopy_docs_rs_rustdoc_args, }) } @@ -1628,6 +1648,101 @@ fn validate_workspace_package_classification_from_manifests( } } +/// Extracts the docs.rs Rustdoc arguments from the one package which owns the +/// Zerocopy CI contract. +/// +/// Cargo exposes arbitrary TOML package metadata as JSON. Treating a missing +/// key, `null`, a scalar, or an object as an empty argument list would silently +/// weaken the nightly documentation command. This parser accepts exactly an +/// ordered, nonempty array of nonempty, whitespace-free strings and preserves +/// the declared order verbatim. The whitespace restriction is load-bearing: +/// the current workflow joins these elements into `RUSTDOCFLAGS`, whose parser +/// cannot preserve an argument-internal space. +fn validate_zerocopy_docs_rs_rustdoc_args( + policy: &Policy, + packages: &BTreeMap, + errors: &mut ErrorSink, +) -> Vec { + let Some(configured) = policy.packages().get(PRIMARY_PACKAGE_ID) else { + errors.push( + "packages.zerocopy", + "canonical Zerocopy package is absent, so docs.rs arguments have no owner", + ); + return Vec::new(); + }; + let manifest = configured.manifest().as_path(); + let location = format!("{}.package.metadata.docs.rs.rustdoc-args", manifest.display()); + let Some(package) = packages.get(manifest) else { + errors.push( + &location, + format!( + "canonical Zerocopy manifest `{}` is absent from Cargo metadata", + manifest.display() + ), + ); + return Vec::new(); + }; + if package.name != PRIMARY_PACKAGE_ID || package.manifest != manifest { + errors.push( + &location, + format!( + "metadata owner must be package `{PRIMARY_PACKAGE_ID}` at `{}`, found package `{}` at `{}`", + manifest.display(), + package.name, + package.manifest.display() + ), + ); + return Vec::new(); + } + + let Some(package_metadata) = package.metadata.as_object() else { + errors.push(&location, "package metadata must be a JSON object"); + return Vec::new(); + }; + let Some(docs) = package_metadata.get("docs").and_then(serde_json::Value::as_object) else { + errors.push(&location, "`package.metadata.docs` must be a table"); + return Vec::new(); + }; + let Some(docs_rs) = docs.get("rs").and_then(serde_json::Value::as_object) else { + errors.push(&location, "`package.metadata.docs.rs` must be a table"); + return Vec::new(); + }; + let Some(arguments) = docs_rs.get("rustdoc-args").and_then(serde_json::Value::as_array) else { + errors.push(&location, "`rustdoc-args` must be an array of strings"); + return Vec::new(); + }; + if arguments.is_empty() { + errors.push(&location, "`rustdoc-args` must not be empty"); + return Vec::new(); + } + + let mut parsed = Vec::with_capacity(arguments.len()); + for (index, argument) in arguments.iter().enumerate() { + let argument_location = format!("{location}[{index}]"); + let Some(argument) = argument.as_str() else { + errors.push(argument_location, "Rustdoc argument must be a string"); + continue; + }; + if argument.is_empty() { + errors.push(argument_location, "Rustdoc argument must not be empty"); + continue; + } + if argument.chars().any(char::is_control) { + errors.push(argument_location, "Rustdoc argument must not contain control characters"); + continue; + } + if argument.chars().any(char::is_whitespace) { + errors.push( + argument_location, + "Rustdoc argument must not contain whitespace because RUSTDOCFLAGS cannot preserve that argument boundary", + ); + continue; + } + parsed.push(argument.to_owned()); + } + parsed +} + fn relative_path(repository_root: &Path, path: &Path) -> Result { let resolved = resolve_repository_path(repository_root, path)?; let relative = resolved.strip_prefix(repository_root).map_err(|_| { @@ -3860,6 +3975,7 @@ mod tests { manifest: "example/Cargo.toml".into(), rust_version: Some("1.56.0".to_owned()), editions: ["2021".to_owned()].into_iter().collect(), + metadata: serde_json::Value::Null, features: [( "stable".to_owned(), root_members.iter().map(|member| (*member).to_owned()).collect(), @@ -3892,6 +4008,7 @@ mod tests { manifest: manifest.into(), rust_version: None, editions: ["2015".to_owned()].into_iter().collect(), + metadata: serde_json::Value::Null, features: BTreeMap::new(), dependencies: BTreeMap::new(), workspace_dependencies: workspace_dependencies.iter().map(PathBuf::from).collect(), @@ -4600,6 +4717,7 @@ mod tests { manifest: manifest.into(), rust_version: None, editions: ["2015".to_owned()].into_iter().collect(), + metadata: serde_json::Value::Null, features: BTreeMap::new(), dependencies: BTreeMap::new(), workspace_dependencies: BTreeSet::new(), @@ -4974,6 +5092,7 @@ mod tests { manifest: manifest.clone(), rust_version: None, editions: ["2018".to_owned()].into_iter().collect(), + metadata: serde_json::Value::Null, features: BTreeMap::new(), dependencies: BTreeMap::new(), workspace_dependencies: BTreeSet::new(), @@ -5834,6 +5953,79 @@ mod tests { .cargo_targets() .iter() .any(|target| target.name() == "ui" && target.package() == "zerocopy")); + assert_eq!( + inventory.zerocopy_docs_rs_rustdoc_args(), + [ + "--cfg", + "doc_cfg", + "--generate-link-to-definition", + "--extend-css", + "rustdoc/style.css", + ] + ); + } + + #[test] + fn docs_rs_rustdoc_args_fail_closed_on_malformed_metadata() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + let policy = crate::policy::Policy::read(root.join("ci/zc.toml")).unwrap(); + let collected = super::CollectedRepository::collect(&root, &policy).unwrap(); + + for (case, metadata) in [ + ("missing tables", serde_json::json!({})), + ( + "scalar instead of array", + serde_json::json!({"docs": {"rs": {"rustdoc-args": "--cfg"}}}), + ), + ("empty array", serde_json::json!({"docs": {"rs": {"rustdoc-args": []}}})), + ("non-string argument", serde_json::json!({"docs": {"rs": {"rustdoc-args": [true]}}})), + ("empty argument", serde_json::json!({"docs": {"rs": {"rustdoc-args": [""]}}})), + ( + "control character", + serde_json::json!({"docs": {"rs": {"rustdoc-args": ["bad\nargument"]}}}), + ), + ( + "argument-internal whitespace", + serde_json::json!({"docs": {"rs": {"rustdoc-args": ["two words"]}}}), + ), + ] { + let mut mutation = collected.clone(); + mutation.packages.get_mut(Path::new("zerocopy/Cargo.toml")).unwrap().metadata = + metadata; + + let errors = mutation.validate(&policy).unwrap_err(); + assert!( + errors.errors().iter().any(|error| { + error + .location() + .starts_with("zerocopy/Cargo.toml.package.metadata.docs.rs.rustdoc-args") + }), + "{case}: {errors}" + ); + } + } + + #[test] + fn docs_rs_arguments_are_owned_only_by_the_canonical_package() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + let policy = crate::policy::Policy::read(root.join("ci/zc.toml")).unwrap(); + let mut collected = super::CollectedRepository::collect(&root, &policy).unwrap(); + let metadata = + collected.packages.get(Path::new("zerocopy/Cargo.toml")).unwrap().metadata.clone(); + collected.packages.get_mut(Path::new("zerocopy/Cargo.toml")).unwrap().metadata = + serde_json::json!({}); + collected + .packages + .get_mut(Path::new("zerocopy/zerocopy-derive/Cargo.toml")) + .unwrap() + .metadata = metadata; + + let errors = collected.validate(&policy).unwrap_err(); + assert!( + errors.errors().iter().any(|error| error.location() + == "zerocopy/Cargo.toml.package.metadata.docs.rs.rustdoc-args"), + "{errors}" + ); } #[test]