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

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

@@ -35,11 +35,13 @@ use crate::schema::TomlValueWrapper;
 #[serde(rename_all = "kebab-case")]
 #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))]
 pub struct TomlManifest {
-    // when adding new fields, be sure to check whether `requires_package` should disallow them
     pub cargo_features: Option<Vec<String>>,
+
+    // Update `requires_package` when adding new package-specific fields
     pub package: Option<Box<TomlPackage>>,
     pub project: Option<Box<TomlPackage>>,
-    pub profile: Option<TomlProfiles>,
+    pub badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
+    pub features: Option<BTreeMap<FeatureName, Vec<String>>>,
     pub lib: Option<TomlLibTarget>,
     pub bin: Option<Vec<TomlBinTarget>>,
     pub example: Option<Vec<TomlExampleTarget>>,
@@ -52,14 +54,14 @@ pub struct TomlManifest {
     pub build_dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
     #[serde(rename = "build_dependencies")]
     pub build_dependencies2: Option<BTreeMap<PackageName, InheritableDependency>>,
-    pub features: Option<BTreeMap<FeatureName, Vec<String>>>,
     pub target: Option<BTreeMap<String, TomlPlatform>>,
-    pub replace: Option<BTreeMap<String, TomlDependency>>,
-    pub patch: Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
-    pub workspace: Option<TomlWorkspace>,
-    pub badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
     pub lints: Option<InheritableLints>,
 
+    pub workspace: Option<TomlWorkspace>,
+    pub profile: Option<TomlProfiles>,
+    pub patch: Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
+    pub replace: Option<BTreeMap<String, TomlDependency>>,
+
     /// Report unused keys (see also nested `_unused_keys`)
     /// Note: this is populated by the caller, rather than automatically
     #[serde(skip)]
@@ -69,6 +71,8 @@ pub struct TomlManifest {
 impl TomlManifest {
     pub fn requires_package(&self) -> impl Iterator<Item = &'static str> {
         [
+            self.badges.as_ref().map(|_| "badges"),
+            self.features.as_ref().map(|_| "features"),
             self.lib.as_ref().map(|_| "lib"),
             self.bin.as_ref().map(|_| "bin"),
             self.example.as_ref().map(|_| "example"),
@@ -79,9 +83,7 @@ impl TomlManifest {
             self.build_dependencies()
                 .as_ref()
                 .map(|_| "build-dependencies"),
-            self.features.as_ref().map(|_| "features"),
             self.target.as_ref().map(|_| "target"),
-            self.badges.as_ref().map(|_| "badges"),
             self.lints.as_ref().map(|_| "lints"),
         ]
         .into_iter()

src/cargo/util/toml/mod.rs

@@ -275,11 +275,21 @@ fn normalize_toml(
     warnings: &mut Vec<String>,
     errors: &mut Vec<String>,
 ) -> CargoResult<manifest::TomlManifest> {
+    let package_root = manifest_file.parent().unwrap();
+
+    let inherit_cell: LazyCell<InheritableFields> = LazyCell::new();
+    let inherit = || {
+        inherit_cell
+            .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))
+    };
+    let workspace_root = || inherit().map(|fields| fields.ws_root().as_path());
+
     let mut normalized_toml = manifest::TomlManifest {
         cargo_features: original_toml.cargo_features.clone(),
         package: None,
         project: None,
-        profile: original_toml.profile.clone(),
+        badges: None,
+        features: None,
         lib: None,
         bin: None,
         example: None,
@@ -290,25 +300,20 @@ fn normalize_toml(
         dev_dependencies2: None,
         build_dependencies: None,
         build_dependencies2: None,
-        features: None,
         target: None,
-        replace: original_toml.replace.clone(),
-        patch: None,
-        workspace: original_toml.workspace.clone(),
-        badges: None,
         lints: None,
+        workspace: original_toml.workspace.clone(),
+        profile: original_toml.profile.clone(),
+        patch: normalize_patch(
+            gctx,
+            original_toml.patch.as_ref(),
+            &workspace_root,
+            features,
+        )?,
+        replace: original_toml.replace.clone(),
         _unused_keys: Default::default(),
     };
 
-    let package_root = manifest_file.parent().unwrap();
-
-    let inherit_cell: LazyCell<InheritableFields> = LazyCell::new();
-    let inherit = || {
-        inherit_cell
-            .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))
-    };
-    let workspace_root = || inherit().map(|fields| fields.ws_root().as_path());
-
     if let Some(original_package) = original_toml.package() {
         let package_name = &original_package.name;
 
@@ -483,13 +488,6 @@ fn normalize_toml(
         }
         normalized_toml.target = (!normalized_target.is_empty()).then_some(normalized_target);
 
-        normalized_toml.patch = normalize_patch(
-            gctx,
-            original_toml.patch.as_ref(),
-            &workspace_root,
-            features,
-        )?;
-
         let normalized_lints = original_toml
             .lints
             .clone()
@@ -1733,14 +1731,14 @@ fn to_virtual_manifest(
             root,
         };
         (
-            replace(&original_toml, &mut manifest_ctx)?,
-            patch(&original_toml, &mut manifest_ctx)?,
+            replace(&normalized_toml, &mut manifest_ctx)?,
+            patch(&normalized_toml, &mut manifest_ctx)?,
         )
     };
-    if let Some(profiles) = &original_toml.profile {
+    if let Some(profiles) = &normalized_toml.profile {
         validate_profiles(profiles, gctx.cli_unstable(), &features, warnings)?;
     }
-    let resolve_behavior = original_toml
+    let resolve_behavior = normalized_toml
         .workspace
         .as_ref()
         .and_then(|ws| ws.resolver.as_deref())
@@ -2820,9 +2818,11 @@ fn prepare_toml_for_publish(
 
     let all = |_d: &manifest::TomlDependency| true;
     let mut manifest = manifest::TomlManifest {
+        cargo_features: me.cargo_features.clone(),
         package: Some(package),
         project: None,
-        profile: me.profile.clone(),
+        badges: me.badges.clone(),
+        features: me.features.clone(),
         lib,
         bin,
         example,
@@ -2837,7 +2837,6 @@ fn prepare_toml_for_publish(
         dev_dependencies2: None,
         build_dependencies: map_deps(gctx, me.build_dependencies(), all)?,
         build_dependencies2: None,
-        features: me.features.clone(),
         target: match me.target.as_ref().map(|target_map| {
             target_map
                 .iter()
@@ -2863,12 +2862,11 @@ fn prepare_toml_for_publish(
             Some(Err(e)) => return Err(e),
             None => None,
         },
-        replace: None,
-        patch: None,
-        workspace: None,
-        badges: me.badges.clone(),
-        cargo_features: me.cargo_features.clone(),
         lints: me.lints.clone(),
+        workspace: None,
+        profile: me.profile.clone(),
+        patch: None,
+        replace: None,
         _unused_keys: Default::default(),
     };
     strip_features(&mut manifest);

tests/testsuite/features_namespaced.rs

@@ -1006,6 +1006,9 @@ homepage = "https://example.com/"
 readme = false
 license = "MIT"
 
+[features]
+feat = ["opt-dep1"]
+
 [lib]
 name = "foo"
 path = "src/lib.rs"
@@ -1018,9 +1021,6 @@ optional = true
 version = "1.0"
 optional = true
 
-[features]
-feat = ["opt-dep1"]
-
 "##]],
         )],
     );
@@ -1143,6 +1143,11 @@ homepage = "https://example.com/"
 readme = false
 license = "MIT"
 
+[features]
+feat1 = []
+feat2 = ["dep:bar"]
+feat3 = ["feat2"]
+
 [lib]
 name = "foo"
 path = "src/lib.rs"
@@ -1151,11 +1156,6 @@ path = "src/lib.rs"
 version = "1.0"
 optional = true
 
-[features]
-feat1 = []
-feat2 = ["dep:bar"]
-feat3 = ["feat2"]
-
 "##]],
         )],
     );

tests/testsuite/patch.rs

@@ -1479,8 +1479,17 @@ fn patch_in_virtual() {
         .file("foo/src/lib.rs", r#""#)
         .build();
 
-    p.cargo("check").run();
-    p.cargo("check")
+    p.cargo("check -p foo")
+        .with_stderr_data(str![[r#"
+[UPDATING] `dummy-registry` index
+[LOCKING] 1 package to latest compatible version
+[CHECKING] bar v0.1.0 ([ROOT]/foo/bar)
+[CHECKING] foo v0.1.0 ([ROOT]/foo/foo)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]])
+        .run();
+    p.cargo("check -p foo")
         .with_stderr_data(str![[r#"
 [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 
@@ -3029,7 +3038,7 @@ foo v0.0.0 ([ROOT]/foo)
 }
 
 #[cargo_test]
-fn patch_with_base() {
+fn patch_in_real_with_base() {
     let bar = project()
         .at("bar")
         .file("Cargo.toml", &basic_manifest("bar", "0.5.0"))
@@ -3069,7 +3078,70 @@ fn patch_with_base() {
         .file("src/lib.rs", "use bar::hello as _;")
         .build();
 
-    p.cargo("build -v")
+    p.cargo("tree")
+        .masquerade_as_nightly_cargo(&["path-bases"])
+        .with_stdout_data(str![[r#"
+foo v0.5.0 ([ROOT]/foo)
+└── bar v0.5.0 ([ROOT]/bar)
+
+"#]])
+        .run();
+}
+
+#[cargo_test]
+fn patch_in_virtual_with_base() {
+    let bar = project()
+        .at("bar")
+        .file("Cargo.toml", &basic_manifest("bar", "0.5.0"))
+        .file("src/lib.rs", "pub fn hello() {}")
+        .build();
+    Package::new("bar", "0.5.0").publish();
+
+    let p = project()
+        .file(
+            ".cargo/config.toml",
+            &format!(
+                r#"
+                    [path-bases]
+                    test = '{}'
+                "#,
+                bar.root().parent().unwrap().display()
+            ),
+        )
+        .file(
+            "Cargo.toml",
+            r#"
+                cargo-features = ["path-bases"]
+
+                [workspace]
+                members = ["foo"]
+
+                [patch.crates-io]
+                bar = { base = 'test', path = 'bar' }
+            "#,
+        )
+        .file(
+            "foo/Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.5.0"
+                authors = ["[email protected]"]
+                edition = "2018"
+
+                [dependencies]
+                bar = "0.5.0"
+            "#,
+        )
+        .file("foo/src/lib.rs", "use bar::hello as _;")
+        .build();
+
+    p.cargo("tree")
         .masquerade_as_nightly_cargo(&["path-bases"])
+        .with_stdout_data(str![[r#"
+foo v0.5.0 ([ROOT]/foo/foo)
+└── bar v0.5.0 ([ROOT]/bar)
+
+"#]])
         .run();
 }

tests/testsuite/publish.rs

@@ -2015,6 +2015,20 @@ readme = false
 license = "MIT"
 repository = "foo"
 
+[features]
+foo_feature = [
+    "normal-only/cat",
+    "build-only/cat",
+    "normal-and-dev/cat",
+    "target-normal-only/cat",
+    "target-build-only/cat",
+    "target-normal-and-dev/cat",
+    "optional-dep-feature/cat",
+    "dep:optional-namespaced",
+    "optional-renamed-dep-feature10/cat",
+    "dep:optional-renamed-namespaced10",
+]
+
 [[bin]]
 name = "foo"
 path = "src/main.rs"
@@ -2057,20 +2071,6 @@ features = ["cat"]
 version = "1.0"
 features = ["cat"]
 
-[features]
-foo_feature = [
-    "normal-only/cat",
-    "build-only/cat",
-    "normal-and-dev/cat",
-    "target-normal-only/cat",
-    "target-build-only/cat",
-    "target-normal-and-dev/cat",
-    "optional-dep-feature/cat",
-    "dep:optional-namespaced",
-    "optional-renamed-dep-feature10/cat",
-    "dep:optional-renamed-namespaced10",
-]
-
 [target."cfg(unix)".dependencies.target-normal-and-dev]
 version = "1.0"
 features = ["cat"]

tests/testsuite/weak_dep_features.rs

@@ -650,6 +650,10 @@ homepage = "https://example.com/"
 readme = false
 license = "MIT"
 
+[features]
+feat1 = []
+feat2 = ["bar?/feat"]
+
 [lib]
 name = "foo"
 path = "src/lib.rs"
@@ -658,10 +662,6 @@ path = "src/lib.rs"
 version = "1.0"
 optional = true
 
-[features]
-feat1 = []
-feat2 = ["bar?/feat"]
-
 "##]],
         )],
     );