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

feat(pkgid): Allow incomplete versions when unambigious #12614

Merged
merged 9 commits into from
Sep 16, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Sep 1, 2023

What does this PR try to resolve?

This was proposed in #12425 to help sand off some of the rough edges around cargo update for its wider use it would be getting. Its easy to accidentally get duplicate copies packages in a repo and a pain to have to specify the full version when cargo update -p foo@1 is sufficient to describe it.

Other effects

  • profile overrides also supports this since we already allow a spec to match multiple items
  • cargo clean -p foo@... already ignored the version, so now we also parse and ignore the partial version
  • cargo tree --prune will now accept partial versions and will match all of them

Parts not effected:

  • Replacements
    • Two of the cases were found and we treat it as if the version isn't present which will error, so I think that is correct

How should we test and review this PR?

This extracts PartialVersion from RustVersion where RustVersion is a more specialized variant, not allowing prerelease or build.

This works by adopting PartialVersion into PackageIdSpec. For PackageIdSpec::query, this will "just work".

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2023

r? @weihanglo

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-dependency-resolution Area: dependency resolution and the resolver A-manifest Area: Cargo.toml issues A-registries Area: registries A-semver Area: semver specifications, version matching, etc. A-workspaces Area: workspaces Command-add Command-package Command-remove S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 1, 2023
@epage epage added the T-cargo Team: Cargo label Sep 1, 2023
@rustbot rustbot added the A-documenting-cargo-itself Area: Cargo's documentation label Sep 1, 2023
@bors
Copy link
Contributor

bors commented Sep 3, 2023

