From 5f529f792d81b537d816c08c4fe8b556f1a80337 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 14:54:30 -0600 Subject: [PATCH 01/20] test(toml): Verify missing package.name --- tests/testsuite/bad_config.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 57b84a17f90..5ac8c1216ad 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -470,6 +470,31 @@ expected `}` .run(); } +#[cargo_test] +fn cargo_toml_missing_package_name() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + "#, + ) + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] missing field `name` + --> Cargo.toml:2:16 + | +2 | [package] + | ^^^^^^^^^ + | + +"#]]) + .run(); +} + #[cargo_test] fn duplicate_binary_names() { let p = project() From d398ce60b160a13776f174478106b8a2d3b1c61a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 10 Feb 2025 14:28:21 -0600 Subject: [PATCH 02/20] fix(embedded): Make errors consistent --- src/cargo/util/toml/embedded.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 686fcb77a22..7e90ffb9bf9 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -65,13 +65,13 @@ pub(super) fn expand_manifest( cargo_util::paths::write_if_changed(&hacked_path, hacked_source)?; let manifest = expand_manifest_(&frontmatter, &hacked_path, gctx) - .with_context(|| format!("failed to parse manifest at {}", path.display()))?; + .with_context(|| format!("failed to parse manifest at `{}`", path.display()))?; let manifest = toml::to_string_pretty(&manifest)?; Ok(manifest) } else { let frontmatter = ""; let manifest = expand_manifest_(frontmatter, path, gctx) - .with_context(|| format!("failed to parse manifest at {}", path.display()))?; + .with_context(|| format!("failed to parse manifest at `{}`", path.display()))?; let manifest = toml::to_string_pretty(&manifest)?; Ok(manifest) } From b7db1833b851b81f4b3ae59e92bea194d9338e61 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 10 Feb 2025 14:03:43 -0600 Subject: [PATCH 03/20] test(embedded): Verify fields are unsupported --- tests/testsuite/script.rs | 400 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 400 insertions(+) diff --git a/tests/testsuite/script.rs b/tests/testsuite/script.rs index 0c0e19b16d7..5c6a46bb271 100644 --- a/tests/testsuite/script.rs +++ b/tests/testsuite/script.rs @@ -842,6 +842,406 @@ Hello world! .run(); } +#[cargo_test] +fn disallow_explicit_workspace() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +package.edition = "2021" + +[workspace] +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `workspace` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_lib() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +package.edition = "2021" + +[lib] +name = "script" +path = "script.rs" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `lib` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_bin() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +package.edition = "2021" + +[[bin]] +name = "script" +path = "script.rs" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `bin` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_example() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +package.edition = "2021" + +[[example]] +name = "script" +path = "script.rs" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `example` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_test() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +package.edition = "2021" + +[[test]] +name = "script" +path = "script.rs" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `test` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_bench() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +package.edition = "2021" + +[[bench]] +name = "script" +path = "script.rs" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `bench` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_build() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +build = "script.rs" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.build` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_links() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +links = "script" +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.links` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_autolib() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +autolib = true +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.autolib` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_autobins() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +autobins = true +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.autobins` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_autoexamples() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +autoexamples = true +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.autoexamples` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_autotests() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +autotests = true +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.autotests` is not allowed in embedded manifests + +"#]]) + .run(); +} + +#[cargo_test] +fn disallow_explicit_package_autobenches() { + let p = cargo_test_support::project() + .file( + "script.rs", + r#" +---- +[package] +edition = "2021" +autobenches = true +---- + +fn main() {} +"#, + ) + .build(); + + p.cargo("-Zscript -v script.rs") + .masquerade_as_nightly_cargo(&["script"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] failed to parse manifest at `[ROOT]/foo/script.rs` + +Caused by: + `package.autobenches` is not allowed in embedded manifests + +"#]]) + .run(); +} + #[cargo_test(nightly, reason = "edition2024 hasn't hit stable yet")] fn implicit_target_dir() { let script = ECHO_SCRIPT; From c1db5cf99cf0dc5f8b45c0b62f4b71b3be6c1df5 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 11:16:43 -0600 Subject: [PATCH 04/20] refactor(toml): Centralize is_embedded query Need access to this in more places. I decided to centralize it (instead of adding more calls) because of the fact that it touches the filesystem. --- src/cargo/ops/vendor.rs | 1 + src/cargo/util/toml/mod.rs | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs index c6c76f624da..44e2a2addd2 100644 --- a/src/cargo/ops/vendor.rs +++ b/src/cargo/ops/vendor.rs @@ -434,6 +434,7 @@ fn prepare_for_vendor( workspace_config, source_id, me.manifest_path(), + me.manifest().is_embedded(), gctx, &mut warnings, &mut errors, diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 4eed082eb69..c273e408673 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -66,8 +66,9 @@ pub fn read_manifest( let mut warnings = Default::default(); let mut errors = Default::default(); - let contents = - read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?; + let is_embedded = is_embedded(path); + let contents = read_toml_string(path, is_embedded, 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 original_toml = deserialize_toml(&document) @@ -104,12 +105,14 @@ pub fn read_manifest( workspace_config, source_id, path, + is_embedded, gctx, &mut warnings, &mut errors, ) .map(EitherManifest::Real) } else if normalized_toml.workspace.is_some() { + assert!(!is_embedded); to_virtual_manifest( contents, document, @@ -146,9 +149,9 @@ pub fn read_manifest( } #[tracing::instrument(skip_all)] -fn read_toml_string(path: &Path, gctx: &GlobalContext) -> CargoResult { +fn read_toml_string(path: &Path, is_embedded: bool, gctx: &GlobalContext) -> CargoResult { let mut contents = paths::read(path)?; - if is_embedded(path) { + if is_embedded { if !gctx.cli_unstable().script { anyhow::bail!("parsing `{}` requires `-Zscript`", path.display()); } @@ -1122,11 +1125,11 @@ pub fn to_real_manifest( workspace_config: WorkspaceConfig, source_id: SourceId, manifest_file: &Path, + is_embedded: bool, gctx: &GlobalContext, warnings: &mut Vec, _errors: &mut Vec, ) -> CargoResult { - let embedded = is_embedded(manifest_file); let package_root = manifest_file.parent().unwrap(); if !package_root.is_dir() { bail!( @@ -1598,7 +1601,7 @@ pub fn to_real_manifest( metabuild, resolve_behavior, rustflags, - embedded, + is_embedded, ); if manifest .normalized_toml() @@ -2672,6 +2675,7 @@ pub fn prepare_for_publish( workspace_config, source_id, me.manifest_path(), + me.manifest().is_embedded(), gctx, &mut warnings, &mut errors, From b6619af3639dbe6074e67d062209eb1cc26f3ffb Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 11:22:00 -0600 Subject: [PATCH 05/20] refactor(toml): Ensure package normalization has access to the file name --- src/cargo/util/toml/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index c273e408673..efcfa8e00a7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -320,7 +320,7 @@ fn normalize_toml( if let Some(original_package) = original_toml.package() { let package_name = &original_package.name; - let normalized_package = normalize_package_toml(original_package, package_root, &inherit)?; + let normalized_package = normalize_package_toml(original_package, manifest_file, &inherit)?; let edition = normalized_package .normalized_edition() .expect("previously normalized") @@ -545,9 +545,11 @@ fn normalize_patch<'a>( #[tracing::instrument(skip_all)] fn normalize_package_toml<'a>( original_package: &manifest::TomlPackage, - package_root: &Path, + manifest_file: &Path, inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, ) -> CargoResult> { + let package_root = manifest_file.parent().unwrap(); + let normalized_package = manifest::TomlPackage { edition: original_package .edition From 21f008e30c6f997779ff431a8d5dda81aebc987a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 11:29:13 -0600 Subject: [PATCH 06/20] refactor(toml): Pull out field normalization for TomlPackage --- src/cargo/util/toml/mod.rs | 277 +++++++++++++++++++++---------------- 1 file changed, 154 insertions(+), 123 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index efcfa8e00a7..0100a4be437 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -550,132 +550,163 @@ fn normalize_package_toml<'a>( ) -> CargoResult> { let package_root = manifest_file.parent().unwrap(); - let normalized_package = manifest::TomlPackage { - edition: original_package - .edition - .clone() - .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) - .transpose()? - .map(manifest::InheritableField::Value), - rust_version: original_package - .rust_version - .clone() - .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) - .transpose()? - .map(manifest::InheritableField::Value), - name: original_package.name.clone(), - version: original_package - .version - .clone() - .map(|value| field_inherit_with(value, "version", || inherit()?.version())) - .transpose()? - .map(manifest::InheritableField::Value), - authors: original_package - .authors - .clone() - .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) - .transpose()? - .map(manifest::InheritableField::Value), - build: targets::normalize_build(original_package.build.as_ref(), package_root), - metabuild: original_package.metabuild.clone(), - default_target: original_package.default_target.clone(), - forced_target: original_package.forced_target.clone(), - links: original_package.links.clone(), - exclude: original_package - .exclude - .clone() - .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) - .transpose()? - .map(manifest::InheritableField::Value), - include: original_package - .include - .clone() - .map(|value| field_inherit_with(value, "include", || inherit()?.include())) - .transpose()? - .map(manifest::InheritableField::Value), - publish: original_package - .publish - .clone() - .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) - .transpose()? - .map(manifest::InheritableField::Value), - workspace: original_package.workspace.clone(), - im_a_teapot: original_package.im_a_teapot.clone(), - autolib: Some(false), - autobins: Some(false), - autoexamples: Some(false), - autotests: Some(false), - autobenches: Some(false), - default_run: original_package.default_run.clone(), - description: original_package - .description - .clone() - .map(|value| field_inherit_with(value, "description", || inherit()?.description())) - .transpose()? - .map(manifest::InheritableField::Value), - homepage: original_package - .homepage - .clone() - .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) - .transpose()? - .map(manifest::InheritableField::Value), - documentation: original_package - .documentation - .clone() - .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) - .transpose()? - .map(manifest::InheritableField::Value), - readme: normalize_package_readme( - package_root, - original_package - .readme - .clone() - .map(|value| { - field_inherit_with(value, "readme", || inherit()?.readme(package_root)) - }) - .transpose()? - .as_ref(), - ) - .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))) - .or(Some(manifest::InheritableField::Value(StringOrBool::Bool( - false, - )))), - keywords: original_package - .keywords - .clone() - .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) - .transpose()? - .map(manifest::InheritableField::Value), - categories: original_package - .categories - .clone() - .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) - .transpose()? - .map(manifest::InheritableField::Value), - license: original_package - .license + let edition = original_package + .edition + .clone() + .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) + .transpose()? + .map(manifest::InheritableField::Value); + let rust_version = original_package + .rust_version + .clone() + .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) + .transpose()? + .map(manifest::InheritableField::Value); + let name = original_package.name.clone(); + let version = original_package + .version + .clone() + .map(|value| field_inherit_with(value, "version", || inherit()?.version())) + .transpose()? + .map(manifest::InheritableField::Value); + let authors = original_package + .authors + .clone() + .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) + .transpose()? + .map(manifest::InheritableField::Value); + let build = targets::normalize_build(original_package.build.as_ref(), package_root); + let metabuild = original_package.metabuild.clone(); + let default_target = original_package.default_target.clone(); + let forced_target = original_package.forced_target.clone(); + let links = original_package.links.clone(); + let exclude = original_package + .exclude + .clone() + .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) + .transpose()? + .map(manifest::InheritableField::Value); + let include = original_package + .include + .clone() + .map(|value| field_inherit_with(value, "include", || inherit()?.include())) + .transpose()? + .map(manifest::InheritableField::Value); + let publish = original_package + .publish + .clone() + .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) + .transpose()? + .map(manifest::InheritableField::Value); + let workspace = original_package.workspace.clone(); + let im_a_teapot = original_package.im_a_teapot.clone(); + let autolib = Some(false); + let autobins = Some(false); + let autoexamples = Some(false); + let autotests = Some(false); + let autobenches = Some(false); + let default_run = original_package.default_run.clone(); + let description = original_package + .description + .clone() + .map(|value| field_inherit_with(value, "description", || inherit()?.description())) + .transpose()? + .map(manifest::InheritableField::Value); + let homepage = original_package + .homepage + .clone() + .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) + .transpose()? + .map(manifest::InheritableField::Value); + let documentation = original_package + .documentation + .clone() + .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) + .transpose()? + .map(manifest::InheritableField::Value); + let readme = normalize_package_readme( + package_root, + original_package + .readme .clone() - .map(|value| field_inherit_with(value, "license", || inherit()?.license())) + .map(|value| field_inherit_with(value, "readme", || inherit()?.readme(package_root))) .transpose()? - .map(manifest::InheritableField::Value), - license_file: original_package - .license_file - .clone() - .map(|value| { - field_inherit_with(value, "license-file", || { - inherit()?.license_file(package_root) - }) + .as_ref(), + ) + .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))) + .or(Some(manifest::InheritableField::Value(StringOrBool::Bool( + false, + )))); + let keywords = original_package + .keywords + .clone() + .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) + .transpose()? + .map(manifest::InheritableField::Value); + let categories = original_package + .categories + .clone() + .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) + .transpose()? + .map(manifest::InheritableField::Value); + let license = original_package + .license + .clone() + .map(|value| field_inherit_with(value, "license", || inherit()?.license())) + .transpose()? + .map(manifest::InheritableField::Value); + let license_file = original_package + .license_file + .clone() + .map(|value| { + field_inherit_with(value, "license-file", || { + inherit()?.license_file(package_root) }) - .transpose()? - .map(manifest::InheritableField::Value), - repository: original_package - .repository - .clone() - .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) - .transpose()? - .map(manifest::InheritableField::Value), - resolver: original_package.resolver.clone(), - metadata: original_package.metadata.clone(), + }) + .transpose()? + .map(manifest::InheritableField::Value); + let repository = original_package + .repository + .clone() + .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) + .transpose()? + .map(manifest::InheritableField::Value); + let resolver = original_package.resolver.clone(); + let metadata = original_package.metadata.clone(); + + let normalized_package = manifest::TomlPackage { + edition, + rust_version, + name, + version, + authors, + build, + metabuild, + default_target, + forced_target, + links, + exclude, + include, + publish, + workspace, + im_a_teapot, + autolib, + autobins, + autoexamples, + autotests, + autobenches, + default_run, + description, + homepage, + documentation, + readme, + keywords, + categories, + license, + license_file, + repository, + resolver, + metadata, _invalid_cargo_features: Default::default(), }; From 8d6985679f2ff9fa205409989d5ec8ac527a930d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 11 Dec 2024 16:40:01 -0600 Subject: [PATCH 07/20] refactor(toml): Centralize package name lookup --- src/cargo/util/toml/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 0100a4be437..e963a93b9d5 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -334,7 +334,7 @@ fn normalize_toml( normalized_toml.lib = targets::normalize_lib( original_toml.lib.as_ref(), package_root, - &original_package.name, + package_name, edition, original_package.autolib, warnings, @@ -342,7 +342,7 @@ fn normalize_toml( normalized_toml.bin = Some(targets::normalize_bins( original_toml.bin.as_ref(), package_root, - &original_package.name, + package_name, edition, original_package.autobins, warnings, From 4da874fae0d014455177d94c80f46751847b9ec8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 14:08:58 -0600 Subject: [PATCH 08/20] refactor(toml): Centralize package name lookup --- src/cargo/util/toml/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e963a93b9d5..1c2b06e3d3a 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1515,7 +1515,7 @@ pub fn to_real_manifest( } let pkgid = PackageId::new( - normalized_package.name.as_str().into(), + package_name.as_str().into(), version .cloned() .unwrap_or_else(|| semver::Version::new(0, 0, 0)), From 7cd183386faca5ff85b5709f65d68f33f07e32db Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 11:34:06 -0600 Subject: [PATCH 09/20] refactor(toml): Track the normalized name, not original --- src/cargo/util/toml/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1c2b06e3d3a..ab51f0931ea 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -318,9 +318,8 @@ fn normalize_toml( }; if let Some(original_package) = original_toml.package() { - let package_name = &original_package.name; - let normalized_package = normalize_package_toml(original_package, manifest_file, &inherit)?; + let package_name = &normalized_package.name.clone(); let edition = normalized_package .normalized_edition() .expect("previously normalized") From f2a20cf3c1efa4bde68a92c841865c07f7a51915 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 11:40:25 -0600 Subject: [PATCH 10/20] refactor(toml): Track the normalized name, not original --- src/cargo/util/toml/mod.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index ab51f0931ea..7608bf61752 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1170,18 +1170,13 @@ pub fn to_real_manifest( ); }; - let original_package = original_toml + let normalized_package = normalized_toml .package() - .ok_or_else(|| anyhow::format_err!("no `package` section found"))?; - - let package_name = &original_package.name; + .expect("previously verified to have a `[package]`"); + let package_name = &normalized_package.name; if package_name.contains(':') { features.require(Feature::open_namespaces())?; } - - let normalized_package = normalized_toml - .package() - .expect("previously verified to have a `[package]`"); let rust_version = normalized_package .normalized_rust_version() .expect("previously normalized") From 95270822982e443c37a5d73daf0294dfc09c7951 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 12:00:51 -0600 Subject: [PATCH 11/20] refactor(embedded): Move package.build to normalization --- src/cargo/util/toml/embedded.rs | 5 ----- src/cargo/util/toml/mod.rs | 12 ++++++++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 7e90ffb9bf9..7da61c5e869 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -124,9 +124,6 @@ fn expand_manifest_( )); toml::Value::String(DEFAULT_EDITION.to_string()) }); - package - .entry("build".to_owned()) - .or_insert_with(|| toml::Value::Boolean(false)); for field in AUTO_FIELDS { package .entry(field.to_owned()) @@ -571,7 +568,6 @@ autobins = false autoexamples = false autolib = false autotests = false -build = false edition = "2024" name = "test-" @@ -606,7 +602,6 @@ autobins = false autoexamples = false autolib = false autotests = false -build = false edition = "2024" name = "test-" diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 7608bf61752..06b0b556137 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -90,6 +90,7 @@ pub fn read_manifest( &features, &workspace_config, path, + is_embedded, gctx, &mut warnings, &mut errors, @@ -274,6 +275,7 @@ fn normalize_toml( features: &Features, workspace_config: &WorkspaceConfig, manifest_file: &Path, + is_embedded: bool, gctx: &GlobalContext, warnings: &mut Vec, errors: &mut Vec, @@ -318,7 +320,8 @@ fn normalize_toml( }; if let Some(original_package) = original_toml.package() { - let normalized_package = normalize_package_toml(original_package, manifest_file, &inherit)?; + let normalized_package = + normalize_package_toml(original_package, manifest_file, is_embedded, &inherit)?; let package_name = &normalized_package.name.clone(); let edition = normalized_package .normalized_edition() @@ -545,6 +548,7 @@ fn normalize_patch<'a>( fn normalize_package_toml<'a>( original_package: &manifest::TomlPackage, manifest_file: &Path, + is_embedded: bool, inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, ) -> CargoResult> { let package_root = manifest_file.parent().unwrap(); @@ -574,7 +578,11 @@ fn normalize_package_toml<'a>( .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) .transpose()? .map(manifest::InheritableField::Value); - let build = targets::normalize_build(original_package.build.as_ref(), package_root); + let build = if is_embedded { + Some(StringOrBool::Bool(false)) + } else { + targets::normalize_build(original_package.build.as_ref(), package_root) + }; let metabuild = original_package.metabuild.clone(); let default_target = original_package.default_target.clone(); let forced_target = original_package.forced_target.clone(); From f6b9c8f87483a7234a5266d0e952bea84fe46292 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 12:10:08 -0600 Subject: [PATCH 12/20] refactor(embedded): Move package.auto* to normalization --- src/cargo/util/toml/embedded.rs | 15 --------------- src/cargo/util/toml/mod.rs | 11 ++++++----- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 7da61c5e869..2090791cd4c 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -124,11 +124,6 @@ fn expand_manifest_( )); toml::Value::String(DEFAULT_EDITION.to_string()) }); - for field in AUTO_FIELDS { - package - .entry(field.to_owned()) - .or_insert_with(|| toml::Value::Boolean(false)); - } let mut bin = toml::Table::new(); bin.insert("name".to_owned(), toml::Value::String(bin_name)); @@ -563,11 +558,6 @@ name = "test-" path = "/home/me/test.rs" [package] -autobenches = false -autobins = false -autoexamples = false -autolib = false -autotests = false edition = "2024" name = "test-" @@ -597,11 +587,6 @@ path = [..] time = "0.1.25" [package] -autobenches = false -autobins = false -autoexamples = false -autolib = false -autotests = false edition = "2024" name = "test-" diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 06b0b556137..688d8c184d8 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -333,12 +333,13 @@ fn normalize_toml( normalized_toml.features = normalize_features(original_toml.features.as_ref())?; + let auto_embedded = is_embedded.then_some(false); normalized_toml.lib = targets::normalize_lib( original_toml.lib.as_ref(), package_root, package_name, edition, - original_package.autolib, + original_package.autolib.or(auto_embedded), warnings, )?; normalized_toml.bin = Some(targets::normalize_bins( @@ -346,7 +347,7 @@ fn normalize_toml( package_root, package_name, edition, - original_package.autobins, + original_package.autobins.or(auto_embedded), warnings, errors, normalized_toml.lib.is_some(), @@ -355,7 +356,7 @@ fn normalize_toml( original_toml.example.as_ref(), package_root, edition, - original_package.autoexamples, + original_package.autoexamples.or(auto_embedded), warnings, errors, )?); @@ -363,7 +364,7 @@ fn normalize_toml( original_toml.test.as_ref(), package_root, edition, - original_package.autotests, + original_package.autotests.or(auto_embedded), warnings, errors, )?); @@ -371,7 +372,7 @@ fn normalize_toml( original_toml.bench.as_ref(), package_root, edition, - original_package.autobenches, + original_package.autobenches.or(auto_embedded), warnings, errors, )?); From 911f1748329168ae4a350c7631f56f94e90b6056 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 12:27:03 -0600 Subject: [PATCH 13/20] refactor(embedded): Move package.edition to normalization --- src/cargo/util/toml/embedded.rs | 21 +++------------------ src/cargo/util/toml/mod.rs | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 2090791cd4c..d36480ea0ec 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -6,8 +6,6 @@ use crate::util::restricted_names; use crate::CargoResult; use crate::GlobalContext; -const DEFAULT_EDITION: crate::core::features::Edition = - crate::core::features::Edition::LATEST_STABLE; const AUTO_FIELDS: &[&str] = &[ "autolib", "autobins", @@ -64,24 +62,20 @@ pub(super) fn expand_manifest( } cargo_util::paths::write_if_changed(&hacked_path, hacked_source)?; - let manifest = expand_manifest_(&frontmatter, &hacked_path, gctx) + let manifest = expand_manifest_(&frontmatter, &hacked_path) .with_context(|| format!("failed to parse manifest at `{}`", path.display()))?; let manifest = toml::to_string_pretty(&manifest)?; Ok(manifest) } else { let frontmatter = ""; - let manifest = expand_manifest_(frontmatter, path, gctx) + let manifest = expand_manifest_(frontmatter, path) .with_context(|| format!("failed to parse manifest at `{}`", path.display()))?; let manifest = toml::to_string_pretty(&manifest)?; Ok(manifest) } } -fn expand_manifest_( - manifest: &str, - path: &std::path::Path, - gctx: &GlobalContext, -) -> CargoResult { +fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult { let mut manifest: toml::Table = toml::from_str(&manifest)?; for key in ["workspace", "lib", "bin", "example", "test", "bench"] { @@ -117,13 +111,6 @@ fn expand_manifest_( package .entry("name".to_owned()) .or_insert(toml::Value::String(name)); - package.entry("edition".to_owned()).or_insert_with(|| { - let _ = gctx.shell().warn(format_args!( - "`package.edition` is unspecified, defaulting to `{}`", - DEFAULT_EDITION - )); - toml::Value::String(DEFAULT_EDITION.to_string()) - }); let mut bin = toml::Table::new(); bin.insert("name".to_owned(), toml::Value::String(bin_name)); @@ -558,7 +545,6 @@ name = "test-" path = "/home/me/test.rs" [package] -edition = "2024" name = "test-" [workspace] @@ -587,7 +573,6 @@ path = [..] time = "0.1.25" [package] -edition = "2024" name = "test-" [workspace] diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 688d8c184d8..8491e2ae70d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -321,7 +321,7 @@ fn normalize_toml( if let Some(original_package) = original_toml.package() { let normalized_package = - normalize_package_toml(original_package, manifest_file, is_embedded, &inherit)?; + normalize_package_toml(original_package, manifest_file, is_embedded, gctx, &inherit)?; let package_name = &normalized_package.name.clone(); let edition = normalized_package .normalized_edition() @@ -550,6 +550,7 @@ fn normalize_package_toml<'a>( original_package: &manifest::TomlPackage, manifest_file: &Path, is_embedded: bool, + gctx: &GlobalContext, inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, ) -> CargoResult> { let package_root = manifest_file.parent().unwrap(); @@ -559,7 +560,22 @@ fn normalize_package_toml<'a>( .clone() .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) .transpose()? - .map(manifest::InheritableField::Value); + .map(manifest::InheritableField::Value) + .or_else(|| { + if is_embedded { + const DEFAULT_EDITION: crate::core::features::Edition = + crate::core::features::Edition::LATEST_STABLE; + let _ = gctx.shell().warn(format_args!( + "`package.edition` is unspecified, defaulting to `{}`", + DEFAULT_EDITION + )); + Some(manifest::InheritableField::Value( + DEFAULT_EDITION.to_string(), + )) + } else { + None + } + }); let rust_version = original_package .rust_version .clone() From 6ec8da96f42a3e6ea2dde43034a783d8fa23ebe6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 14:22:47 -0600 Subject: [PATCH 14/20] fix(schema): Mark package.name as optional --- crates/cargo-util-schemas/manifest.schema.json | 10 +++++----- crates/cargo-util-schemas/src/manifest/mod.rs | 10 +++++++--- src/cargo/util/toml/mod.rs | 16 +++++++++++++--- src/cargo/util/toml/targets.rs | 2 +- tests/testsuite/bad_config.rs | 10 ++++------ 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/crates/cargo-util-schemas/manifest.schema.json b/crates/cargo-util-schemas/manifest.schema.json index 5bc43e49d37..bbf85455913 100644 --- a/crates/cargo-util-schemas/manifest.schema.json +++ b/crates/cargo-util-schemas/manifest.schema.json @@ -241,7 +241,10 @@ ] }, "name": { - "type": "string" + "type": [ + "string", + "null" + ] }, "version": { "anyOf": [ @@ -475,10 +478,7 @@ } ] } - }, - "required": [ - "name" - ] + } }, "InheritableField_for_string": { "description": "An enum that allows for inheriting keys from a workspace in a Cargo.toml.", diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 906ff3d431f..d3706407a2d 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -178,8 +178,8 @@ pub struct TomlPackage { pub edition: Option, #[cfg_attr(feature = "unstable-schema", schemars(with = "Option"))] pub rust_version: Option, - #[cfg_attr(feature = "unstable-schema", schemars(with = "String"))] - pub name: PackageName, + #[cfg_attr(feature = "unstable-schema", schemars(with = "Option"))] + pub name: Option, pub version: Option, pub authors: Option, pub build: Option, @@ -226,7 +226,7 @@ pub struct TomlPackage { impl TomlPackage { pub fn new(name: PackageName) -> Self { Self { - name, + name: Some(name), edition: None, rust_version: None, @@ -263,6 +263,10 @@ impl TomlPackage { } } + pub fn normalized_name(&self) -> Result<&PackageName, UnresolvedError> { + self.name.as_ref().ok_or(UnresolvedError) + } + pub fn normalized_edition(&self) -> Result, UnresolvedError> { self.edition.as_ref().map(|v| v.normalized()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 8491e2ae70d..d341874fd34 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -322,7 +322,10 @@ fn normalize_toml( if let Some(original_package) = original_toml.package() { let normalized_package = normalize_package_toml(original_package, manifest_file, is_embedded, gctx, &inherit)?; - let package_name = &normalized_package.name.clone(); + let package_name = &normalized_package + .normalized_name() + .expect("previously normalized") + .clone(); let edition = normalized_package .normalized_edition() .expect("previously normalized") @@ -582,7 +585,12 @@ fn normalize_package_toml<'a>( .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) .transpose()? .map(manifest::InheritableField::Value); - let name = original_package.name.clone(); + let name = Some( + original_package + .name + .clone() + .ok_or_else(|| anyhow::format_err!("missing field `package.name`"))?, + ); let version = original_package .version .clone() @@ -1198,7 +1206,9 @@ pub fn to_real_manifest( let normalized_package = normalized_toml .package() .expect("previously verified to have a `[package]`"); - let package_name = &normalized_package.name; + let package_name = normalized_package + .normalized_name() + .expect("previously normalized"); if package_name.contains(':') { features.require(Feature::open_namespaces())?; } diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 3fe018433b6..9c71b4d766d 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -117,7 +117,7 @@ pub(super) fn to_targets( targets.push(Target::metabuild_target(&format!( "metabuild-{}", - package.name + package.normalized_name().expect("previously normalized") ))); } diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 5ac8c1216ad..8e7316dd246 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -484,12 +484,10 @@ fn cargo_toml_missing_package_name() { p.cargo("check") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] missing field `name` - --> Cargo.toml:2:16 - | -2 | [package] - | ^^^^^^^^^ - | +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + missing field `package.name` "#]]) .run(); From d5b5074024ecf7901759af153789ccafae77d484 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 20:51:41 -0600 Subject: [PATCH 15/20] refactor(schema): Make TomlPackage defaultable --- crates/cargo-util-schemas/src/manifest/mod.rs | 36 ++----------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index d3706407a2d..563b7d329c7 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -171,7 +171,7 @@ pub struct InheritablePackage { /// are serialized to a TOML file. For example, you cannot have values after /// the field `metadata`, since it is a table and values cannot appear after /// tables. -#[derive(Deserialize, Serialize, Clone, Debug)] +#[derive(Deserialize, Serialize, Clone, Debug, Default)] #[serde(rename_all = "kebab-case")] #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] pub struct TomlPackage { @@ -227,39 +227,7 @@ impl TomlPackage { pub fn new(name: PackageName) -> Self { Self { name: Some(name), - - edition: None, - rust_version: None, - version: None, - authors: None, - build: None, - metabuild: None, - default_target: None, - forced_target: None, - links: None, - exclude: None, - include: None, - publish: None, - workspace: None, - im_a_teapot: None, - autolib: None, - autobins: None, - autoexamples: None, - autotests: None, - autobenches: None, - default_run: None, - description: None, - homepage: None, - documentation: None, - readme: None, - keywords: None, - categories: None, - license: None, - license_file: None, - repository: None, - resolver: None, - metadata: None, - _invalid_cargo_features: None, + ..Default::default() } } From 8a40b36fdc4ef2c78154927d3f8218c6ea9f2519 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 20:46:15 -0600 Subject: [PATCH 16/20] refactor(embedded): Move package.name to normalization --- src/cargo/util/toml/embedded.rs | 7 +------ src/cargo/util/toml/mod.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index d36480ea0ec..14921e8a538 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -108,9 +108,6 @@ fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult CargoResult String { +pub fn sanitize_name(name: &str) -> String { let placeholder = if name.contains('_') { '_' } else { @@ -545,7 +542,6 @@ name = "test-" path = "/home/me/test.rs" [package] -name = "test-" [workspace] @@ -573,7 +569,6 @@ path = [..] time = "0.1.25" [package] -name = "test-" [workspace] diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index d341874fd34..caeb4955f6a 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -589,6 +589,20 @@ fn normalize_package_toml<'a>( original_package .name .clone() + .or_else(|| { + if is_embedded { + let file_stem = manifest_file + .file_stem() + .expect("file name enforced previously") + .to_string_lossy(); + let name = embedded::sanitize_name(file_stem.as_ref()); + let name = + manifest::PackageName::new(name).expect("sanitize made the name valid"); + Some(name) + } else { + None + } + }) .ok_or_else(|| anyhow::format_err!("missing field `package.name`"))?, ); let version = original_package From 3582b788cd2771a9dae5f6390f6865a14e1fbb64 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 6 Feb 2025 20:56:13 -0600 Subject: [PATCH 17/20] refactor(embedded): Move package to normalization --- src/cargo/util/toml/embedded.rs | 24 +++++++++--------------- src/cargo/util/toml/mod.rs | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 14921e8a538..be13898c9a9 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -87,19 +87,17 @@ fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult Date: Fri, 7 Feb 2025 11:05:30 -0600 Subject: [PATCH 18/20] refactor(embedded): Move workspace to normalization --- src/cargo/util/toml/embedded.rs | 7 ------- src/cargo/util/toml/mod.rs | 5 ++++- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index be13898c9a9..4c26c73dbc0 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -84,9 +84,6 @@ fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult Date: Fri, 7 Feb 2025 11:46:20 -0600 Subject: [PATCH 19/20] refactor(embedded): Move embedded validation to to_real_manifest This is where a lot of the other logic like it is --- src/cargo/util/toml/embedded.rs | 35 +++----------- src/cargo/util/toml/mod.rs | 83 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 4c26c73dbc0..ec2d102c194 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -6,14 +6,6 @@ use crate::util::restricted_names; use crate::CargoResult; use crate::GlobalContext; -const AUTO_FIELDS: &[&str] = &[ - "autolib", - "autobins", - "autoexamples", - "autotests", - "autobenches", -]; - pub(super) fn expand_manifest( content: &str, path: &std::path::Path, @@ -78,23 +70,6 @@ pub(super) fn expand_manifest( fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult { let mut manifest: toml::Table = toml::from_str(&manifest)?; - for key in ["workspace", "lib", "bin", "example", "test", "bench"] { - if manifest.contains_key(key) { - anyhow::bail!("`{key}` is not allowed in embedded manifests") - } - } - - if let Some(package) = manifest.get("package").and_then(|v| v.as_table()) { - for key in ["workspace", "build", "links"] - .iter() - .chain(AUTO_FIELDS.iter()) - { - if package.contains_key(*key) { - anyhow::bail!("`package.{key}` is not allowed in embedded manifests") - } - } - } - // HACK: Using an absolute path while `hacked_path` is in use let bin_path = path.to_string_lossy().into_owned(); let file_stem = path @@ -107,10 +82,12 @@ fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult::new().into()) + .as_array_mut() + .ok_or_else(|| anyhow::format_err!("`bin` must be an array"))? + .push(toml::Value::Table(bin)); Ok(manifest) } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index f1f03966ef7..e66c5a00aa4 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1333,6 +1333,89 @@ pub fn to_real_manifest( features.require(Feature::metabuild())?; } + if is_embedded { + let invalid_fields = [ + ("`workspace`", original_toml.workspace.is_some()), + ("`lib`", original_toml.lib.is_some()), + ( + "`bin`", + original_toml.bin.as_ref().map(|b| b.len()).unwrap_or(0) != 1, + ), + ("`example`", original_toml.example.is_some()), + ("`test`", original_toml.test.is_some()), + ("`bench`", original_toml.bench.is_some()), + ( + "`package.workspace`", + original_toml + .package() + .map(|p| p.workspace.is_some()) + .unwrap_or(false), + ), + ( + "`package.build`", + original_toml + .package() + .map(|p| p.build.is_some()) + .unwrap_or(false), + ), + ( + "`package.links`", + original_toml + .package() + .map(|p| p.links.is_some()) + .unwrap_or(false), + ), + ( + "`package.autolib`", + original_toml + .package() + .map(|p| p.autolib.is_some()) + .unwrap_or(false), + ), + ( + "`package.autobins`", + original_toml + .package() + .map(|p| p.autobins.is_some()) + .unwrap_or(false), + ), + ( + "`package.autoexamples`", + original_toml + .package() + .map(|p| p.autoexamples.is_some()) + .unwrap_or(false), + ), + ( + "`package.autotests`", + original_toml + .package() + .map(|p| p.autotests.is_some()) + .unwrap_or(false), + ), + ( + "`package.autobenches`", + original_toml + .package() + .map(|p| p.autobenches.is_some()) + .unwrap_or(false), + ), + ]; + let invalid_fields = invalid_fields + .into_iter() + .filter_map(|(name, invalid)| invalid.then_some(name)) + .collect::>(); + if !invalid_fields.is_empty() { + let fields = invalid_fields.join(", "); + let are = if invalid_fields.len() == 1 { + "is" + } else { + "are" + }; + anyhow::bail!("{fields} {are} not allowed in embedded manifests") + } + } + let resolve_behavior = match ( normalized_package.resolver.as_ref(), normalized_toml From 0ee181e98669c7451ecdde7f15d993461e1193b7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 10 Feb 2025 14:32:49 -0600 Subject: [PATCH 20/20] refactor(embedded): Clarify functions limited role --- src/cargo/util/toml/embedded.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index ec2d102c194..e151c4adde2 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -54,23 +54,23 @@ pub(super) fn expand_manifest( } cargo_util::paths::write_if_changed(&hacked_path, hacked_source)?; - let manifest = expand_manifest_(&frontmatter, &hacked_path) + let manifest = inject_bin_path(&frontmatter, &hacked_path) .with_context(|| format!("failed to parse manifest at `{}`", path.display()))?; let manifest = toml::to_string_pretty(&manifest)?; Ok(manifest) } else { let frontmatter = ""; - let manifest = expand_manifest_(frontmatter, path) + let manifest = inject_bin_path(frontmatter, path) .with_context(|| format!("failed to parse manifest at `{}`", path.display()))?; let manifest = toml::to_string_pretty(&manifest)?; Ok(manifest) } } -fn expand_manifest_(manifest: &str, path: &std::path::Path) -> CargoResult { +/// HACK: Add a `[[bin]]` table to the `original_toml` +fn inject_bin_path(manifest: &str, path: &std::path::Path) -> CargoResult { let mut manifest: toml::Table = toml::from_str(&manifest)?; - // HACK: Using an absolute path while `hacked_path` is in use let bin_path = path.to_string_lossy().into_owned(); let file_stem = path .file_stem()