Skip to content

Commit

Permalink
refactor(embedded): Integrate cargo-script logic into main parser (#1…
Browse files Browse the repository at this point in the history
…5168)

### What does this PR try to resolve?

This is a part of #12207

Now that the experiment is over and we can be intrusive, this merges all
of the permanent cargo-script manifest logic into the regular manifest
loading code path with the goal of reducing the number of places people
need to be aware to check to know how things work so we're less likely
to overlook them (e.g. `package.name`s being optional). This should also
make error reporting better as we will eventually only use the real
manifest users specify, instead of one with a lot of edits by us.

This comes at the cost of some uglier code needed to check some of these
cases.

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

### Additional information
  • Loading branch information
ehuss authored Feb 11, 2025
2 parents 321f14e + 0ee181e commit b64750d
Show file tree
Hide file tree
Showing 8 changed files with 780 additions and 282 deletions.
10 changes: 5 additions & 5 deletions crates/cargo-util-schemas/manifest.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@
]
},
"name": {
"type": "string"
"type": [
"string",
"null"
]
},
"version": {
"anyOf": [
Expand Down Expand Up @@ -475,10 +478,7 @@
}
]
}
},
"required": [
"name"
]
}
},
"InheritableField_for_string": {
"description": "An enum that allows for inheriting keys from a workspace in a Cargo.toml.",
Expand Down
46 changes: 9 additions & 37 deletions crates/cargo-util-schemas/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,15 @@ 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 {
pub edition: Option<InheritableString>,
#[cfg_attr(feature = "unstable-schema", schemars(with = "Option<String>"))]
pub rust_version: Option<InheritableRustVersion>,
#[cfg_attr(feature = "unstable-schema", schemars(with = "String"))]
pub name: PackageName,
#[cfg_attr(feature = "unstable-schema", schemars(with = "Option<String>"))]
pub name: Option<PackageName>,
pub version: Option<InheritableSemverVersion>,
pub authors: Option<InheritableVecString>,
pub build: Option<StringOrBool>,
Expand Down Expand Up @@ -226,43 +226,15 @@ pub struct TomlPackage {
impl TomlPackage {
pub fn new(name: PackageName) -> Self {
Self {
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,
name: Some(name),
..Default::default()
}
}

pub fn normalized_name(&self) -> Result<&PackageName, UnresolvedError> {
self.name.as_ref().ok_or(UnresolvedError)
}

pub fn normalized_edition(&self) -> Result<Option<&String>, UnresolvedError> {
self.edition.as_ref().map(|v| v.normalized()).transpose()
}
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ fn prepare_for_vendor(
workspace_config,
source_id,
me.manifest_path(),
me.manifest().is_embedded(),
gctx,
&mut warnings,
&mut errors,
Expand Down
102 changes: 13 additions & 89 deletions src/cargo/util/toml/embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +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",
"autoexamples",
"autotests",
"autobenches",
];

pub(super) fn expand_manifest(
content: &str,
path: &std::path::Path,
Expand Down Expand Up @@ -64,88 +54,46 @@ 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()))?;
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, gctx)
.with_context(|| format!("failed to parse manifest at {}", path.display()))?;
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,
gctx: &GlobalContext,
) -> CargoResult<toml::Table> {
/// HACK: Add a `[[bin]]` table to the `original_toml`
fn inject_bin_path(manifest: &str, path: &std::path::Path) -> CargoResult<toml::Table> {
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")
}
}

// Prevent looking for a workspace by `read_manifest_from_str`
manifest.insert("workspace".to_owned(), toml::Table::new().into());

let package = manifest
.entry("package".to_owned())
.or_insert_with(|| toml::Table::new().into())
.as_table_mut()
.ok_or_else(|| anyhow::format_err!("`package` must be a 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
.file_stem()
.ok_or_else(|| anyhow::format_err!("no file name"))?
.to_string_lossy();
let name = sanitize_name(file_stem.as_ref());
let bin_name = name.clone();
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())
});
package
.entry("build".to_owned())
.or_insert_with(|| toml::Value::Boolean(false));
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));
bin.insert("path".to_owned(), toml::Value::String(bin_path));
manifest.insert(
"bin".to_owned(),
toml::Value::Array(vec![toml::Value::Table(bin)]),
);
manifest
.entry("bin")
.or_insert_with(|| Vec::<toml::Value>::new().into())
.as_array_mut()
.ok_or_else(|| anyhow::format_err!("`bin` must be an array"))?
.push(toml::Value::Table(bin));

Ok(manifest)
}

/// Ensure the package name matches the validation from `ops::cargo_new::check_name`
fn sanitize_name(name: &str) -> String {
pub fn sanitize_name(name: &str) -> String {
let placeholder = if name.contains('_') {
'_'
} else {
Expand Down Expand Up @@ -565,18 +513,6 @@ fn main() {}
name = "test-"
path = "/home/me/test.rs"
[package]
autobenches = false
autobins = false
autoexamples = false
autolib = false
autotests = false
build = false
edition = "2024"
name = "test-"
[workspace]
"#]]
);
}
Expand All @@ -600,18 +536,6 @@ path = [..]
[dependencies]
time = "0.1.25"
[package]
autobenches = false
autobins = false
autoexamples = false
autolib = false
autotests = false
build = false
edition = "2024"
name = "test-"
[workspace]
"#]]
);
}
Expand Down
Loading

0 comments on commit b64750d

Please sign in to comment.