☔ The latest upstream changes (presumably #12615) made this pull request unmergeable. Please resolve the merge conflicts.

@epage epage marked this pull request as ready for review September 5, 2023 15:55
@epage
Copy link
Contributor Author

epage commented Sep 5, 2023

This was mentioned in #12425 but because it was more of an aside, I want to do a straw poll to make sure people are on board specifically with this before merging as it is a one-way door.

The main deviation from the proposal in #12425 is that I made the change to PackageIdSpec and it applies everywhere that does. I'd recommend checking the documentation and test changes in 53f80bf before checking your box to make sure you are on board with it. The one part I'm least confident in is that the resolver uses PackageIdSpec, rather than PackageId, in some places.

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 5, 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 Sep 5, 2023
@bors
Copy link
Contributor

bors commented Sep 6, 2023

☔ The latest upstream changes (presumably #12629) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Sep 7, 2023

☔ The latest upstream changes (presumably #12527) made this pull request unmergeable. Please resolve the merge conflicts.

@epage epage force-pushed the partial branch 2 times, most recently from c61dc75 to 0f06cd8 Compare September 8, 2023 15:19
bors added a commit that referenced this pull request Sep 14, 2023
feat(pkgid): Allow incomplete versions when unambigious

### What does this PR try to resolve?

This was proposed in #12425 to help sand off some of the rough edges around `cargo update` for its wider use it would be getting.  Its easy to accidentally get duplicate copies packages in a repo and a pain to have to specify the full version when `cargo update -p foo@1` is sufficient to describe it.

Other effects
- profile overrides also supports this since we already allow a spec to match multiple items
- `cargo clean -p foo@...` already ignored the version, so now we also parse and ignore the partial version
- `cargo tree --prune` will now accept partial versions and will match all of them

Parts not effected:
- Replacements
  - Two of the cases were found and we treat it as if the version isn't present which will error, so I think that is correct

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

This extracts `PartialVersion` from `RustVersion` where `RustVersion` is a more specialized variant, not allowing prerelease or build.

This works by adopting `PartialVersion` into `PackageIdSpec`.  For `PackageIdSpec::query`, this will "just work".

### Additional information
@bors
Copy link
Contributor

bors commented Sep 14, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 14, 2023
@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 16, 2023

📌 Commit 82f9bd3 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 16, 2023
@bors
Copy link
Contributor

bors commented Sep 16, 2023

⌛ Testing commit 82f9bd3 with merge da498c8...

@bors
Copy link
Contributor

bors commented Sep 16, 2023

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

@bors bors merged commit da498c8 into rust-lang:master Sep 16, 2023
19 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023
Update cargo

6 commits in d5336f813df39d476e61fc46daabb1446350660a..b4ddf95ad9954118ac0dae835f2966394ad04c02
2023-09-14 19:55:49 +0000 to 2023-09-18 03:48:09 +0000
- doc: differentiate defaults for split-debuginfo (rust-lang/cargo#12680)
- feat(cli): Add '-n' to dry-run (rust-lang/cargo#12660)
- feat: stabilize credential-process and registry-auth (rust-lang/cargo#12649)
- refactor: use `RegistryOrIndex` enum to replace two booleans (rust-lang/cargo#12677)
- doc: clarify caret requirements (rust-lang/cargo#12679)
- feat(pkgid): Allow incomplete versions when unambigious (rust-lang/cargo#12614)

r? ghost
@epage epage deleted the partial branch September 18, 2023 15:41
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Sep 21, 2023
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Comment on lines +179 to +180
let req = v.exact_req();
if !req.matches(package_id.version()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes us to no longer require the build field matches which causes problems. Working on a fix which we'll need to backport.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #12806

epage added a commit to epage/cargo that referenced this pull request Oct 11, 2023
PR rust-lang#12614
- Changed ignored replaces to not-ignored
- Has bugs in matching replaces
epage added a commit to epage/cargo that referenced this pull request Oct 11, 2023
PR rust-lang#12614
- Changed ignored replaces to not-ignored
- Has bugs in matching replaces
epage added a commit to epage/cargo that referenced this pull request Oct 16, 2023
PR rust-lang#12614
- Changed ignored replaces to not-ignored
- Has bugs in matching replaces
epage added a commit to epage/cargo that referenced this pull request Oct 16, 2023
PR rust-lang#12614
- Changed ignored replaces to not-ignored
- Has bugs in matching replaces
bors added a commit that referenced this pull request Oct 18, 2023
fix(replace): Partial-version spec support

### What does this PR try to resolve?

#12614 changed package ID specs to allow fields in the version number to be optional.  This earliest branch with this change is `rust-1.74.0` (beta).  While `@Eh2406` was investigating version metadata issues in #12772, problems with the partial version change were found
- `replace`s that specify version metadata were ignored **(fixed with this PR)**
  - This also extends out to any other place a PackageIDSpec may show up, like `cargo check -p <name>`@<spec>``
  - We explicitly kept the same semantics of version requirements that pre-releases require opt-in.  If nothing else, this gives us more room to change semantics in the future if we ever fix the semantics for pre-release.
- `replace`s that don't specify version metadata when the `Cargo.lock` contained a version metadata, it would previously be ignored (with a warning) but now match **(unchanged with this PR)**
  - When the version metadata in `Cargo.lock` differed from the overriding `Cargo.toml`, cargo would panic **(now an error in this PR)**

With this PR, we are acknowledging that we changed behavior in taking ignored replaces (because of differences with version metadata) and applying them.  Seeing as version metadata is relatively rare, replaces are relatively rare, and differences in it for registries is unsupported, the impact seems very small.

The questions before us are
- Do we revert #12614 in `master` and `rust-1.74.0` or merge this PR into `master`
- If we merge this PR into `master`, do we cherry-pick this into `rust-1.74.0` or revert #12614, giving ourselves more time to find problems

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

The initial commit adds tests that pass as of #12614.  Prior to #12614, these tests would have warned that the `replace` was unused and failed because `bar::bar` didn't exist.  Each commit then changes the behavior (or not) and updates the corresponding test.

### Additional information
epage added a commit to epage/cargo that referenced this pull request Oct 18, 2023
PR rust-lang#12614
- Changed ignored replaces to not-ignored
- Has bugs in matching replaces
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 8, 2024
Pkgsrc changes:

 * Remove NetBSD-8 support (embedded LLVm requires newer C++
   than what is in -8; it's conceivable that this could still
   build with an external LLVM)
 * undo powerpc 9.0 file naming tweak, since we no longer support -8.
 * Remove patch to LLVM for powerpc now included by upstream.
 * Minor adjustments, checksum changes etc.


Upstream changes:

Version 1.74.1 (2023-12-07)
===========================

- [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM]
  (rust-lang/rust#118464)
- [Clarify guarantees for std::mem::discriminant]
  (rust-lang/rust#118006)
- [Fix some subtyping-related regressions]
  (rust-lang/rust#116415)

Version 1.74.0 (2023-11-16)
==========================

Language
--------

- [Codify that `std::mem::Discriminant<T>` does not depend on any
  lifetimes in T]
  (rust-lang/rust#104299)
- [Replace `private_in_public` lint with `private_interfaces` and
  `private_bounds` per RFC 2145]
  (rust-lang/rust#113126)
  Read more in
  [RFC 2145](https://rust-lang.github.io/rfcs/2145-type-privacy.html).
- [Allow explicit `#[repr(Rust)]`]
  (rust-lang/rust#114201)
- [closure field capturing: don't depend on alignment of packed fields]
  (rust-lang/rust#115315)
- [Enable MIR-based drop-tracking for `async` blocks]
  (rust-lang/rust#107421)

Compiler
--------

- [stabilize combining +bundle and +whole-archive link modifiers]
  (rust-lang/rust#113301)
- [Stabilize `PATH` option for `--print KIND=PATH`]
  (rust-lang/rust#114183)
- [Enable ASAN/LSAN/TSAN for `*-apple-ios-macabi`]
  (rust-lang/rust#115644)
- [Promote loongarch64-unknown-none* to Tier 2]
  (rust-lang/rust#115368)
- [Add `i686-pc-windows-gnullvm` as a tier 3 target]
  (rust-lang/rust#115687)

Libraries
---------

- [Implement `From<OwnedFd/Handle>` for ChildStdin/out/err]
  (rust-lang/rust#98704)
- [Implement `From<{&,&mut} [T; N]>` for `Vec<T>` where `T: Clone`]
  (rust-lang/rust#111278)
- [impl Step for IP addresses]
  (rust-lang/rust#113748)
- [Implement `From<[T; N]>` for `Rc<[T]>` and `Arc<[T]>`]
  (rust-lang/rust#114041)
- [`impl TryFrom<char> for u16`]
  (rust-lang/rust#114065)
- [Stabilize `io_error_other` feature]
  (rust-lang/rust#115453)
- [Stabilize the `Saturating` type]
  (rust-lang/rust#115477)
- [Stabilize const_transmute_copy]
  (rust-lang/rust#115520)

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

- [`core::num::Saturating`]
  (https://doc.rust-lang.org/stable/std/num/struct.Saturating.html)
- [`impl From<io::Stdout> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStdout%3E-for-Stdio)
- [`impl From<io::Stderr> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedHandle> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedFd> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`std::ffi::OsString::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsString::into_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.into_encoded_bytes)
- [`std::ffi::OsStr::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsStr::as_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.as_encoded_bytes)
- [`std::io::Error::other`]
  (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.other)
- [`impl TryFrom<char> for u16`]
  (https://doc.rust-lang.org/stable/std/primitive.u16.html#impl-TryFrom%3Cchar%3E-for-u16)
- [`impl<T: Clone, const N: usize> From<&[T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T: Clone, const N: usize> From<&mut [T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26mut+%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Arc<[T]>`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#impl-From%3C%5BT;+N%5D%3E-for-Arc%3C%5BT%5D,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Rc<[T]>`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#impl-From%3C%5BT;+N%5D%3E-for-Rc%3C%5BT%5D,+Global%3E)

These APIs are now stable in const contexts:

- [`core::mem::transmute_copy`]
  (https://doc.rust-lang.org/beta/std/mem/fn.transmute_copy.html)
- [`str::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.str.html#method.is_ascii)
- [`[u8]::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.is_ascii)

Cargo
-----

- [fix: Set MSRV for internal packages]
  (rust-lang/cargo#12381)
- [config: merge lists in precedence order]
  (rust-lang/cargo#12515)
- [fix(update): Clarify meaning of --aggressive as --recursive]
  (rust-lang/cargo#12544)
- [fix(update): Make `-p` more convenient by being positional]
  (rust-lang/cargo#12545)
- [feat(help): Add styling to help output ]
  (rust-lang/cargo#12578)
- [feat(pkgid): Allow incomplete versions when unambigious]
  (rust-lang/cargo#12614)
- [feat: stabilize credential-process and registry-auth]
  (rust-lang/cargo#12649)
- [feat(cli): Add '-n' to dry-run]
  (rust-lang/cargo#12660)
- [Add support for `target.'cfg(..)'.linker`]
  (rust-lang/cargo#12535)
- [Stabilize `--keep-going`]
  (rust-lang/cargo#12568)
- [feat: Stabilize lints]
  (rust-lang/cargo#12648)

Rustdoc
-------

- [Add warning block support in rustdoc]
  (rust-lang/rust#106561)
- [Accept additional user-defined syntax classes in fenced code blocks]
  (rust-lang/rust#110800)
- [rustdoc-search: add support for type parameters]
  (rust-lang/rust#112725)
- [rustdoc: show inner enum and struct in type definition for concrete type]
  (rust-lang/rust#114855)

Compatibility Notes
-------------------

- [Raise minimum supported Apple OS versions]
  (rust-lang/rust#104385)
- [make Cell::swap panic if the Cells partially overlap]
  (rust-lang/rust#114795)
- [Reject invalid crate names in `--extern`]
  (rust-lang/rust#116001)
- [Don't resolve generic impls that may be shadowed by dyn built-in impls]
  (rust-lang/rust#114941)

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.

None this cycle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation A-manifest Area: Cargo.toml issues A-registries Area: registries A-semver Area: semver specifications, version matching, etc. A-workspaces Area: workspaces Command-add Command-clean Command-package Command-remove 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants