diff --git a/Cargo.lock b/Cargo.lock index ea8e4354359..df841251d86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -469,7 +469,7 @@ dependencies = [ [[package]] name = "cargo-util-schemas" -version = "0.2.1" +version = "0.3.0" dependencies = [ "semver", "serde", @@ -3418,9 +3418,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.6" +version = "0.22.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c1b5fd4128cc8d3e0cb74d4ed9a9cc7c7284becd4df68f5f940e1ad123606f6" +checksum = "18769cd1cec395d70860ceb4d932812a0b4d06b1a4bb336745a4d21b9496e992" dependencies = [ "indexmap", "serde", diff --git a/Cargo.toml b/Cargo.toml index b5b508c7b7f..28c475e6eb9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ cargo-platform = { path = "crates/cargo-platform", version = "0.1.5" } cargo-test-macro = { path = "crates/cargo-test-macro" } cargo-test-support = { path = "crates/cargo-test-support" } cargo-util = { version = "0.2.9", path = "crates/cargo-util" } -cargo-util-schemas = { version = "0.2.1", path = "crates/cargo-util-schemas" } +cargo-util-schemas = { version = "0.3.0", path = "crates/cargo-util-schemas" } cargo_metadata = "0.18.1" clap = "4.5.1" color-print = "0.3.5" @@ -97,7 +97,7 @@ tempfile = "3.10.1" thiserror = "1.0.57" time = { version = "0.3", features = ["parsing", "formatting", "serde"] } toml = "0.8.10" -toml_edit = { version = "0.22.6", features = ["serde"] } +toml_edit = { version = "0.22.7", features = ["serde"] } tracing = "0.1.40" # be compatible with rustc_log: https://github.com/rust-lang/rust/blob/e51e98dde6a/compiler/rustc_log/Cargo.toml#L9 tracing-chrome = "0.7.1" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } diff --git a/crates/cargo-util-schemas/Cargo.toml b/crates/cargo-util-schemas/Cargo.toml index 1fe4d1a3907..0352997ff23 100644 --- a/crates/cargo-util-schemas/Cargo.toml +++ b/crates/cargo-util-schemas/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-util-schemas" -version = "0.2.1" +version = "0.3.0" rust-version = "1.76.0" # MSRV:1 edition.workspace = true license.workspace = true diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 75f7f628bef..aa438c54c8d 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -6,6 +6,7 @@ //! - Keys that exist for bookkeeping but don't correspond to the schema have a `_` prefix use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::fmt::{self, Display, Write}; use std::path::PathBuf; use std::str; @@ -28,6 +29,7 @@ pub use rust_version::RustVersionError; #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct TomlManifest { + // when adding new fields, be sure to check whether `requires_package` should disallow them pub cargo_features: Option>, pub package: Option>, pub project: Option>, @@ -51,7 +53,11 @@ pub struct TomlManifest { pub workspace: Option, pub badges: Option, pub lints: Option, - // when adding new fields, be sure to check whether `requires_package` should disallow them + + /// Report unused keys (see also nested `_unused_keys`) + /// Note: this is populated by the caller, rather than automatically + #[serde(skip)] + pub _unused_keys: BTreeSet, } impl TomlManifest { diff --git a/crates/xtask-stale-label/src/main.rs b/crates/xtask-stale-label/src/main.rs index efcb52d01c1..34590995c69 100644 --- a/crates/xtask-stale-label/src/main.rs +++ b/crates/xtask-stale-label/src/main.rs @@ -15,7 +15,7 @@ use std::fmt::Write as _; use std::path::PathBuf; use std::process; -use toml_edit::Document; +use toml_edit::DocumentMut; fn main() { let pkg_root = std::env!("CARGO_MANIFEST_DIR"); @@ -31,7 +31,7 @@ fn main() { let mut passed = 0; let toml = std::fs::read_to_string(path).expect("read from file"); - let doc = toml.parse::().expect("a toml"); + let doc = toml.parse::().expect("a toml"); let autolabel = doc["autolabel"].as_table().expect("a toml table"); for (label, value) in autolabel.iter() { diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index 34e31e9fae4..25179487c93 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -160,7 +160,7 @@ fn parse_section(args: &ArgMatches) -> DepTable { /// Clean up the workspace.dependencies, profile, patch, and replace sections of the root manifest /// by removing dependencies which no longer have a reference to them. fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> { - let mut manifest: toml_edit::Document = + let mut manifest: toml_edit::DocumentMut = cargo_util::paths::read(workspace.root_manifest())?.parse()?; let mut is_modified = true; @@ -315,7 +315,7 @@ fn spec_has_match( /// Removes unused patches from the manifest fn gc_unused_patches(workspace: &Workspace<'_>, resolve: &Resolve) -> CargoResult { - let mut manifest: toml_edit::Document = + let mut manifest: toml_edit::DocumentMut = cargo_util::paths::read(workspace.root_manifest())?.parse()?; let mut modified = false; diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 9f4eb29ada5..0deda834bbf 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -41,7 +41,11 @@ impl EitherManifest { /// This is deserialized using the [`TomlManifest`] type. #[derive(Clone, Debug)] pub struct Manifest { + // alternate forms of manifests: + resolved_toml: Rc, summary: Summary, + + // this form of manifest: targets: Vec, default_kind: Option, forced_kind: Option, @@ -56,7 +60,6 @@ pub struct Manifest { replace: Vec<(PackageIdSpec, Dependency)>, patch: HashMap>, workspace: WorkspaceConfig, - original: Rc, unstable_features: Features, edition: Edition, rust_version: Option, @@ -386,7 +389,9 @@ compact_debug! { impl Manifest { pub fn new( + resolved_toml: Rc, summary: Summary, + default_kind: Option, forced_kind: Option, targets: Vec, @@ -405,14 +410,15 @@ impl Manifest { rust_version: Option, im_a_teapot: Option, default_run: Option, - original: Rc, metabuild: Option>, resolve_behavior: Option, lint_rustflags: Vec, embedded: bool, ) -> Manifest { Manifest { + resolved_toml, summary, + default_kind, forced_kind, targets, @@ -430,7 +436,6 @@ impl Manifest { unstable_features, edition, rust_version, - original, im_a_teapot, default_run, metabuild, @@ -495,8 +500,8 @@ impl Manifest { pub fn replace(&self) -> &[(PackageIdSpec, Dependency)] { &self.replace } - pub fn original(&self) -> &TomlManifest { - &self.original + pub fn resolved_toml(&self) -> &TomlManifest { + &self.resolved_toml } pub fn patch(&self) -> &HashMap> { &self.patch diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index d9151e1a9a6..59b342c53be 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -198,7 +198,7 @@ impl Package { } pub fn to_registry_toml(&self, ws: &Workspace<'_>) -> CargoResult { - let manifest = prepare_for_publish(self.manifest().original(), ws, self.root())?; + let manifest = prepare_for_publish(self.manifest().resolved_toml(), ws, self.root())?; let toml = toml::to_string_pretty(&manifest)?; Ok(format!("{}\n{}", MANIFEST_PREAMBLE, toml)) } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d4be9a43a11..e689933492a 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -420,7 +420,6 @@ impl<'gctx> Workspace<'gctx> { let source = SourceId::for_path(self.root())?; let mut warnings = Vec::new(); - let mut nested_paths = Vec::new(); let mut patch = HashMap::new(); for (url, deps) in config_patch.into_iter().flatten() { @@ -442,7 +441,6 @@ impl<'gctx> Workspace<'gctx> { dep, name, source, - &mut nested_paths, self.gctx, &mut warnings, /* platform */ None, @@ -1006,7 +1004,7 @@ impl<'gctx> Workspace<'gctx> { ); self.gctx.shell().warn(&msg) }; - if manifest.original().has_profiles() { + if manifest.resolved_toml().has_profiles() { emit_warning("profiles")?; } if !manifest.replace().is_empty() { @@ -1063,7 +1061,7 @@ impl<'gctx> Workspace<'gctx> { return Ok(p); } let source_id = SourceId::for_path(manifest_path.parent().unwrap())?; - let (package, _nested_paths) = ops::read_package(manifest_path, source_id, self.gctx)?; + let package = ops::read_package(manifest_path, source_id, self.gctx)?; loaded.insert(manifest_path.to_path_buf(), package.clone()); Ok(package) } @@ -1596,7 +1594,7 @@ impl<'gctx> Packages<'gctx> { Entry::Occupied(e) => Ok(e.into_mut()), Entry::Vacant(v) => { let source_id = SourceId::for_path(key)?; - let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, self.gctx)?; + let manifest = read_manifest(manifest_path, source_id, self.gctx)?; Ok(v.insert(match manifest { EitherManifest::Real(manifest) => { MaybePackage::Package(Package::new(manifest, manifest_path)) @@ -1746,7 +1744,7 @@ pub fn find_workspace_root( find_workspace_root_with_loader(manifest_path, gctx, |self_path| { let key = self_path.parent().unwrap(); let source_id = SourceId::for_path(key)?; - let (manifest, _nested_paths) = read_manifest(self_path, source_id, gctx)?; + let manifest = read_manifest(self_path, source_id, gctx)?; Ok(manifest .workspace_config() .get_ws_root(self_path, manifest_path)) diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index b0bcf6daca4..de3aa052bfa 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -768,7 +768,7 @@ fn mk(gctx: &GlobalContext, opts: &MkOptions<'_>) -> CargoResult<()> { write_ignore_file(path, &ignore, vcs)?; // Create `Cargo.toml` file with necessary `[lib]` and `[[bin]]` sections, if needed. - let mut manifest = toml_edit::Document::new(); + let mut manifest = toml_edit::DocumentMut::new(); manifest["package"] = toml_edit::Item::Table(toml_edit::Table::new()); manifest["package"]["name"] = toml_edit::value(name); manifest["package"]["version"] = toml_edit::value("0.1.0"); @@ -814,7 +814,7 @@ fn mk(gctx: &GlobalContext, opts: &MkOptions<'_>) -> CargoResult<()> { // Sometimes the root manifest is not a valid manifest, so we only try to parse it if it is. // This should not block the creation of the new project. It is only a best effort to // inherit the workspace package keys. - if let Ok(mut workspace_document) = root_manifest.parse::() { + if let Ok(mut workspace_document) = root_manifest.parse::() { let display_path = get_display_path(&root_manifest_path, &path)?; let can_be_a_member = can_be_workspace_member(&display_path, &workspace_document)?; // Only try to inherit the workspace stuff if the new package can be a member of the workspace. @@ -933,14 +933,14 @@ mod tests { // If the option is set, keep the value from the manifest. fn update_manifest_with_inherited_workspace_package_keys( opts: &MkOptions<'_>, - manifest: &mut toml_edit::Document, + manifest: &mut toml_edit::DocumentMut, workspace_package_keys: &toml_edit::Table, ) { if workspace_package_keys.is_empty() { return; } - let try_remove_and_inherit_package_key = |key: &str, manifest: &mut toml_edit::Document| { + let try_remove_and_inherit_package_key = |key: &str, manifest: &mut toml_edit::DocumentMut| { let package = manifest["package"] .as_table_mut() .expect("package is a table"); @@ -974,7 +974,7 @@ fn update_manifest_with_inherited_workspace_package_keys( /// with the new package in it. fn update_manifest_with_new_member( root_manifest_path: &Path, - workspace_document: &mut toml_edit::Document, + workspace_document: &mut toml_edit::DocumentMut, display_path: &str, ) -> CargoResult { // If the members element already exist, check if one of the patterns @@ -1048,7 +1048,7 @@ fn get_display_path(root_manifest_path: &Path, package_path: &Path) -> CargoResu // Check if the package can be a member of the workspace. fn can_be_workspace_member( display_path: &str, - workspace_document: &toml_edit::Document, + workspace_document: &toml_edit::DocumentMut, ) -> CargoResult { if let Some(exclude) = workspace_document .get("workspace") diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 4e6c0585dd3..1bc8f4309f0 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -453,11 +453,10 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult { let orig_resolve = ops::load_pkg_lockfile(ws)?; // Convert Package -> TomlManifest -> Manifest -> Package - let toml_manifest = prepare_for_publish(orig_pkg.manifest().original(), ws, orig_pkg.root())?; - let package_root = orig_pkg.root(); + let toml_manifest = + prepare_for_publish(orig_pkg.manifest().resolved_toml(), ws, orig_pkg.root())?; let source_id = orig_pkg.package_id().source_id(); - let (manifest, _nested_paths) = - to_real_manifest(toml_manifest, false, source_id, package_root, gctx)?; + let manifest = to_real_manifest(toml_manifest, source_id, orig_pkg.manifest_path(), gctx)?; let new_pkg = Package::new(manifest, orig_pkg.manifest_path()); // Regenerate Cargo.lock using the old one as a guide. diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index 0cfc7bb2b00..e8eaf1b43f9 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -3,7 +3,7 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; -use crate::core::{EitherManifest, Package, PackageId, SourceId}; +use crate::core::{EitherManifest, Manifest, Package, PackageId, SourceId}; use crate::util::errors::CargoResult; use crate::util::important_paths::find_project_manifest_exact; use crate::util::toml::read_manifest; @@ -15,13 +15,13 @@ pub fn read_package( path: &Path, source_id: SourceId, gctx: &GlobalContext, -) -> CargoResult<(Package, Vec)> { +) -> CargoResult { trace!( "read_package; path={}; source-id={}", path.display(), source_id ); - let (manifest, nested) = read_manifest(path, source_id, gctx)?; + let manifest = read_manifest(path, source_id, gctx)?; let manifest = match manifest { EitherManifest::Real(manifest) => manifest, EitherManifest::Virtual(..) => anyhow::bail!( @@ -31,7 +31,7 @@ pub fn read_package( ), }; - Ok((Package::new(manifest, path), nested)) + Ok(Package::new(manifest, path)) } pub fn read_packages( @@ -107,6 +107,44 @@ pub fn read_packages( } } +fn nested_paths(manifest: &Manifest) -> Vec { + let mut nested_paths = Vec::new(); + let resolved = manifest.resolved_toml(); + let dependencies = resolved + .dependencies + .iter() + .chain(resolved.build_dependencies()) + .chain(resolved.dev_dependencies()) + .chain( + resolved + .target + .as_ref() + .into_iter() + .flat_map(|t| t.values()) + .flat_map(|t| { + t.dependencies + .iter() + .chain(t.build_dependencies()) + .chain(t.dev_dependencies()) + }), + ); + for dep_table in dependencies { + for dep in dep_table.values() { + let cargo_util_schemas::manifest::InheritableDependency::Value(dep) = dep else { + continue; + }; + let cargo_util_schemas::manifest::TomlDependency::Detailed(dep) = dep else { + continue; + }; + let Some(path) = dep.path.as_ref() else { + continue; + }; + nested_paths.push(PathBuf::from(path.as_str())); + } + } + nested_paths +} + fn walk(path: &Path, callback: &mut dyn FnMut(&Path) -> CargoResult) -> CargoResult<()> { if !callback(path)? { trace!("not processing {}", path.display()); @@ -151,7 +189,7 @@ fn read_nested_packages( let manifest_path = find_project_manifest_exact(path, "Cargo.toml")?; - let (manifest, nested) = match read_manifest(&manifest_path, source_id, gctx) { + let manifest = match read_manifest(&manifest_path, source_id, gctx) { Err(err) => { // Ignore malformed manifests found on git repositories // @@ -174,6 +212,7 @@ fn read_nested_packages( EitherManifest::Real(manifest) => manifest, EitherManifest::Virtual(..) => return Ok(()), }; + let nested = nested_paths(&manifest); let pkg = Package::new(manifest, &manifest_path); let pkg_id = pkg.package_id(); diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 65b9a960730..5615825f3fc 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -420,7 +420,7 @@ fn transmit( .map(|dep| dep.name.clone()) .collect::>(); - let string_features = match manifest.original().features() { + let string_features = match manifest.resolved_toml().features() { Some(features) => features .iter() .map(|(feat, values)| { diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs index e785c6c4096..4413786feb2 100644 --- a/src/cargo/ops/vendor.rs +++ b/src/cargo/ops/vendor.rs @@ -363,7 +363,7 @@ fn cp_sources( let cksum = if dst.file_name() == Some(OsStr::new("Cargo.toml")) && pkg.package_id().source_id().is_git() { - let original_toml = toml::to_string_pretty(pkg.manifest().original())?; + let original_toml = toml::to_string_pretty(pkg.manifest().resolved_toml())?; let contents = format!("{}\n{}", MANIFEST_PREAMBLE, original_toml); copy_and_checksum( &dst, diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 415298eb7c7..2e4d9042038 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -110,7 +110,7 @@ impl<'gctx> PathSource<'gctx> { ops::read_packages(&self.path, self.source_id, self.gctx) } else { let path = self.path.join("Cargo.toml"); - let (pkg, _) = ops::read_package(&path, self.source_id, self.gctx)?; + let pkg = ops::read_package(&path, self.source_id, self.gctx)?; Ok(vec![pkg]) } } diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index 92b6e18279a..da89f40f2dc 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -1365,9 +1365,9 @@ impl GlobalContext { // We only want to allow "dotted key" (see https://toml.io/en/v1.0.0#keys) // expressions followed by a value that's not an "inline table" // (https://toml.io/en/v1.0.0#inline-table). Easiest way to check for that is to - // parse the value as a toml_edit::Document, and check that the (single) + // parse the value as a toml_edit::DocumentMut, and check that the (single) // inner-most table is set via dotted keys. - let doc: toml_edit::Document = arg.parse().with_context(|| { + let doc: toml_edit::DocumentMut = arg.parse().with_context(|| { format!("failed to parse value from --config argument `{arg}` as a dotted key expression") })?; fn non_empty(d: Option<&toml_edit::RawString>) -> bool { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 7470c333db5..9873e9f2a53 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -14,12 +14,11 @@ use cargo_util_schemas::manifest::{self, TomlManifest}; use itertools::Itertools; use lazycell::LazyCell; use pathdiff::diff_paths; -use tracing::{debug, trace}; use url::Url; use crate::core::compiler::{CompileKind, CompileTarget}; use crate::core::dependency::{Artifact, ArtifactTarget, DepKind}; -use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; +use crate::core::manifest::{ManifestMetadata, TargetSourcePath}; use crate::core::resolver::ResolveBehavior; use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue}; use crate::core::{Dependency, Manifest, PackageId, Summary, Target}; @@ -48,36 +47,60 @@ pub fn read_manifest( path: &Path, source_id: SourceId, gctx: &GlobalContext, -) -> CargoResult<(EitherManifest, Vec)> { - trace!( - "read_manifest; path={}; source-id={}", - path.display(), - source_id - ); - let mut contents = paths::read(path).map_err(|err| ManifestError::new(err, path.into()))?; - let embedded = is_embedded(path); - if embedded { +) -> CargoResult { + let contents = + read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?; + let document = + parse_document(&contents).map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?; + let toml = deserialize_toml(&document) + .map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?; + + (|| { + if toml.package().is_some() { + to_real_manifest(toml, source_id, path, gctx).map(EitherManifest::Real) + } else { + to_virtual_manifest(toml, source_id, path, gctx).map(EitherManifest::Virtual) + } + })() + .map_err(|err| { + ManifestError::new( + err.context(format!("failed to parse manifest at `{}`", path.display())), + path.into(), + ) + .into() + }) +} + +#[tracing::instrument(skip_all)] +fn read_toml_string(path: &Path, gctx: &GlobalContext) -> CargoResult { + let mut contents = paths::read(path)?; + if is_embedded(path) { if !gctx.cli_unstable().script { - return Err(ManifestError::new( - anyhow::anyhow!("parsing `{}` requires `-Zscript`", path.display()), - path.into(), - ))?; + anyhow::bail!("parsing `{}` requires `-Zscript`", path.display()); } - contents = embedded::expand_manifest(&contents, path, gctx) - .map_err(|err| ManifestError::new(err, path.into()))?; + contents = embedded::expand_manifest(&contents, path, gctx)?; } + Ok(contents) +} - read_manifest_from_str(&contents, path, embedded, source_id, gctx).map_err(|err| { - if err.is::() { - err - } else { - ManifestError::new( - err.context(format!("failed to parse manifest at `{}`", path.display())), - path.into(), - ) - .into() - } - }) +#[tracing::instrument(skip_all)] +fn parse_document(contents: &str) -> Result, toml_edit::de::Error> { + toml_edit::ImDocument::parse(contents.to_owned()).map_err(Into::into) +} + +#[tracing::instrument(skip_all)] +fn deserialize_toml( + document: &toml_edit::ImDocument, +) -> Result { + let mut unused = BTreeSet::new(); + let deserializer = toml_edit::de::Deserializer::from(document.clone()); + let mut document: manifest::TomlManifest = serde_ignored::deserialize(deserializer, |path| { + let mut key = String::new(); + stringify(&mut key, &path); + unused.insert(key); + })?; + document._unused_keys = unused; + Ok(document) } /// See also `bin/cargo/commands/run.rs`s `is_manifest_command` @@ -88,144 +111,84 @@ pub fn is_embedded(path: &Path) -> bool { (ext.is_none() && path.is_file()) } -/// Parse an already-loaded `Cargo.toml` as a Cargo manifest. -/// -/// This could result in a real or virtual manifest being returned. -/// -/// A list of nested paths is also returned, one for each path dependency -/// within the manifest. For virtual manifests, these paths can only -/// come from patched or replaced dependencies. These paths are not -/// canonicalized. -fn read_manifest_from_str( +fn emit_diagnostic( + e: toml_edit::de::Error, contents: &str, manifest_file: &Path, - embedded: bool, - source_id: SourceId, gctx: &GlobalContext, -) -> CargoResult<(EitherManifest, Vec)> { - let package_root = manifest_file.parent().unwrap(); - - let mut unused = BTreeSet::new(); - let deserializer = toml::de::Deserializer::new(contents); - let manifest: manifest::TomlManifest = match serde_ignored::deserialize(deserializer, |path| { - let mut key = String::new(); - stringify(&mut key, &path); - unused.insert(key); - }) { - Ok(manifest) => manifest, - Err(e) => { - let Some(span) = e.span() else { - return Err(e.into()); - }; - - let (line_num, column) = translate_position(&contents, span.start); - let source_start = contents[0..span.start] - .rfind('\n') - .map(|s| s + 1) - .unwrap_or(0); - let source_end = contents[span.end.saturating_sub(1)..] - .find('\n') - .map(|s| s + span.end) - .unwrap_or(contents.len()); - let source = &contents[source_start..source_end]; - // Make sure we don't try to highlight past the end of the line, - // but also make sure we are highlighting at least one character - let highlight_end = (column + contents[span].chars().count()) - .min(source.len()) - .max(column + 1); - // Get the path to the manifest, relative to the cwd - let manifest_path = diff_paths(manifest_file, gctx.cwd()) - .unwrap_or_else(|| manifest_file.to_path_buf()) - .display() - .to_string(); - let snippet = Snippet { - title: Some(Annotation { - id: None, - label: Some(e.message()), - annotation_type: AnnotationType::Error, - }), - footer: vec![], - slices: vec![Slice { - source: &source, - line_start: line_num + 1, - origin: Some(manifest_path.as_str()), - annotations: vec![SourceAnnotation { - range: (column, highlight_end), - label: "", - annotation_type: AnnotationType::Error, - }], - fold: false, - }], - }; - let renderer = Renderer::styled(); - writeln!(gctx.shell().err(), "{}", renderer.render(snippet))?; - return Err(AlreadyPrintedError::new(e.into()).into()); - } - }; - let add_unused = |warnings: &mut Warnings| { - for key in unused { - warnings.add_warning(format!("unused manifest key: {}", key)); - if key == "profiles.debug" { - warnings.add_warning("use `[profile.dev]` to configure debug builds".to_string()); - } - } +) -> anyhow::Error { + let Some(span) = e.span() else { + return e.into(); }; - if let Some(deps) = manifest - .workspace - .as_ref() - .and_then(|ws| ws.dependencies.as_ref()) - { - for (name, dep) in deps { - if dep.is_optional() { - bail!("{name} is optional, but workspace dependencies cannot be optional",); - } - if dep.is_public() { - bail!("{name} is public, but workspace dependencies cannot be public",); - } - } - } - return if manifest.project.is_some() || manifest.package.is_some() { - let (mut manifest, paths) = - to_real_manifest(manifest, embedded, source_id, package_root, gctx)?; - add_unused(manifest.warnings_mut()); - if manifest.targets().iter().all(|t| t.is_custom_build()) { - bail!( - "no targets specified in the manifest\n\ - either src/lib.rs, src/main.rs, a [lib] section, or \ - [[bin]] section must be present" - ) - } - Ok((EitherManifest::Real(manifest), paths)) - } else { - let (mut m, paths) = to_virtual_manifest(manifest, source_id, package_root, gctx)?; - add_unused(m.warnings_mut()); - Ok((EitherManifest::Virtual(m), paths)) + let (line_num, column) = translate_position(&contents, span.start); + let source_start = contents[0..span.start] + .rfind('\n') + .map(|s| s + 1) + .unwrap_or(0); + let source_end = contents[span.end.saturating_sub(1)..] + .find('\n') + .map(|s| s + span.end) + .unwrap_or(contents.len()); + let source = &contents[source_start..source_end]; + // Make sure we don't try to highlight past the end of the line, + // but also make sure we are highlighting at least one character + let highlight_end = (column + contents[span].chars().count()) + .min(source.len()) + .max(column + 1); + // Get the path to the manifest, relative to the cwd + let manifest_path = diff_paths(manifest_file, gctx.cwd()) + .unwrap_or_else(|| manifest_file.to_path_buf()) + .display() + .to_string(); + let snippet = Snippet { + title: Some(Annotation { + id: None, + label: Some(e.message()), + annotation_type: AnnotationType::Error, + }), + footer: vec![], + slices: vec![Slice { + source: &source, + line_start: line_num + 1, + origin: Some(manifest_path.as_str()), + annotations: vec![SourceAnnotation { + range: (column, highlight_end), + label: "", + annotation_type: AnnotationType::Error, + }], + fold: false, + }], }; + let renderer = Renderer::styled(); + if let Err(err) = writeln!(gctx.shell().err(), "{}", renderer.render(snippet)) { + return err.into(); + } + return AlreadyPrintedError::new(e.into()).into(); +} - fn stringify(dst: &mut String, path: &serde_ignored::Path<'_>) { - use serde_ignored::Path; +fn stringify(dst: &mut String, path: &serde_ignored::Path<'_>) { + use serde_ignored::Path; - match *path { - Path::Root => {} - Path::Seq { parent, index } => { - stringify(dst, parent); - if !dst.is_empty() { - dst.push('.'); - } - dst.push_str(&index.to_string()); + match *path { + Path::Root => {} + Path::Seq { parent, index } => { + stringify(dst, parent); + if !dst.is_empty() { + dst.push('.'); } - Path::Map { parent, ref key } => { - stringify(dst, parent); - if !dst.is_empty() { - dst.push('.'); - } - dst.push_str(key); + dst.push_str(&index.to_string()); + } + Path::Map { parent, ref key } => { + stringify(dst, parent); + if !dst.is_empty() { + dst.push('.'); } - Path::Some { parent } - | Path::NewtypeVariant { parent } - | Path::NewtypeStruct { parent } => stringify(dst, parent), + dst.push_str(key); } + Path::Some { parent } + | Path::NewtypeVariant { parent } + | Path::NewtypeStruct { parent } => stringify(dst, parent), } } @@ -238,6 +201,15 @@ fn warn_on_deprecated(new_path: &str, name: &str, kind: &str, warnings: &mut Vec )) } +fn warn_on_unused(unused: &BTreeSet, warnings: &mut Vec) { + for key in unused { + warnings.push(format!("unused manifest key: {}", key)); + if key == "profiles.debug" { + warnings.push("use `[profile.dev]` to configure debug builds".to_string()); + } + } +} + /// Prepares the manifest for publishing. // - Path and git components of dependency specifications are removed. // - License path is updated to point within the package. @@ -366,6 +338,7 @@ pub fn prepare_for_publish( badges: me.badges.clone(), cargo_features: me.cargo_features.clone(), lints: me.lints.clone(), + _unused_keys: Default::default(), }; strip_features(&mut manifest); return Ok(manifest); @@ -471,11 +444,10 @@ pub fn prepare_for_publish( #[tracing::instrument(skip_all)] pub fn to_real_manifest( me: manifest::TomlManifest, - embedded: bool, source_id: SourceId, - package_root: &Path, + manifest_file: &Path, gctx: &GlobalContext, -) -> CargoResult<(Manifest, Vec)> { +) -> CargoResult { fn get_ws( gctx: &GlobalContext, resolved_path: &Path, @@ -503,6 +475,8 @@ pub fn to_real_manifest( } } + let embedded = is_embedded(manifest_file); + let package_root = manifest_file.parent().unwrap(); if !package_root.is_dir() { bail!( "package root '{}' is not a directory", @@ -510,10 +484,26 @@ pub fn to_real_manifest( ); }; - let mut nested_paths = vec![]; + if let Some(deps) = me + .workspace + .as_ref() + .and_then(|ws| ws.dependencies.as_ref()) + { + for (name, dep) in deps { + if dep.is_optional() { + bail!("{name} is optional, but workspace dependencies cannot be optional",); + } + if dep.is_public() { + bail!("{name} is public, but workspace dependencies cannot be public",); + } + } + } + let mut warnings = vec![]; let mut errors = vec![]; + warn_on_unused(&me._unused_keys, &mut warnings); + // Parse features first so they will be available when parsing other parts of the TOML. let empty = Vec::new(); let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); @@ -722,8 +712,12 @@ pub fn to_real_manifest( &mut errors, )?; - if targets.is_empty() { - debug!("manifest has no build targets"); + if targets.iter().all(|t| t.is_custom_build()) { + bail!( + "no targets specified in the manifest\n\ + either src/lib.rs, src/main.rs, a [lib] section, or \ + [[bin]] section must be present" + ) } if let Err(conflict_targets) = unique_build_targets(&targets, package_root) { @@ -758,7 +752,6 @@ pub fn to_real_manifest( let mut manifest_ctx = ManifestContext { deps: &mut deps, source_id, - nested_paths: &mut nested_paths, gctx, warnings: &mut warnings, features: &features, @@ -1212,8 +1205,10 @@ pub fn to_real_manifest( workspace: false, lints, }), + _unused_keys: Default::default(), }; let mut manifest = Manifest::new( + Rc::new(resolved_toml), summary, default_kind, forced_kind, @@ -1233,7 +1228,6 @@ pub fn to_real_manifest( rust_version, package.im_a_teapot, package.default_run.clone(), - Rc::new(resolved_toml), package.metabuild.clone().map(|sov| sov.0), resolve_behavior, rustflags, @@ -1259,31 +1253,48 @@ pub fn to_real_manifest( manifest.feature_gate()?; - Ok((manifest, nested_paths)) + Ok(manifest) } fn to_virtual_manifest( me: manifest::TomlManifest, source_id: SourceId, - root: &Path, + manifest_file: &Path, gctx: &GlobalContext, -) -> CargoResult<(VirtualManifest, Vec)> { +) -> CargoResult { + let root = manifest_file.parent().unwrap(); + + if let Some(deps) = me + .workspace + .as_ref() + .and_then(|ws| ws.dependencies.as_ref()) + { + for (name, dep) in deps { + if dep.is_optional() { + bail!("{name} is optional, but workspace dependencies cannot be optional",); + } + if dep.is_public() { + bail!("{name} is public, but workspace dependencies cannot be public",); + } + } + } + for field in me.requires_package() { bail!("this virtual manifest specifies a `{field}` section, which is not allowed"); } - let mut nested_paths = Vec::new(); let mut warnings = Vec::new(); let mut deps = Vec::new(); let empty = Vec::new(); let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); let features = Features::new(cargo_features, gctx, &mut warnings, source_id.is_path())?; + warn_on_unused(&me._unused_keys, &mut warnings); + let (replace, patch) = { let mut manifest_ctx = ManifestContext { deps: &mut deps, source_id, - nested_paths: &mut nested_paths, gctx, warnings: &mut warnings, platform: None, @@ -1332,17 +1343,19 @@ fn to_virtual_manifest( bail!("virtual manifests must be configured with [workspace]"); } }; - Ok(( - VirtualManifest::new( - replace, - patch, - workspace_config, - profiles, - features, - resolve_behavior, - ), - nested_paths, - )) + let mut manifest = VirtualManifest::new( + replace, + patch, + workspace_config, + profiles, + features, + resolve_behavior, + ); + for warning in warnings { + manifest.warnings_mut().add_warning(warning); + } + + Ok(manifest) } fn replace( @@ -1433,7 +1446,6 @@ fn patch( struct ManifestContext<'a, 'b> { deps: &'a mut Vec, source_id: SourceId, - nested_paths: &'a mut Vec, gctx: &'b GlobalContext, warnings: &'a mut Vec, platform: Option, @@ -1529,7 +1541,7 @@ fn inheritable_from_path( }; let source_id = SourceId::for_path(workspace_path_root)?; - let (man, _) = read_manifest(&workspace_path, source_id, gctx)?; + let man = read_manifest(&workspace_path, source_id, gctx)?; match man.workspace_config() { WorkspaceConfig::Root(root) => { gctx.ws_roots @@ -1851,7 +1863,6 @@ pub(crate) fn to_dependency( dep: &manifest::TomlDependency

, name: &str, source_id: SourceId, - nested_paths: &mut Vec, gctx: &GlobalContext, warnings: &mut Vec, platform: Option, @@ -1865,7 +1876,6 @@ pub(crate) fn to_dependency( &mut ManifestContext { deps: &mut Vec::new(), source_id, - nested_paths, gctx, warnings, platform, @@ -2034,7 +2044,6 @@ fn detailed_dep_to_dependency( } (None, Some(path), _, _) => { let path = path.resolve(manifest_ctx.gctx); - manifest_ctx.nested_paths.push(path.clone()); // If the source ID for the package we're parsing is a path // source, then we normalize the path here to get rid of // components like `..`. diff --git a/src/cargo/util/toml_mut/manifest.rs b/src/cargo/util/toml_mut/manifest.rs index 3e3b4e69ae3..1fbfb9decca 100644 --- a/src/cargo/util/toml_mut/manifest.rs +++ b/src/cargo/util/toml_mut/manifest.rs @@ -82,7 +82,7 @@ impl From for DepTable { #[derive(Debug, Clone)] pub struct Manifest { /// Manifest contents as TOML data. - pub data: toml_edit::Document, + pub data: toml_edit::DocumentMut, } impl Manifest { @@ -225,7 +225,7 @@ impl str::FromStr for Manifest { /// Read manifest data from string fn from_str(input: &str) -> ::std::result::Result { - let d: toml_edit::Document = input.parse().context("Manifest not valid TOML")?; + let d: toml_edit::DocumentMut = input.parse().context("Manifest not valid TOML")?; Ok(Manifest { data: d }) } diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index c7c751b5052..d506605a853 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -711,6 +711,7 @@ fn cargo_compile_with_invalid_non_numeric_dep_version() { crossbeam = "y" "#, ) + .file("src/lib.rs", "") .build(); p.cargo("build")