Auto merge of #12914 - epage:metadata, r=weihanglo

fix(metadata): Stabilize id format as PackageIDSpec

### What does this PR try to resolve?

For tools integrating with cargo, `cargo metadata` is the primary interface.  Limitations include:
-  There isn't an unambiguous way to map a package entry from `cargo metadata`  to a parameter to pass to other `cargo` commands.  An `id` field exists but it is documented as an opaque string, useful only for comparisons with other `id`s within the document.
- There isn't an unambiguous way of taking user parameters (`--package`) and mapping them to `cargo metadata` entries.  `cargo pkgid` could help but it returns a `PackageIdSpec` which doesn't exist within the `cargo metadata` output.

This attempts to solve these problems by switching the `id` field from `PackageId` to `PackageIdSpec` which is a [publicly documented format](https://doc.rust-lang.org/cargo/reference/pkgid-spec.html), can be generated by `cargo pkgid`, and is accepted by most commands via the `--package` flag.

As the `"id"` field is documented as opaque, this technically isn't a breaking change though people could be parsing it.

For `cargo_metadata` they do [use a new type that documents it as opaque but publicly expose the inner `String`](https://docs.rs/cargo_metadata/latest/cargo_metadata/struct.PackageId.html).  The `String` wasn't publicly exposed due to a request by users but instead their `PackageId` type replaced using `String`s in the API in oli-obk/cargo_metadata#59 with no indication given as to why the `String` was still exposed.  However, you'll note that before that PR, they had `WorkspaceMember` that parsed `PackageId`.  This was introduced in oli-obk/cargo_metadata#26 without a motivation given.

**Note that `PackageIdSpec` has multiple representation that might uniquely identify a package and we can return any one of them.**

Fixes #7267

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

### Additional information

cc `@oli-obk`

Author: bors

src/cargo/core/package.rs

@@ -22,7 +22,7 @@ use crate::core::compiler::{CompileKind, RustcTargetData};
 use crate::core::dependency::DepKind;
 use crate::core::resolver::features::ForceAllTargets;
 use crate::core::resolver::{HasDevUnits, Resolve};
-use crate::core::{Dependency, Manifest, PackageId, SourceId, Target};
+use crate::core::{Dependency, Manifest, PackageId, PackageIdSpec, SourceId, Target};
 use crate::core::{Summary, Workspace};
 use crate::sources::source::{MaybePackage, SourceMap};
 use crate::util::cache_lock::{CacheLock, CacheLockMode};
@@ -82,7 +82,7 @@ impl PartialOrd for Package {
 pub struct SerializedPackage {
     name: InternedString,
     version: Version,
-    id: PackageId,
+    id: PackageIdSpec,
     license: Option<String>,
     license_file: Option<String>,
     description: Option<String>,
@@ -239,7 +239,7 @@ impl Package {
         SerializedPackage {
             name: package_id.name(),
             version: package_id.version().clone(),
-            id: package_id,
+            id: package_id.to_spec(),
             license: manmeta.license.clone(),
             license_file: manmeta.license_file.clone(),
             description: manmeta.description.clone(),

src/cargo/ops/cargo_output_metadata.rs

@@ -3,7 +3,7 @@ use crate::core::compiler::{CompileKind, RustcTargetData};
 use crate::core::dependency::DepKind;
 use crate::core::package::SerializedPackage;
 use crate::core::resolver::{features::CliFeatures, HasDevUnits, Resolve};
-use crate::core::{Package, PackageId, Workspace};
+use crate::core::{Package, PackageId, PackageIdSpec, Workspace};
 use crate::ops::{self, Packages};
 use crate::util::interning::InternedString;
 use crate::util::CargoResult;
@@ -42,8 +42,11 @@ pub fn output_metadata(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> Cargo
 
     Ok(ExportInfo {
         packages,
-        workspace_members: ws.members().map(|pkg| pkg.package_id()).collect(),
-        workspace_default_members: ws.default_members().map(|pkg| pkg.package_id()).collect(),
+        workspace_members: ws.members().map(|pkg| pkg.package_id().to_spec()).collect(),
+        workspace_default_members: ws
+            .default_members()
+            .map(|pkg| pkg.package_id().to_spec())
+            .collect(),
         resolve,
         target_directory: ws.target_dir().into_path_unlocked(),
         version: VERSION,
@@ -58,8 +61,8 @@ pub fn output_metadata(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> Cargo
 #[derive(Serialize)]
 pub struct ExportInfo {
     packages: Vec<SerializedPackage>,
-    workspace_members: Vec<PackageId>,
-    workspace_default_members: Vec<PackageId>,
+    workspace_members: Vec<PackageIdSpec>,
+    workspace_default_members: Vec<PackageIdSpec>,
     resolve: Option<MetadataResolve>,
     target_directory: PathBuf,
     version: u32,
@@ -70,13 +73,13 @@ pub struct ExportInfo {
 #[derive(Serialize)]
 struct MetadataResolve {
     nodes: Vec<MetadataResolveNode>,
-    root: Option<PackageId>,
+    root: Option<PackageIdSpec>,
 }
 
 #[derive(Serialize)]
 struct MetadataResolveNode {
-    id: PackageId,
-    dependencies: Vec<PackageId>,
+    id: PackageIdSpec,
+    dependencies: Vec<PackageIdSpec>,
     deps: Vec<Dep>,
     features: Vec<InternedString>,
 }
@@ -86,7 +89,9 @@ struct Dep {
     // TODO(bindeps): after -Zbindeps gets stabilized,
     // mark this field as deprecated in the help manual of cargo-metadata
     name: InternedString,
-    pkg: PackageId,
+    pkg: PackageIdSpec,
+    #[serde(skip)]
+    pkg_id: PackageId,
     dep_kinds: Vec<DepKindInfo>,
 }
 
@@ -179,7 +184,7 @@ fn build_resolve_graph(
 
     let mr = MetadataResolve {
         nodes: node_map.into_iter().map(|(_pkg_id, node)| node).collect(),
-        root: ws.current_opt().map(|pkg| pkg.package_id()),
+        root: ws.current_opt().map(|pkg| pkg.package_id().to_spec()),
     };
     Ok((actual_packages, mr))
 }
@@ -301,18 +306,20 @@ fn build_resolve_graph_r(
 
             dep_kinds.sort();
 
-            let pkg = normalize_id(dep_id);
+            let pkg_id = normalize_id(dep_id);
 
             let dep = match (lib_target, dep_kinds.len()) {
                 (Some(target), _) => Dep {
                     name: extern_name(target)?,
-                    pkg,
+                    pkg: pkg_id.to_spec(),
+                    pkg_id,
                     dep_kinds,
                 },
                 // No lib target exists but contains artifact deps.
                 (None, 1..) => Dep {
                     name: InternedString::new(""),
-                    pkg,
+                    pkg: pkg_id.to_spec(),
+                    pkg_id,
                     dep_kinds,
                 },
                 // No lib or artifact dep exists.
@@ -325,11 +332,10 @@ fn build_resolve_graph_r(
         dep_metadatas
     };
 
-    let dumb_deps: Vec<PackageId> = deps.iter().map(|dep| dep.pkg).collect();
-    let to_visit = dumb_deps.clone();
+    let to_visit: Vec<PackageId> = deps.iter().map(|dep| dep.pkg_id).collect();
     let node = MetadataResolveNode {
-        id: normalize_id(pkg_id),
-        dependencies: dumb_deps,
+        id: normalize_id(pkg_id).to_spec(),
+        dependencies: to_visit.iter().map(|id| id.to_spec()).collect(),
         deps,
         features,
     };

src/doc/man/cargo-metadata.md

@@ -34,8 +34,8 @@ considersed as incompatible:
 * **Adding new values for enum-like fields** — Same as adding new fields. It
   keeps metadata evolving without stagnation.
 * **Changing opaque representations** — The inner representations of some
-  fields are implementation details. For example, fields related to "Package ID"
-  or "Source ID" are treated as opaque identifiers to differentiate packages or
+  fields are implementation details. For example, fields related to
+  "Source ID" are treated as opaque identifiers to differentiate packages or
   sources. Consumers shouldn't rely on those representations unless specified.
 
 ### JSON format
@@ -53,10 +53,10 @@ The JSON output has the following format:
             "name": "my-package",
             /* The version of the package. */
             "version": "0.1.0",
-            /* The Package ID, an opaque and unique identifier for referring to the
-               package. See "Compatibility" above for the stability guarantee.
+            /* The Package ID for referring to the
+               package within the document and as the `--package` argument to many commands
             */
-            "id": "my-package 0.1.0 (path+file:///path/to/my-package)",
+            "id": "file:///path/to/my-package#0.1.0",
             /* The license value from the manifest, or null. */
             "license": "MIT/Apache-2.0",
             /* The license-file value from the manifest, or null. */
@@ -242,13 +242,13 @@ The JSON output has the following format:
        Each entry is the Package ID for the package.
     */
     "workspace_members": [
-        "my-package 0.1.0 (path+file:///path/to/my-package)",
+        "file:///path/to/my-package#0.1.0",
     ],
     /* Array of default members of the workspace.
        Each entry is the Package ID for the package.
     */
     "workspace_default_members": [
-        "my-package 0.1.0 (path+file:///path/to/my-package)",
+        "file:///path/to/my-package#0.1.0",
     ],
     // The resolved dependency graph for the entire workspace. The enabled
     // features are based on the enabled features for the "current" package.
@@ -266,10 +266,10 @@ The JSON output has the following format:
         "nodes": [
             {
                 /* The Package ID of this node. */
-                "id": "my-package 0.1.0 (path+file:///path/to/my-package)",
+                "id": "file:///path/to/my-package#0.1.0",
                 /* The dependencies of this package, an array of Package IDs. */
                 "dependencies": [
-                    "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)"
+                    "https://github.com/rust-lang/crates.io-index#bitflags@1.0.4"
                 ],
                 /* The dependencies of this package. This is an alternative to
                    "dependencies" which contains additional information. In
@@ -283,7 +283,7 @@ The JSON output has the following format:
                         */
                         "name": "bitflags",
                         /* The Package ID of the dependency. */
-                        "pkg": "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
+                        "pkg": "https://github.com/rust-lang/crates.io-index#bitflags@1.0.4"
                         /* Array of dependency kinds. Added in Cargo 1.40. */
                         "dep_kinds": [
                             {
@@ -309,7 +309,7 @@ The JSON output has the following format:
            This is null if this is a virtual workspace. Otherwise it is
            the Package ID of the root package.
         */
-        "root": "my-package 0.1.0 (path+file:///path/to/my-package)"
+        "root": "file:///path/to/my-package#0.1.0",
     },
     /* The absolute path to the build directory where Cargo places its output. */
     "target_directory": "/path/to/my-package/target",
@@ -331,6 +331,11 @@ The JSON output has the following format:
 }
 ````
 
+Notes:
+- For `"id"` field syntax, see [Package ID Specifications] in the reference.
+
+[Package ID Specifications]: ../reference/pkgid-spec.html
+
 ## OPTIONS
 
 ### Output Options

src/doc/man/generated_txt/cargo-metadata.txt

@@ -32,9 +32,9 @@ OUTPUT FORMAT
 
        o  Changing opaque representations — The inner representations of some
           fields are implementation details. For example, fields related to
-          “Package ID” or “Source ID” are treated as opaque identifiers
-          to differentiate packages or sources. Consumers shouldn’t rely on
-          those representations unless specified.
+          “Source ID” are treated as opaque identifiers to differentiate
+          packages or sources. Consumers shouldn’t rely on those
+          representations unless specified.
 
    JSON format
        The JSON output has the following format:
@@ -49,10 +49,10 @@ OUTPUT FORMAT
                        "name": "my-package",
                        /* The version of the package. */
                        "version": "0.1.0",
-                       /* The Package ID, an opaque and unique identifier for referring to the
-                          package. See "Compatibility" above for the stability guarantee.
+                       /* The Package ID for referring to the
+                          package within the document and as the `--package` argument to many commands
                        */
-                       "id": "my-package 0.1.0 (path+file:///path/to/my-package)",
+                       "id": "file:///path/to/my-package#0.1.0",
                        /* The license value from the manifest, or null. */
                        "license": "MIT/Apache-2.0",
                        /* The license-file value from the manifest, or null. */
@@ -238,13 +238,13 @@ OUTPUT FORMAT
                   Each entry is the Package ID for the package.
                */
                "workspace_members": [
-                   "my-package 0.1.0 (path+file:///path/to/my-package)",
+                   "file:///path/to/my-package#0.1.0",
                ],
                /* Array of default members of the workspace.
                   Each entry is the Package ID for the package.
                */
                "workspace_default_members": [
-                   "my-package 0.1.0 (path+file:///path/to/my-package)",
+                   "file:///path/to/my-package#0.1.0",
                ],
                // The resolved dependency graph for the entire workspace. The enabled
                // features are based on the enabled features for the "current" package.
@@ -262,10 +262,10 @@ OUTPUT FORMAT
                    "nodes": [
                        {
                            /* The Package ID of this node. */
-                           "id": "my-package 0.1.0 (path+file:///path/to/my-package)",
+                           "id": "file:///path/to/my-package#0.1.0",
                            /* The dependencies of this package, an array of Package IDs. */
                            "dependencies": [
-                               "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)"
+                               "https://github.com/rust-lang/crates.io-index#bitflags@1.0.4"
                            ],
                            /* The dependencies of this package. This is an alternative to
                               "dependencies" which contains additional information. In
@@ -279,7 +279,7 @@ OUTPUT FORMAT
                                    */
                                    "name": "bitflags",
                                    /* The Package ID of the dependency. */
-                                   "pkg": "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
+                                   "pkg": "https://github.com/rust-lang/crates.io-index#bitflags@1.0.4"
                                    /* Array of dependency kinds. Added in Cargo 1.40. */
                                    "dep_kinds": [
                                        {
@@ -305,7 +305,7 @@ OUTPUT FORMAT
                       This is null if this is a virtual workspace. Otherwise it is
                       the Package ID of the root package.
                    */
-                   "root": "my-package 0.1.0 (path+file:///path/to/my-package)"
+                   "root": "file:///path/to/my-package#0.1.0",
                },
                /* The absolute path to the build directory where Cargo places its output. */
                "target_directory": "/path/to/my-package/target",
@@ -326,6 +326,12 @@ OUTPUT FORMAT
                }
            }
 
+       Notes:
+
+       o  For "id" field syntax, see Package ID Specifications
+          <https://doc.rust-lang.org/cargo/reference/pkgid-spec.html> in the
+          reference.
+
 OPTIONS
    Output Options
        --no-deps

src/doc/src/commands/cargo-metadata.md

@@ -34,8 +34,8 @@ considersed as incompatible:
 * **Adding new values for enum-like fields** — Same as adding new fields. It
   keeps metadata evolving without stagnation.
 * **Changing opaque representations** — The inner representations of some
-  fields are implementation details. For example, fields related to "Package ID"
-  or "Source ID" are treated as opaque identifiers to differentiate packages or
+  fields are implementation details. For example, fields related to
+  "Source ID" are treated as opaque identifiers to differentiate packages or
   sources. Consumers shouldn't rely on those representations unless specified.
 
 ### JSON format
@@ -53,10 +53,10 @@ The JSON output has the following format:
             "name": "my-package",
             /* The version of the package. */
             "version": "0.1.0",
-            /* The Package ID, an opaque and unique identifier for referring to the
-               package. See "Compatibility" above for the stability guarantee.
+            /* The Package ID for referring to the
+               package within the document and as the `--package` argument to many commands
             */
-            "id": "my-package 0.1.0 (path+file:///path/to/my-package)",
+            "id": "file:///path/to/my-package#0.1.0",
             /* The license value from the manifest, or null. */
             "license": "MIT/Apache-2.0",
             /* The license-file value from the manifest, or null. */
@@ -242,13 +242,13 @@ The JSON output has the following format:
        Each entry is the Package ID for the package.
     */
     "workspace_members": [
-        "my-package 0.1.0 (path+file:///path/to/my-package)",
+        "file:///path/to/my-package#0.1.0",
     ],
     /* Array of default members of the workspace.
        Each entry is the Package ID for the package.
     */
     "workspace_default_members": [
-        "my-package 0.1.0 (path+file:///path/to/my-package)",
+        "file:///path/to/my-package#0.1.0",
     ],
     // The resolved dependency graph for the entire workspace. The enabled
     // features are based on the enabled features for the "current" package.
@@ -266,10 +266,10 @@ The JSON output has the following format:
         "nodes": [
             {
                 /* The Package ID of this node. */
-                "id": "my-package 0.1.0 (path+file:///path/to/my-package)",
+                "id": "file:///path/to/my-package#0.1.0",
                 /* The dependencies of this package, an array of Package IDs. */
                 "dependencies": [
-                    "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)"
+                    "https://github.com/rust-lang/crates.io-index#bitflags@1.0.4"
                 ],
                 /* The dependencies of this package. This is an alternative to
                    "dependencies" which contains additional information. In
@@ -283,7 +283,7 @@ The JSON output has the following format:
                         */
                         "name": "bitflags",
                         /* The Package ID of the dependency. */
-                        "pkg": "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
+                        "pkg": "https://github.com/rust-lang/crates.io-index#bitflags@1.0.4"
                         /* Array of dependency kinds. Added in Cargo 1.40. */
                         "dep_kinds": [
                             {
@@ -309,7 +309,7 @@ The JSON output has the following format:
            This is null if this is a virtual workspace. Otherwise it is
            the Package ID of the root package.
         */
-        "root": "my-package 0.1.0 (path+file:///path/to/my-package)"
+        "root": "file:///path/to/my-package#0.1.0",
     },
     /* The absolute path to the build directory where Cargo places its output. */
     "target_directory": "/path/to/my-package/target",
@@ -331,6 +331,11 @@ The JSON output has the following format:
 }
 ````
 
+Notes:
+- For `"id"` field syntax, see [Package ID Specifications] in the reference.
+
+[Package ID Specifications]: ../reference/pkgid-spec.html
+
 ## OPTIONS
 
 ### Output Options