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]