Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(metadata): Stabilize id format as PackageIDSpec #12914

Merged
merged 2 commits into from Jan 15, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Nov 3, 2023

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 ids 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, 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. The String wasn't publicly exposed due to a request by users but instead their PackageId type replaced using Strings 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

@rustbot
Copy link
Collaborator

rustbot commented Nov 3, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation Command-metadata S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 3, 2023
tests/testsuite/script.rs Outdated Show resolved Hide resolved
tests/testsuite/git.rs Outdated Show resolved Hide resolved
@epage epage added the T-cargo Team: Cargo label Nov 3, 2023
src/doc/man/cargo-metadata.md Outdated Show resolved Hide resolved
@epage epage force-pushed the metadata branch 2 times, most recently from 667ab2c to dd80f44 Compare November 7, 2023 17:28
bors added a commit that referenced this pull request Dec 6, 2023
feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs

### What does this PR try to resolve?

This tries to add just enough information to Package ID Specs that we can be sure they are unambiguous.  On its own thats important but this will unblock #12914 so we can have a user-facing ID that can be used with cargo's CLI.

More specifically, this adds
- `git+`, etc prefixes to the scheme
  - These were previously stabilized for `cargo metadata`s source id urls
  - Like with `SourceID`, this will allow `PackageIDSpec` to generate `directory+` and `local-registry+` prefixes but not parse them.  I'm assuming that the layers where those are dealt with they won't appear here but I don't have enough experience with them
- git ref query strings for URLs

Things from `SourceID` that this does not include
- precise: this seems less related to matching (e.g. ignored for `impl Ord for SourceId`)
- canonical URL for git kinds: this could be nice for users but users aren't the target audience and we can switch to these later

Fixes #10256

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

Per-commit

### Additional information
This makes it so you can take a package from `cargo metadata` and
then pass it with the `--package` option without worrying about
ambiguity.

Fixes rust-lang#7267
@epage epage marked this pull request as ready for review December 6, 2023 20:30
@epage
Copy link
Contributor Author

epage commented Dec 6, 2023

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Dec 6, 2023

Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Dec 6, 2023
@epage epage force-pushed the metadata branch 2 times, most recently from 39027ba to 111f0d0 Compare December 8, 2023 22:55
@weihanglo
Copy link
Member

@rfcbot reviewed

BTW, do we have a plan for SourceId?

@epage
Copy link
Contributor Author

epage commented Dec 11, 2023

BTW, do we have a plan for SourceId?

I do not as I don't see as much reason to stabilize it. The end-user benefit here is that you can run a cargo command against a package you came across in cargo metadata. I don't see similar use cases at this time for SourceId.

@epage epage force-pushed the metadata branch 3 times, most recently from d9a2730 to 8915e76 Compare December 12, 2023 17:17
@bors
Copy link
Collaborator

bors commented Jan 15, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 77f2da7 to master...

@bors bors merged commit 77f2da7 into rust-lang:master Jan 15, 2024
20 checks passed
@epage epage deleted the metadata branch January 15, 2024 22:36
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2024
Update cargo

