Skip to content

Commit b64750d

Browse files
authored
refactor(embedded): Integrate cargo-script logic into main parser (#15168)
### 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
2 parents 321f14e + 0ee181e commit b64750d

File tree

8 files changed

+780
-282
lines changed

8 files changed

+780
-282
lines changed

Diff for: crates/cargo-util-schemas/manifest.schema.json

+5-5
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,10 @@
241241
]
242242
},
243243
"name": {
244-
"type": "string"
244+
"type": [
245+
"string",
246+
"null"
247+
]
245248
},
246249
"version": {
247250
"anyOf": [
@@ -475,10 +478,7 @@
475478
}
476479
]
477480
}
478-
},
479-
"required": [
480-
"name"
481-
]
481+
}
482482
},
483483
"InheritableField_for_string": {
484484
"description": "An enum that allows for inheriting keys from a workspace in a Cargo.toml.",

Diff for: crates/cargo-util-schemas/src/manifest/mod.rs

+9-37
Original file line numberDiff line numberDiff line change
@@ -171,15 +171,15 @@ pub struct InheritablePackage {
171171
/// are serialized to a TOML file. For example, you cannot have values after
172172
/// the field `metadata`, since it is a table and values cannot appear after
173173
/// tables.
174-
#[derive(Deserialize, Serialize, Clone, Debug)]
174+
#[derive(Deserialize, Serialize, Clone, Debug, Default)]
175175
#[serde(rename_all = "kebab-case")]
176176
#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))]
177177
pub struct TomlPackage {
178178
pub edition: Option<InheritableString>,
179179
#[cfg_attr(feature = "unstable-schema", schemars(with = "Option<String>"))]
180180
pub rust_version: Option<InheritableRustVersion>,
181-
#[cfg_attr(feature = "unstable-schema", schemars(with = "String"))]
182-
pub name: PackageName,
181+
#[cfg_attr(feature = "unstable-schema", schemars(with = "Option<String>"))]
182+
pub name: Option<PackageName>,
183183
pub version: Option<InheritableSemverVersion>,
184184
pub authors: Option<InheritableVecString>,
185185
pub build: Option<StringOrBool>,
@@ -226,43 +226,15 @@ pub struct TomlPackage {
226226
impl TomlPackage {
227227
pub fn new(name: PackageName) -> Self {
228228
Self {
229-
name,
230-
231-
edition: None,
232-
rust_version: None,
233-
version: None,
234-
authors: None,
235-
build: None,
236-
metabuild: None,
237-
default_target: None,
238-
forced_target: None,
239-
links: None,
240-
exclude: None,
241-
include: None,
242-
publish: None,
243-
workspace: None,
244-
im_a_teapot: None,
245-
autolib: None,
246-
autobins: None,
247-
autoexamples: None,
248-
autotests: None,
249-
autobenches: None,
250-
default_run: None,
251-
description: None,
252-
homepage: None,
253-
documentation: None,
254-
readme: None,
255-
keywords: None,
256-
categories: None,
257-
license: None,
258-
license_file: None,
259-
repository: None,
260-
resolver: None,
261-
metadata: None,
262-
_invalid_cargo_features: None,
229+
name: Some(name),
230+
..Default::default()
263231
}
264232
}
265233

234+
pub fn normalized_name(&self) -> Result<&PackageName, UnresolvedError> {
235+
self.name.as_ref().ok_or(UnresolvedError)
236+
}
237+
266238
pub fn normalized_edition(&self) -> Result<Option<&String>, UnresolvedError> {
267239
self.edition.as_ref().map(|v| v.normalized()).transpose()
268240
}

Diff for: src/cargo/ops/vendor.rs

+1
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ fn prepare_for_vendor(
434434
workspace_config,
435435
source_id,
436436
me.manifest_path(),
437+
me.manifest().is_embedded(),
437438
gctx,
438439
&mut warnings,
439440
&mut errors,

Diff for: src/cargo/util/toml/embedded.rs

+13-89
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,6 @@ use crate::util::restricted_names;
66
use crate::CargoResult;
77
use crate::GlobalContext;
88

9-
const DEFAULT_EDITION: crate::core::features::Edition =
10-
crate::core::features::Edition::LATEST_STABLE;
11-
const AUTO_FIELDS: &[&str] = &[
12-
"autolib",
13-
"autobins",
14-
"autoexamples",
15-
"autotests",
16-
"autobenches",
17-
];
18-
199
pub(super) fn expand_manifest(
2010
content: &str,
2111
path: &std::path::Path,
@@ -64,88 +54,46 @@ pub(super) fn expand_manifest(
6454
}
6555
cargo_util::paths::write_if_changed(&hacked_path, hacked_source)?;
6656

67-
let manifest = expand_manifest_(&frontmatter, &hacked_path, gctx)
68-
.with_context(|| format!("failed to parse manifest at {}", path.display()))?;
57+
let manifest = inject_bin_path(&frontmatter, &hacked_path)
58+
.with_context(|| format!("failed to parse manifest at `{}`", path.display()))?;
6959
let manifest = toml::to_string_pretty(&manifest)?;
7060
Ok(manifest)
7161
} else {
7262
let frontmatter = "";
73-
let manifest = expand_manifest_(frontmatter, path, gctx)
74-
.with_context(|| format!("failed to parse manifest at {}", path.display()))?;
63+
let manifest = inject_bin_path(frontmatter, path)
64+
.with_context(|| format!("failed to parse manifest at `{}`", path.display()))?;
7565
let manifest = toml::to_string_pretty(&manifest)?;
7666
Ok(manifest)
7767
}
7868
}
7969

80-
fn expand_manifest_(
81-
manifest: &str,
82-
path: &std::path::Path,
83-
gctx: &GlobalContext,
84-
) -> CargoResult<toml::Table> {
70+
/// HACK: Add a `[[bin]]` table to the `original_toml`
71+
fn inject_bin_path(manifest: &str, path: &std::path::Path) -> CargoResult<toml::Table> {
8572
let mut manifest: toml::Table = toml::from_str(&manifest)?;
8673

87-
for key in ["workspace", "lib", "bin", "example", "test", "bench"] {
88-
if manifest.contains_key(key) {
89-
anyhow::bail!("`{key}` is not allowed in embedded manifests")
90-
}
91-
}
92-
93-
// Prevent looking for a workspace by `read_manifest_from_str`
94-
manifest.insert("workspace".to_owned(), toml::Table::new().into());
95-
96-
let package = manifest
97-
.entry("package".to_owned())
98-
.or_insert_with(|| toml::Table::new().into())
99-
.as_table_mut()
100-
.ok_or_else(|| anyhow::format_err!("`package` must be a table"))?;
101-
for key in ["workspace", "build", "links"]
102-
.iter()
103-
.chain(AUTO_FIELDS.iter())
104-
{
105-
if package.contains_key(*key) {
106-
anyhow::bail!("`package.{key}` is not allowed in embedded manifests")
107-
}
108-
}
109-
// HACK: Using an absolute path while `hacked_path` is in use
11074
let bin_path = path.to_string_lossy().into_owned();
11175
let file_stem = path
11276
.file_stem()
11377
.ok_or_else(|| anyhow::format_err!("no file name"))?
11478
.to_string_lossy();
11579
let name = sanitize_name(file_stem.as_ref());
11680
let bin_name = name.clone();
117-
package
118-
.entry("name".to_owned())
119-
.or_insert(toml::Value::String(name));
120-
package.entry("edition".to_owned()).or_insert_with(|| {
121-
let _ = gctx.shell().warn(format_args!(
122-
"`package.edition` is unspecified, defaulting to `{}`",
123-
DEFAULT_EDITION
124-
));
125-
toml::Value::String(DEFAULT_EDITION.to_string())
126-
});
127-
package
128-
.entry("build".to_owned())
129-
.or_insert_with(|| toml::Value::Boolean(false));
130-
for field in AUTO_FIELDS {
131-
package
132-
.entry(field.to_owned())
133-
.or_insert_with(|| toml::Value::Boolean(false));
134-
}
13581

13682
let mut bin = toml::Table::new();
13783
bin.insert("name".to_owned(), toml::Value::String(bin_name));
13884
bin.insert("path".to_owned(), toml::Value::String(bin_path));
139-
manifest.insert(
140-
"bin".to_owned(),
141-
toml::Value::Array(vec![toml::Value::Table(bin)]),
142-
);
85+
manifest
86+
.entry("bin")
87+
.or_insert_with(|| Vec::<toml::Value>::new().into())
88+
.as_array_mut()
89+
.ok_or_else(|| anyhow::format_err!("`bin` must be an array"))?
90+
.push(toml::Value::Table(bin));
14391

14492
Ok(manifest)
14593
}
14694

14795
/// Ensure the package name matches the validation from `ops::cargo_new::check_name`
148-
fn sanitize_name(name: &str) -> String {
96+
pub fn sanitize_name(name: &str) -> String {
14997
let placeholder = if name.contains('_') {
15098
'_'
15199
} else {
@@ -565,18 +513,6 @@ fn main() {}
565513
name = "test-"
566514
path = "/home/me/test.rs"
567515
568-
[package]
569-
autobenches = false
570-
autobins = false
571-
autoexamples = false
572-
autolib = false
573-
autotests = false
574-
build = false
575-
edition = "2024"
576-
name = "test-"
577-
578-
[workspace]
579-
580516
"#]]
581517
);
582518
}
@@ -600,18 +536,6 @@ path = [..]
600536
[dependencies]
601537
time = "0.1.25"
602538
603-
[package]
604-
autobenches = false
605-
autobins = false
606-
autoexamples = false
607-
autolib = false
608-
autotests = false
609-
build = false
610-
edition = "2024"
611-
name = "test-"
612-
613-
[workspace]
614-
615539
"#]]
616540
);
617541
}

0 commit comments

Comments
 (0)