development/cargo
development/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo synced 2024-09-17 23:01:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #13589 - epage:toml, r=Muscraft

Browse source 
refactor(toml): Flatten manifest parsing

### What does this PR try to resolve?

This is just a clean up but the goals are
- Support diagnostics that show source by tracking `&str`, `ImDocument`, etc in `Manifest` by making each accessible in the creation of a `Manifest`
- Defer warning analysis until we know what is a local vs non-local workspace by refactoring warnings out into a dedicated step
- Centralize the logic for `cargo publish` stripping of dev-dependencies and their feature activations by allowing a `Summary` to be created from any "resolved" `TomlManifest`
- Enumerate all build targets in the "resolved" `TomlManifest` so they get included in `cargo publish`, reducing the work done on registry dependencies and resolving problems like #13456

Along the way, this fixed a bug where we were not reporting warnings from virtual manifests

### How should we test and review this PR?

### Additional information
This commit is contained in:
bors 2024-03-15 19:50:39 +00:00
parent 403fbe2b49 4ed67e3a33
commit 48fb957dcc
19 changed files with 287 additions and 230 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
Cargo.lock generated
View file

@ -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",

4
Cargo.toml
View file

@ -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"] }

2
crates/cargo-util-schemas/Cargo.toml
View file

@ -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

8
crates/cargo-util-schemas/src/manifest/mod.rs
View file