10 commits in 84976cd699f4aea56cb3a90ce3eedeed9e20d5a5..1cff2ee6b92e0ad3f87c44b70b28f788b2528b3c
2024-01-12 15:55:43 +0000 to 2024-01-16 16:56:57 +0000
- doc: add a heading to highlight "How to find features enabled on dependencies" (rust-lang/cargo#13305)
- fix(cargo-update): `--precise` accept arbitrary git revisions (rust-lang/cargo#13250)
- Strip debuginfo when debuginfo is not requested (rust-lang/cargo#13257)
- Update ahash dependency to 0.8.7 (rust-lang/cargo#13301)
- docs: add more links to pkgid spec chapter (rust-lang/cargo#13298)
- fix(metadata): Stabilize id format as PackageIDSpec (rust-lang/cargo#12914)
- Introduce `-Zprecise-pre-release` unstable flag (rust-lang/cargo#13296)
- Delete sentence about parentheses being unsupported in license (rust-lang/cargo#13292)
- Add guidance on setting homepage in manifest (rust-lang/cargo#13293)
- Clarify the function of the test options (rust-lang/cargo#13236)

r? ghost
@rustbot rustbot added this to the 1.77.0 milestone Jan 17, 2024
Jake-Shadle added a commit to EmbarkStudios/krates that referenced this pull request Jan 19, 2024
This refactors the internals, and the public API, to support both the current "opaque" package ids as well as the new package id format stabilized in <rust-lang/cargo#12914>
Jake-Shadle added a commit to EmbarkStudios/krates that referenced this pull request Jan 19, 2024
* Refactor to support stabilized package ids

This refactors the internals, and the public API, to support both the current "opaque" package ids as well as the new package id format stabilized in <rust-lang/cargo#12914>

* Add test to validate opaque and stable match

* Remove unused code

* Fix lint

* Update snapshot

* Add docs
github-merge-queue bot pushed a commit to near/nearcore that referenced this pull request Mar 25, 2024
Release notes: https://blog.rust-lang.org/2024/03/21/Rust-1.77.0.html

With split_last_chunk bunch of stdx code can now be deleted. \o/

As for other changes:

- Rust now doesn’t consider Clone and Debug derives as using fields of
  a type.  This lead to a handful of dead_code warnings.  I’ve fixed
  them by either removing offending type or adding `allow(dead_code)`.

- Cargo package id format has been stabilised¹ and krates crate
  stopped working.  Why do we care about that crate? It’s a dependency
  of cargo-deny.  To have CI pass I’ve updated taiki-e/install-action
  so that it fetches the newest cargo-deny.

¹ rust-lang/cargo#12914
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Mar 29, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.77.0 (2024-03-21)
==========================

- [Reveal opaque types within the defining body for exhaustiveness checking.]
  (rust-lang/rust#116821)
- [Stabilize C-string literals.]
  (rust-lang/rust#117472)
- [Stabilize THIR unsafeck.]
  (rust-lang/rust#117673)
- [Add lint `static_mut_refs` to warn on references to mutable statics.]
  (rust-lang/rust#117556)
- [Support async recursive calls (as long as they have indirection).]
  (rust-lang/rust#117703)
- [Undeprecate lint `unstable_features` and make use of it in the compiler.]
  (rust-lang/rust#118639)
- [Make inductive cycles in coherence ambiguous always.]
  (rust-lang/rust#118649)
- [Get rid of type-driven traversal in const-eval interning]
  (rust-lang/rust#119044),
  only as a [future compatiblity lint]
  (rust-lang/rust#122204) for now.
- [Deny braced macro invocations in let-else.]
  (rust-lang/rust#119062)

Compiler
--------

- [Include lint `soft_unstable` in future breakage reports.]
  (rust-lang/rust#116274)
- [Make `i128` and `u128` 16-byte aligned on x86-based targets.]
  (rust-lang/rust#116672)
- [Use `--verbose` in diagnostic output.]
  (rust-lang/rust#119129)
- [Improve spacing between printed tokens.]
  (rust-lang/rust#120227)
- [Merge the `unused_tuple_struct_fields` lint into `dead_code`.]
  (rust-lang/rust#118297)
- [Error on incorrect implied bounds in well-formedness check]
  (rust-lang/rust#118553),
  with a temporary exception for Bevy.
- [Fix coverage instrumentation/reports for non-ASCII source code.]
  (rust-lang/rust#119033)
- [Fix `fn`/`const` items implied bounds and well-formedness check.]
  (rust-lang/rust#120019)
- [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.]
  (rust-lang/rust#118704)
- Add several new tier 3 targets:
  - [`aarch64-unknown-illumos`]
    (rust-lang/rust#112936)
  - [`hexagon-unknown-none-elf`]
    (rust-lang/rust#117601)
  - [`riscv32imafc-esp-espidf`]
    (rust-lang/rust#119738)
  - [`riscv32im-risc0-zkvm-elf`]
    (rust-lang/rust#117958)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------

- [Implement `From<&[T; N]>` for `Cow<[T]>`.]
  (rust-lang/rust#113489)
- [Remove special-case handling of `vec.split_off
  (0)`.](rust-lang/rust#119917)

Stabilized APIs
---------------

- [`array::each_ref`]
  (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref)
- [`array::each_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut)
- [`core::net`]
  (https://doc.rust-lang.org/stable/core/net/index.html)
- [`f32::round_ties_even`]
  (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even)
- [`f64::round_ties_even`]
  (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even)
- [`mem::offset_of!`]
  (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html)
- [`slice::first_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk)
- [`slice::first_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut)
- [`slice::split_first_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk)
- [`slice::split_first_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut)
- [`slice::last_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk)
- [`slice::last_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut)
- [`slice::split_last_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk)
- [`slice::split_last_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut)
- [`slice::chunk_by`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by)
- [`slice::chunk_by_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut)
- [`Bound::map`]
  (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map)
- [`File::create_new`]
  (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new)
- [`Mutex::clear_poison`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison)
- [`RwLock::clear_poison`]
  (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison)

Cargo
-----

- [Extend the build directive syntax with `cargo::`.]
  (rust-lang/cargo#12201)
- [Stabilize metadata `id` format as `PackageIDSpec`.]
  (rust-lang/cargo#12914)
- [Pull out as `cargo-util-schemas` as a crate.]
  (rust-lang/cargo#13178)
- [Strip all debuginfo when debuginfo is not requested.]
  (rust-lang/cargo#13257)
- [Inherit jobserver from env for all kinds of runners.]
  (rust-lang/cargo#12776)
- [Deprecate rustc plugin support in cargo.]
  (rust-lang/cargo#13248)

Rustdoc
-----

- [Allows links in markdown headings.]
  (rust-lang/rust#117662)
- [Search for tuples and unit by type with `()`.]
  (rust-lang/rust#118194)
- [Clean up the source sidebar's hide button.]
  (rust-lang/rust#119066)
- [Prevent JS injection from `localStorage`.]
  (rust-lang/rust#120250)

Misc
----

- [Recommend version-sorting for all sorting in style guide.]
  (rust-lang/rust#115046)

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Add more weirdness to `weird-exprs.rs`.]
  (rust-lang/rust#119028)
mystor added a commit to mystor/cargo-vet that referenced this pull request Apr 1, 2024
In rust-lang/cargo#12914, the format used for
PackageId strings in cargo metadata was changed. This led to a number of
test failures due to the ordering of nodes when logging graphs changing
relative to earlier versions of cargo.

To work around this issue, the ordering of nodes in the graph was
changed to be based on package name and version explicitly, rather than
implicitly through the PackageId string. This slightly changes the
ordering of some crates in outputs.

In addition, in order to keep tests passing across all versions, the
package_id member has been hidden from the JSON graph dump output. This
field was already missing for path dependencies.

Fixes mozilla#602
hubot pushed a commit to aosp-mirror/platform_development that referenced this pull request Apr 16, 2024
After rust-lang/cargo#12914, the format of the
package ID changed to be

    path+file:///path/crate_name#version

See https://doc.rust-lang.org/cargo/reference/pkgid-spec.html for
details on the format.

The test files were regenerated with `cargo metadata`. This procedure
is now documented in a `README.md` file for future generations.

Bug: 333808266
Test: atest --host cargo_embargo.test
Test: Ran `cargo_embargo generate cargo_embargo.json` on chrono
Change-Id: I055d19baf6e78f30b708296434082121762573af
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation Command-metadata disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Package spec in build plan and metadata?
6 participants