Skip to content

Commit 7847c03

Browse files
authored
fix(base): Support bases in patches in virtual manifests (#14931)
### What does this PR try to resolve? This bug has been there since #14360 Found this when looking to change the normalization code for cargo script. As a result, I also changed `cargo-util-schemas` in the hope that grouping the fields would make the intent clearer. Since I was already changing field order, I also re-ordered for my personal taste (user facing features first) ### How should we test and review this PR? ### Additional information
2 parents 876ea2c + 5b8b2ac commit 7847c03

File tree

6 files changed

+144
-72
lines changed

6 files changed

+144
-72
lines changed

crates/cargo-util-schemas/src/manifest/mod.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,13 @@ use crate::schema::TomlValueWrapper;
3535
#[serde(rename_all = "kebab-case")]
3636
#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))]
3737
pub struct TomlManifest {
38-
// when adding new fields, be sure to check whether `requires_package` should disallow them
3938
pub cargo_features: Option<Vec<String>>,
39+
40+
// Update `requires_package` when adding new package-specific fields
4041
pub package: Option<Box<TomlPackage>>,
4142
pub project: Option<Box<TomlPackage>>,
42-
pub profile: Option<TomlProfiles>,
43+
pub badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
44+
pub features: Option<BTreeMap<FeatureName, Vec<String>>>,
4345
pub lib: Option<TomlLibTarget>,
4446
pub bin: Option<Vec<TomlBinTarget>>,
4547
pub example: Option<Vec<TomlExampleTarget>>,
@@ -52,14 +54,14 @@ pub struct TomlManifest {
5254
pub build_dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
5355
#[serde(rename = "build_dependencies")]
5456
pub build_dependencies2: Option<BTreeMap<PackageName, InheritableDependency>>,
55-
pub features: Option<BTreeMap<FeatureName, Vec<String>>>,
5657
pub target: Option<BTreeMap<String, TomlPlatform>>,
57-
pub replace: Option<BTreeMap<String, TomlDependency>>,
58-
pub patch: Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
59-
pub workspace: Option<TomlWorkspace>,
60-
pub badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
6158
pub lints: Option<InheritableLints>,
6259

60+
pub workspace: Option<TomlWorkspace>,
61+
pub profile: Option<TomlProfiles>,
62+
pub patch: Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
63+
pub replace: Option<BTreeMap<String, TomlDependency>>,
64+
6365
/// Report unused keys (see also nested `_unused_keys`)
6466
/// Note: this is populated by the caller, rather than automatically
6567
#[serde(skip)]
@@ -69,6 +71,8 @@ pub struct TomlManifest {
6971
impl TomlManifest {
7072
pub fn requires_package(&self) -> impl Iterator<Item = &'static str> {
7173
[
74+
self.badges.as_ref().map(|_| "badges"),
75+
self.features.as_ref().map(|_| "features"),
7276
self.lib.as_ref().map(|_| "lib"),
7377
self.bin.as_ref().map(|_| "bin"),
7478
self.example.as_ref().map(|_| "example"),
@@ -79,9 +83,7 @@ impl TomlManifest {
7983
self.build_dependencies()
8084
.as_ref()
8185
.map(|_| "build-dependencies"),
82-
self.features.as_ref().map(|_| "features"),
8386
self.target.as_ref().map(|_| "target"),
84-
self.badges.as_ref().map(|_| "badges"),
8587
self.lints.as_ref().map(|_| "lints"),
8688
]
8789
.into_iter()

src/cargo/util/toml/mod.rs

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,21 @@ fn normalize_toml(
275275
warnings: &mut Vec<String>,
276276
errors: &mut Vec<String>,
277277
) -> CargoResult<manifest::TomlManifest> {
278+
let package_root = manifest_file.parent().unwrap();
279+
280+
let inherit_cell: LazyCell<InheritableFields> = LazyCell::new();
281+
let inherit = || {
282+
inherit_cell
283+
.try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))
284+
};
285+
let workspace_root = || inherit().map(|fields| fields.ws_root().as_path());
286+
278287
let mut normalized_toml = manifest::TomlManifest {
279288
cargo_features: original_toml.cargo_features.clone(),
280289
package: None,
281290
project: None,
282-
profile: original_toml.profile.clone(),
291+
badges: None,
292+
features: None,
283293
lib: None,
284294
bin: None,
285295
example: None,
@@ -290,25 +300,20 @@ fn normalize_toml(
290300
dev_dependencies2: None,
291301
build_dependencies: None,
292302
build_dependencies2: None,
293-
features: None,
294303
target: None,
295-
replace: original_toml.replace.clone(),
296-
patch: None,
297-
workspace: original_toml.workspace.clone(),
298-
badges: None,
299304
lints: None,
305+
workspace: original_toml.workspace.clone(),
306+
profile: original_toml.profile.clone(),
307+
patch: normalize_patch(
308+
gctx,
309+
original_toml.patch.as_ref(),
310+
&workspace_root,
311+
features,
312+
)?,
313+
replace: original_toml.replace.clone(),
300314
_unused_keys: Default::default(),
301315
};
302316

303-
let package_root = manifest_file.parent().unwrap();
304-
305-
let inherit_cell: LazyCell<InheritableFields> = LazyCell::new();
306-
let inherit = || {
307-
inherit_cell
308-
.try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))
309-
};
310-
let workspace_root = || inherit().map(|fields| fields.ws_root().as_path());
311-
312317
if let Some(original_package) = original_toml.package() {
313318
let package_name = &original_package.name;
314319

@@ -483,13 +488,6 @@ fn normalize_toml(
483488
}
484489
normalized_toml.target = (!normalized_target.is_empty()).then_some(normalized_target);
485490

486-
normalized_toml.patch = normalize_patch(
487-
gctx,
488-
original_toml.patch.as_ref(),
489-
&workspace_root,
490-
features,
491-
)?;
492-
493491
let normalized_lints = original_toml
494492
.lints
495493
.clone()
@@ -1733,14 +1731,14 @@ fn to_virtual_manifest(
17331731
root,
17341732
};
17351733
(
1736-
replace(&original_toml, &mut manifest_ctx)?,
1737-
patch(&original_toml, &mut manifest_ctx)?,
1734+
replace(&normalized_toml, &mut manifest_ctx)?,
1735+
patch(&normalized_toml, &mut manifest_ctx)?,
17381736
)
17391737
};
1740-
if let Some(profiles) = &original_toml.profile {
1738+
if let Some(profiles) = &normalized_toml.profile {
17411739
validate_profiles(profiles, gctx.cli_unstable(), &features, warnings)?;
17421740
}
1743-
let resolve_behavior = original_toml
1741+
let resolve_behavior = normalized_toml
17441742
.workspace
17451743
.as_ref()
17461744
.and_then(|ws| ws.resolver.as_deref())
@@ -2820,9 +2818,11 @@ fn prepare_toml_for_publish(
28202818

28212819
let all = |_d: &manifest::TomlDependency| true;
28222820
let mut manifest = manifest::TomlManifest {
2821+
cargo_features: me.cargo_features.clone(),
28232822
package: Some(package),
28242823
project: None,
2825-
profile: me.profile.clone(),
2824+
badges: me.badges.clone(),
2825+
features: me.features.clone(),
28262826
lib,
28272827
bin,
28282828
example,
@@ -2837,7 +2837,6 @@ fn prepare_toml_for_publish(
28372837
dev_dependencies2: None,
28382838
build_dependencies: map_deps(gctx, me.build_dependencies(), all)?,
28392839
build_dependencies2: None,
2840-
features: me.features.clone(),
28412840
target: match me.target.as_ref().map(|target_map| {
28422841
target_map
28432842
.iter()
@@ -2863,12 +2862,11 @@ fn prepare_toml_for_publish(
28632862
Some(Err(e)) => return Err(e),
28642863
None => None,
28652864
},
2866-
replace: None,
2867-
patch: None,
2868-
workspace: None,
2869-
badges: me.badges.clone(),
2870-
cargo_features: me.cargo_features.clone(),
28712865
lints: me.lints.clone(),
2866+
workspace: None,
2867+
profile: me.profile.clone(),
2868+
patch: None,
2869+
replace: None,
28722870
_unused_keys: Default::default(),
28732871
};
28742872
strip_features(&mut manifest);

tests/testsuite/features_namespaced.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,9 @@ homepage = "https://example.com/"
10061006
readme = false
10071007
license = "MIT"
10081008
1009+
[features]
1010+
feat = ["opt-dep1"]
1011+
10091012
[lib]
10101013
name = "foo"
10111014
path = "src/lib.rs"
@@ -1018,9 +1021,6 @@ optional = true
10181021
version = "1.0"
10191022
optional = true
10201023
1021-
[features]
1022-
feat = ["opt-dep1"]
1023-
10241024
"##]],
10251025
)],
10261026
);
@@ -1143,6 +1143,11 @@ homepage = "https://example.com/"
11431143
readme = false
11441144
license = "MIT"
11451145
1146+
[features]
1147+
feat1 = []
1148+
feat2 = ["dep:bar"]
1149+
feat3 = ["feat2"]
1150+
11461151
[lib]
11471152
name = "foo"
11481153
path = "src/lib.rs"
@@ -1151,11 +1156,6 @@ path = "src/lib.rs"
11511156
version = "1.0"
11521157
optional = true
11531158
1154-
[features]
1155-
feat1 = []
1156-
feat2 = ["dep:bar"]
1157-
feat3 = ["feat2"]
1158-
11591159
"##]],
11601160
)],
11611161
);

tests/testsuite/patch.rs

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,8 +1479,17 @@ fn patch_in_virtual() {
14791479
.file("foo/src/lib.rs", r#""#)
14801480
.build();
14811481

1482-
p.cargo("check").run();
1483-
p.cargo("check")
1482+
p.cargo("check -p foo")
1483+
.with_stderr_data(str![[r#"
1484+
[UPDATING] `dummy-registry` index
1485+
[LOCKING] 1 package to latest compatible version
1486+
[CHECKING] bar v0.1.0 ([ROOT]/foo/bar)
1487+
[CHECKING] foo v0.1.0 ([ROOT]/foo/foo)
1488+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
1489+
1490+
"#]])
1491+
.run();
1492+
p.cargo("check -p foo")
14841493
.with_stderr_data(str![[r#"
14851494
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
14861495
@@ -3029,7 +3038,7 @@ foo v0.0.0 ([ROOT]/foo)
30293038
}
30303039

30313040
#[cargo_test]
3032-
fn patch_with_base() {
3041+
fn patch_in_real_with_base() {
30333042
let bar = project()
30343043
.at("bar")
30353044
.file("Cargo.toml", &basic_manifest("bar", "0.5.0"))
@@ -3069,7 +3078,70 @@ fn patch_with_base() {
30693078
.file("src/lib.rs", "use bar::hello as _;")
30703079
.build();
30713080

3072-
p.cargo("build -v")
3081+
p.cargo("tree")
3082+
.masquerade_as_nightly_cargo(&["path-bases"])
3083+
.with_stdout_data(str![[r#"
3084+
foo v0.5.0 ([ROOT]/foo)
3085+
└── bar v0.5.0 ([ROOT]/bar)
3086+
3087+
"#]])
3088+
.run();
3089+
}
3090+
3091+
#[cargo_test]
3092+
fn patch_in_virtual_with_base() {
3093+
let bar = project()
3094+
.at("bar")
3095+
.file("Cargo.toml", &basic_manifest("bar", "0.5.0"))
3096+
.file("src/lib.rs", "pub fn hello() {}")
3097+
.build();
3098+
Package::new("bar", "0.5.0").publish();
3099+
3100+
let p = project()
3101+
.file(
3102+
".cargo/config.toml",
3103+
&format!(
3104+
r#"
3105+
[path-bases]
3106+
test = '{}'
3107+
"#,
3108+
bar.root().parent().unwrap().display()
3109+
),
3110+
)
3111+
.file(
3112+
"Cargo.toml",
3113+
r#"
3114+
cargo-features = ["path-bases"]
3115+
3116+
[workspace]
3117+
members = ["foo"]
3118+
3119+
[patch.crates-io]
3120+
bar = { base = 'test', path = 'bar' }
3121+
"#,
3122+
)
3123+
.file(
3124+
"foo/Cargo.toml",
3125+
r#"
3126+
[package]
3127+
name = "foo"
3128+
version = "0.5.0"
3129+
authors = ["[email protected]"]
3130+
edition = "2018"
3131+
3132+
[dependencies]
3133+
bar = "0.5.0"
3134+
"#,
3135+
)
3136+
.file("foo/src/lib.rs", "use bar::hello as _;")
3137+
.build();
3138+
3139+
p.cargo("tree")
30733140
.masquerade_as_nightly_cargo(&["path-bases"])
3141+
.with_stdout_data(str![[r#"
3142+
foo v0.5.0 ([ROOT]/foo/foo)
3143+
└── bar v0.5.0 ([ROOT]/bar)
3144+
3145+
"#]])
30743146
.run();
30753147
}

tests/testsuite/publish.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,6 +2015,20 @@ readme = false
20152015
license = "MIT"
20162016
repository = "foo"
20172017
2018+
[features]
2019+
foo_feature = [
2020+
"normal-only/cat",
2021+
"build-only/cat",
2022+
"normal-and-dev/cat",
2023+
"target-normal-only/cat",
2024+
"target-build-only/cat",
2025+
"target-normal-and-dev/cat",
2026+
"optional-dep-feature/cat",
2027+
"dep:optional-namespaced",
2028+
"optional-renamed-dep-feature10/cat",
2029+
"dep:optional-renamed-namespaced10",
2030+
]
2031+
20182032
[[bin]]
20192033
name = "foo"
20202034
path = "src/main.rs"
@@ -2057,20 +2071,6 @@ features = ["cat"]
20572071
version = "1.0"
20582072
features = ["cat"]
20592073
2060-
[features]
2061-
foo_feature = [
2062-
"normal-only/cat",
2063-
"build-only/cat",
2064-
"normal-and-dev/cat",
2065-
"target-normal-only/cat",
2066-
"target-build-only/cat",
2067-
"target-normal-and-dev/cat",
2068-
"optional-dep-feature/cat",
2069-
"dep:optional-namespaced",
2070-
"optional-renamed-dep-feature10/cat",
2071-
"dep:optional-renamed-namespaced10",
2072-
]
2073-
20742074
[target."cfg(unix)".dependencies.target-normal-and-dev]
20752075
version = "1.0"
20762076
features = ["cat"]

tests/testsuite/weak_dep_features.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,10 @@ homepage = "https://example.com/"
650650
readme = false
651651
license = "MIT"
652652
653+
[features]
654+
feat1 = []
655+
feat2 = ["bar?/feat"]
656+
653657
[lib]
654658
name = "foo"
655659
path = "src/lib.rs"
@@ -658,10 +662,6 @@ path = "src/lib.rs"
658662
version = "1.0"
659663
optional = true
660664
661-
[features]
662-
feat1 = []
663-
feat2 = ["bar?/feat"]
664-
665665
"##]],
666666
)],
667667
);

0 commit comments

Comments
 (0)