@ -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<Vec<String>>,
pub package: Option<Box<TomlPackage>>,
pub project: Option<Box<TomlPackage>>,
@ -51,7 +53,11 @@ pub struct TomlManifest {
pub workspace: Option<TomlWorkspace>,
pub badges: Option<InheritableBtreeMap>,
pub lints: Option<InheritableLints>,
// 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<String>,
}
impl TomlManifest {

4
crates/xtask-stale-label/src/main.rs
View file

@ -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::<Document>().expect("a toml");
let doc = toml.parse::<DocumentMut>().expect("a toml");
let autolabel = doc["autolabel"].as_table().expect("a toml table");
for (label, value) in autolabel.iter() {

4
src/bin/cargo/commands/remove.rs
View file

@ -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<bool> {
let mut manifest: toml_edit::Document =
let mut manifest: toml_edit::DocumentMut =
cargo_util::paths::read(workspace.root_manifest())?.parse()?;
let mut modified = false;

15
src/cargo/core/manifest.rs
View file

@ -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<TomlManifest>,
summary: Summary,
// this form of manifest:
targets: Vec<Target>,
default_kind: Option<CompileKind>,
forced_kind: Option<CompileKind>,
@ -56,7 +60,6 @@ pub struct Manifest {
replace: Vec<(PackageIdSpec, Dependency)>,
patch: HashMap<Url, Vec<Dependency>>,
workspace: WorkspaceConfig,
original: Rc<TomlManifest>,
unstable_features: Features,
edition: Edition,
rust_version: Option<RustVersion>,
@ -386,7 +389,9 @@ compact_debug! {
impl Manifest {
pub fn new(
resolved_toml: Rc<TomlManifest>,
summary: Summary,
default_kind: Option<CompileKind>,
forced_kind: Option<CompileKind>,
targets: Vec<Target>,
@ -405,14 +410,15 @@ impl Manifest {
rust_version: Option<RustVersion>,
im_a_teapot: Option<bool>,
default_run: Option<String>,
original: Rc<TomlManifest>,
metabuild: Option<Vec<String>>,
resolve_behavior: Option<ResolveBehavior>,
lint_rustflags: Vec<String>,
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<Url, Vec<Dependency>> {
&self.patch

2
src/cargo/core/package.rs
View file

@ -198,7 +198,7 @@ impl Package {
}
pub fn to_registry_toml(&self, ws: &Workspace<'_>) -> CargoResult<String> {
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))
}

10
src/cargo/core/workspace.rs
View file

@ -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))

12
src/cargo/ops/cargo_new.rs
View file

@ -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::<toml_edit::Document>() {
if let Ok(mut workspace_document) = root_manifest.parse::<toml_edit::DocumentMut>() {
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<bool> {
// 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<bool> {
if let Some(exclude) = workspace_document
.get("workspace")

7
src/cargo/ops/cargo_package.rs
View file

@ -453,11 +453,10 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
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.

49
src/cargo/ops/cargo_read_manifest.rs
View file

@ -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<PathBuf>)> {
) -> CargoResult<Package> {
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<PathBuf> {
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<bool>) -> 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();

2
src/cargo/ops/registry/publish.rs
View file

@ -420,7 +420,7 @@ fn transmit(
.map(|dep| dep.name.clone())
.collect::<BTreeSet<String>>();
let string_features = match manifest.original().features() {
let string_features = match manifest.resolved_toml().features() {
Some(features) => features
.iter()
.map(|(feat, values)| {

2
src/cargo/ops/vendor.rs
View file

@ -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,

2
src/cargo/sources/path.rs
View file

@ -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])
}
}

4
src/cargo/util/context/mod.rs
View file

@ -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 {

379
src/cargo/util/toml/mod.rs
View file

@ -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,38 +47,62 @@ pub fn read_manifest(
path: &Path,
source_id: SourceId,
gctx: &GlobalContext,
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
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 {
if !gctx.cli_unstable().script {
return Err(ManifestError::new(
anyhow::anyhow!("parsing `{}` requires `-Zscript`", path.display()),
path.into(),
))?;
}
contents = embedded::expand_manifest(&contents, path, gctx)
.map_err(|err| ManifestError::new(err, path.into()))?;
}
) -> CargoResult<EitherManifest> {
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))?;
read_manifest_from_str(&contents, path, embedded, source_id, gctx).map_err(|err| {
if err.is::<AlreadyPrintedError>() {
err
(|| {
if toml.package().is_some() {
to_real_manifest(toml, source_id, path, gctx).map(EitherManifest::Real)
} else {
ManifestError::new(
err.context(format!("failed to parse manifest at `{}`", path.display())),
path.into(),
)
.into()
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<String> {
let mut contents = paths::read(path)?;
if is_embedded(path) {
if !gctx.cli_unstable().script {
anyhow::bail!("parsing `{}` requires `-Zscript`", path.display());
}
contents = embedded::expand_manifest(&contents, path, gctx)?;
}
Ok(contents)
}
#[tracing::instrument(skip_all)]
fn parse_document(contents: &str) -> Result<toml_edit::ImDocument<String>, 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<String>,
) -> Result<manifest::TomlManifest, toml_edit::de::Error> {
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`
pub fn is_embedded(path: &Path) -> bool {
let ext = path.extension();
@ -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<PathBuf>)> {
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",);
}
}
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 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))
};
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);
}
Path::Some { parent }
| Path::NewtypeVariant { parent }
| Path::NewtypeStruct { parent } => stringify(dst, parent),
dst.push_str(&index.to_string());
}
Path::Map { parent, ref key } => {
stringify(dst, parent);
if !dst.is_empty() {
dst.push('.');
}
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<String>, warnings: &mut Vec<String>) {
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<PathBuf>)> {
) -> CargoResult<Manifest> {
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<PathBuf>)> {
) -> CargoResult<VirtualManifest> {
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<Dependency>,
source_id: SourceId,
nested_paths: &'a mut Vec<PathBuf>,
gctx: &'b GlobalContext,
warnings: &'a mut Vec<String>,
platform: Option<Platform>,
@ -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<P: ResolveToPath + Clone>(
dep: &manifest::TomlDependency<P>,
name: &str,
source_id: SourceId,
nested_paths: &mut Vec<PathBuf>,
gctx: &GlobalContext,
warnings: &mut Vec<String>,
platform: Option<Platform>,
@ -1865,7 +1876,6 @@ pub(crate) fn to_dependency<P: ResolveToPath + Clone>(
&mut ManifestContext {
deps: &mut Vec::new(),
source_id,
nested_paths,
gctx,
warnings,
platform,
@ -2034,7 +2044,6 @@ fn detailed_dep_to_dependency<P: ResolveToPath + Clone>(
}
(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 `..`.

4
src/cargo/util/toml_mut/manifest.rs
View file

@ -82,7 +82,7 @@ impl From<DepKind> 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<Self, Self::Err> {
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 })
}

1
tests/testsuite/build.rs
View file

@ -711,6 +711,7 @@ fn cargo_compile_with_invalid_non_numeric_dep_version() {
crossbeam = "y"
"#,
)
.file("src/lib.rs", "")
.build();
p.cargo("build")