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(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs #12933

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Nov 8, 2023

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 metadatas 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

@rustbot
Copy link
Collaborator

rustbot commented Nov 8, 2023

r? @weihanglo

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

@epage epage added the T-cargo Team: Cargo label Nov 8, 2023
@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2023
@epage
Copy link
Contributor Author

epage commented Nov 8, 2023

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 8, 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 Nov 8, 2023
bors added a commit that referenced this pull request Nov 10, 2023
refactor(source): Prepare for new PackageIDSpec syntax

### What does this PR try to resolve?

This adds tests and refactors to prepare for #12933

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

### Additional information
@bors
Copy link
Contributor

bors commented Nov 10, 2023

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

@rfcbot rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Dec 1, 2023
@rfcbot
Copy link
Collaborator

rfcbot commented Dec 1, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Dec 1, 2023
@epage epage force-pushed the pkgid-spec branch 3 times, most recently from d1f5a34 to ec64b41 Compare December 2, 2023 16:10
@epage epage changed the title feat(spec): Track kind + git ref feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs Dec 5, 2023
@weihanglo
Copy link
Member

Going to merge this. I don't think we need to wait for 10 day long.

If people think it is wrong, or have a better alternative, please call it out and we can reconsider.

@bors r+

@bors
Copy link
Contributor

bors commented Dec 6, 2023

📌 Commit 64f5b0c 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 Dec 6, 2023
@bors
Copy link
Contributor

bors commented Dec 6, 2023

⌛ Testing commit 64f5b0c with merge 9787229...

@bors
Copy link
Contributor

bors commented Dec 6, 2023

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

@bors bors merged commit 9787229 into rust-lang:master Dec 6, 2023
20 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 6, 2023
Update cargo

5 commits in 623b788496b3e51dc2f9282373cf0f6971a229b5..9787229614b27854cf73d57ffae430d7c1e6caa4
2023-12-02 18:10:03 +0000 to 2023-12-06 02:29:23 +0000
- feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs (rust-lang/cargo#12933)
- test(trim-paths): assert `OSO` and `SO` cannot be trimmed (rust-lang/cargo#13118)
- refactor(schemas): Pull out mod for proposed schemas package (rust-lang/cargo#13097)
- chore(test): remove unnecesary packages and versions for `optionals` tests (rust-lang/cargo#13108)
- chore(config): migrate renovate config (rust-lang/cargo#13106)

r? ghost
@rustbot rustbot added this to the 1.76.0 milestone Dec 6, 2023
@epage epage deleted the pkgid-spec branch December 6, 2023 14:51
@rfcbot rfcbot added finished-final-comment-period FCP complete and removed final-comment-period FCP — a period for last comments before action is taken labels Dec 11, 2023
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
Update cargo

5 commits in 623b788496b3e51dc2f9282373cf0f6971a229b5..9787229614b27854cf73d57ffae430d7c1e6caa4
2023-12-02 18:10:03 +0000 to 2023-12-06 02:29:23 +0000
- feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs (rust-lang/cargo#12933)
- test(trim-paths): assert `OSO` and `SO` cannot be trimmed (rust-lang/cargo#13118)
- refactor(schemas): Pull out mod for proposed schemas package (rust-lang/cargo#13097)
- chore(test): remove unnecesary packages and versions for `optionals` tests (rust-lang/cargo#13108)
- chore(config): migrate renovate config (rust-lang/cargo#13106)

r? ghost
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Update cargo

5 commits in 623b788496b3e51dc2f9282373cf0f6971a229b5..9787229614b27854cf73d57ffae430d7c1e6caa4
2023-12-02 18:10:03 +0000 to 2023-12-06 02:29:23 +0000
- feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs (rust-lang/cargo#12933)
- test(trim-paths): assert `OSO` and `SO` cannot be trimmed (rust-lang/cargo#13118)
- refactor(schemas): Pull out mod for proposed schemas package (rust-lang/cargo#13097)
- chore(test): remove unnecesary packages and versions for `optionals` tests (rust-lang/cargo#13108)
- chore(config): migrate renovate config (rust-lang/cargo#13106)

r? ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation 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
None yet
Development

Successfully merging this pull request may close these issues.

Multiple git dependencies with different refs confuses cargo
6 participants