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

Make the precise field of a source an Enum #12849

Merged
merged 5 commits into from
Oct 19, 2023
Merged

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Oct 18, 2023

What does this PR try to resolve?

The precise field of a source_id is a stringly typed untagged union for storing various kinds of data. This is a series of changes untangle what this field is used for. Eventually leading to it being stored as an Enum with dedicated helpers for setting or getting the values different use cases need.

How should we test and review this PR?

This is an internal re-factor, and all the tests still pass.
This can be reviewed one PR at a time. The first three look at patterns and how precise is used the throughout the code base and make dedicated helpers for each common use case. The last PR switches to an Enum and make helpers for the one-off use cases.

Additional information

I remember having an issue to do this work, but I cannot find it now.

It's not clear to me if I went overboard on making helpers, API design iteration is welcome.

It would be nice if the precise was entirely an internal of the source, this PR does not get us all the way there. This PR makes the representation of the field and internally detail, but its existence still matters to many other parts of the code. Suggestions are welcome.

@rustbot
Copy link
Collaborator

rustbot commented Oct 18, 2023

r? @weihanglo

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

@rustbot rustbot added A-crate-dependencies Area: [dependencies] of any kind A-dependency-resolution Area: dependency resolution and the resolver A-git Area: anything dealing with git A-lockfile Area: Cargo.lock issues A-registries Area: registries Command-generate-lockfile Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 18, 2023
src/cargo/core/source_id.rs Outdated Show resolved Hide resolved
@epage
Copy link
Contributor

epage commented Oct 18, 2023

Thanks for doing this! Last night I was thinking about this being needed for #12425

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Looks good :)

src/cargo/core/source_id.rs Outdated Show resolved Hide resolved
@Eh2406
Copy link
Contributor Author

Eh2406 commented Oct 19, 2023

Rebased and fixed.

@epage
Copy link
Contributor

epage commented Oct 19, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Oct 19, 2023

📌 Commit fa94b11 has been approved by epage

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 Oct 19, 2023
@bors
Copy link
Contributor

bors commented Oct 19, 2023

⌛ Testing commit fa94b11 with merge 48a79c9...

@bors
Copy link
Contributor

bors commented Oct 19, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 48a79c9 to master...

@bors bors merged commit 48a79c9 into rust-lang:master Oct 19, 2023
20 checks passed
@Eh2406 Eh2406 deleted the Precise branch October 19, 2023 18:36
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2023
Update cargo

22 commits in 8eb8acbb116e7923ea2ce33a50109933ed5ab375..d2f6a048529eb8e9ebc55d793abd63456c98fac2
2023-10-17 11:55:04 +0000 to 2023-10-20 18:25:30 +0000
- chore(deps): bump rustix from 0.38.18 to 0.38.19 (rust-lang/cargo#12851)
- refactor: centralize logic of getting max resolve version (rust-lang/cargo#12860)
- If there's a version in the lock file only use that exact version (rust-lang/cargo#12772)
- Make the precise field of a source an Enum (rust-lang/cargo#12849)
- fix(cli): Provide next steps for bad -Z flag (rust-lang/cargo#12857)
- fix(remove): Preserve feature comments (rust-lang/cargo#12837)
- docs: fix typo (rust-lang/cargo#12844)
- chore(triagebot): auto label when PR review state changes (rust-lang/cargo#12856)
- fix(add): Preserve more comments (rust-lang/cargo#12838)
- ci: big ⚠️ to ensure the CNAME file is always there (rust-lang/cargo#12853)
- docs(cargo-bench): `--bench` is passed in unconditionally to bench harnesses (rust-lang/cargo#12850)
- docs(contrib): generate redirection HTML pages in CI (rust-lang/cargo#12846)
- docs: remove review capacity notice (rust-lang/cargo#12842)
- fix(help):Clarify install's positional (rust-lang/cargo#12841)
- Adjust `-Zcheck-cfg` for new rustc syntax and behavior (rust-lang/cargo#12845)
- fix(replace): Partial-version spec support (rust-lang/cargo#12806)
- Print environment variables for build script executions with `-vv` (rust-lang/cargo#12829)
- fix(cli): Suggest cargo-search on bad commands (rust-lang/cargo#12840)
- docs(contrib): Policy on manifest editing (rust-lang/cargo#12836)
- ci/contrib: use separate concurrency group (rust-lang/cargo#12835)
- ci/contrib: do not fail on missing gh-pages (rust-lang/cargo#12834)
- Clarify flag behavior in `cargo remove --help` (rust-lang/cargo#12823)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 21, 2023
Update cargo

22 commits in 8eb8acbb116e7923ea2ce33a50109933ed5ab375..d2f6a048529eb8e9ebc55d793abd63456c98fac2
2023-10-17 11:55:04 +0000 to 2023-10-20 18:25:30 +0000
- chore(deps): bump rustix from 0.38.18 to 0.38.19 (rust-lang/cargo#12851)
- refactor: centralize logic of getting max resolve version (rust-lang/cargo#12860)
- If there's a version in the lock file only use that exact version (rust-lang/cargo#12772)
- Make the precise field of a source an Enum (rust-lang/cargo#12849)
- fix(cli): Provide next steps for bad -Z flag (rust-lang/cargo#12857)
- fix(remove): Preserve feature comments (rust-lang/cargo#12837)
- docs: fix typo (rust-lang/cargo#12844)
- chore(triagebot): auto label when PR review state changes (rust-lang/cargo#12856)
- fix(add): Preserve more comments (rust-lang/cargo#12838)
- ci: big ⚠️ to ensure the CNAME file is always there (rust-lang/cargo#12853)
- docs(cargo-bench): `--bench` is passed in unconditionally to bench harnesses (rust-lang/cargo#12850)
- docs(contrib): generate redirection HTML pages in CI (rust-lang/cargo#12846)
- docs: remove review capacity notice (rust-lang/cargo#12842)
- fix(help):Clarify install's positional (rust-lang/cargo#12841)
- Adjust `-Zcheck-cfg` for new rustc syntax and behavior (rust-lang/cargo#12845)
- fix(replace): Partial-version spec support (rust-lang/cargo#12806)
- Print environment variables for build script executions with `-vv` (rust-lang/cargo#12829)
- fix(cli): Suggest cargo-search on bad commands (rust-lang/cargo#12840)
- docs(contrib): Policy on manifest editing (rust-lang/cargo#12836)
- ci/contrib: use separate concurrency group (rust-lang/cargo#12835)
- ci/contrib: do not fail on missing gh-pages (rust-lang/cargo#12834)
- Clarify flag behavior in `cargo remove --help` (rust-lang/cargo#12823)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2023
Update cargo

22 commits in 8eb8acbb116e7923ea2ce33a50109933ed5ab375..d2f6a048529eb8e9ebc55d793abd63456c98fac2
2023-10-17 11:55:04 +0000 to 2023-10-20 18:25:30 +0000
- chore(deps): bump rustix from 0.38.18 to 0.38.19 (rust-lang/cargo#12851)
- refactor: centralize logic of getting max resolve version (rust-lang/cargo#12860)
- If there's a version in the lock file only use that exact version (rust-lang/cargo#12772)
- Make the precise field of a source an Enum (rust-lang/cargo#12849)
- fix(cli): Provide next steps for bad -Z flag (rust-lang/cargo#12857)
- fix(remove): Preserve feature comments (rust-lang/cargo#12837)
- docs: fix typo (rust-lang/cargo#12844)
- chore(triagebot): auto label when PR review state changes (rust-lang/cargo#12856)
- fix(add): Preserve more comments (rust-lang/cargo#12838)
- ci: big ⚠️ to ensure the CNAME file is always there (rust-lang/cargo#12853)
- docs(cargo-bench): `--bench` is passed in unconditionally to bench harnesses (rust-lang/cargo#12850)
- docs(contrib): generate redirection HTML pages in CI (rust-lang/cargo#12846)
- docs: remove review capacity notice (rust-lang/cargo#12842)
- fix(help):Clarify install's positional (rust-lang/cargo#12841)
- Adjust `-Zcheck-cfg` for new rustc syntax and behavior (rust-lang/cargo#12845)
- fix(replace): Partial-version spec support (rust-lang/cargo#12806)
- Print environment variables for build script executions with `-vv` (rust-lang/cargo#12829)
- fix(cli): Suggest cargo-search on bad commands (rust-lang/cargo#12840)
- docs(contrib): Policy on manifest editing (rust-lang/cargo#12836)
- ci/contrib: use separate concurrency group (rust-lang/cargo#12835)
- ci/contrib: do not fail on missing gh-pages (rust-lang/cargo#12834)
- Clarify flag behavior in `cargo remove --help` (rust-lang/cargo#12823)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2023
Update cargo

22 commits in 8eb8acbb116e7923ea2ce33a50109933ed5ab375..d2f6a048529eb8e9ebc55d793abd63456c98fac2
2023-10-17 11:55:04 +0000 to 2023-10-20 18:25:30 +0000
- chore(deps): bump rustix from 0.38.18 to 0.38.19 (rust-lang/cargo#12851)
- refactor: centralize logic of getting max resolve version (rust-lang/cargo#12860)
- If there's a version in the lock file only use that exact version (rust-lang/cargo#12772)
- Make the precise field of a source an Enum (rust-lang/cargo#12849)
- fix(cli): Provide next steps for bad -Z flag (rust-lang/cargo#12857)
- fix(remove): Preserve feature comments (rust-lang/cargo#12837)
- docs: fix typo (rust-lang/cargo#12844)
- chore(triagebot): auto label when PR review state changes (rust-lang/cargo#12856)
- fix(add): Preserve more comments (rust-lang/cargo#12838)
- ci: big ⚠️ to ensure the CNAME file is always there (rust-lang/cargo#12853)
- docs(cargo-bench): `--bench` is passed in unconditionally to bench harnesses (rust-lang/cargo#12850)
- docs(contrib): generate redirection HTML pages in CI (rust-lang/cargo#12846)
- docs: remove review capacity notice (rust-lang/cargo#12842)
- fix(help):Clarify install's positional (rust-lang/cargo#12841)
- Adjust `-Zcheck-cfg` for new rustc syntax and behavior (rust-lang/cargo#12845)
- fix(replace): Partial-version spec support (rust-lang/cargo#12806)
- Print environment variables for build script executions with `-vv` (rust-lang/cargo#12829)
- fix(cli): Suggest cargo-search on bad commands (rust-lang/cargo#12840)
- docs(contrib): Policy on manifest editing (rust-lang/cargo#12836)
- ci/contrib: use separate concurrency group (rust-lang/cargo#12835)
- ci/contrib: do not fail on missing gh-pages (rust-lang/cargo#12834)
- Clarify flag behavior in `cargo remove --help` (rust-lang/cargo#12823)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2023
Update cargo

22 commits in 8eb8acbb116e7923ea2ce33a50109933ed5ab375..d2f6a048529eb8e9ebc55d793abd63456c98fac2
2023-10-17 11:55:04 +0000 to 2023-10-20 18:25:30 +0000
- chore(deps): bump rustix from 0.38.18 to 0.38.19 (rust-lang/cargo#12851)
- refactor: centralize logic of getting max resolve version (rust-lang/cargo#12860)
- If there's a version in the lock file only use that exact version (rust-lang/cargo#12772)
- Make the precise field of a source an Enum (rust-lang/cargo#12849)
- fix(cli): Provide next steps for bad -Z flag (rust-lang/cargo#12857)
- fix(remove): Preserve feature comments (rust-lang/cargo#12837)
- docs: fix typo (rust-lang/cargo#12844)
- chore(triagebot): auto label when PR review state changes (rust-lang/cargo#12856)
- fix(add): Preserve more comments (rust-lang/cargo#12838)
- ci: big ⚠️ to ensure the CNAME file is always there (rust-lang/cargo#12853)
- docs(cargo-bench): `--bench` is passed in unconditionally to bench harnesses (rust-lang/cargo#12850)
- docs(contrib): generate redirection HTML pages in CI (rust-lang/cargo#12846)
- docs: remove review capacity notice (rust-lang/cargo#12842)
- fix(help):Clarify install's positional (rust-lang/cargo#12841)
- Adjust `-Zcheck-cfg` for new rustc syntax and behavior (rust-lang/cargo#12845)
- fix(replace): Partial-version spec support (rust-lang/cargo#12806)
- Print environment variables for build script executions with `-vv` (rust-lang/cargo#12829)
- fix(cli): Suggest cargo-search on bad commands (rust-lang/cargo#12840)
- docs(contrib): Policy on manifest editing (rust-lang/cargo#12836)
- ci/contrib: use separate concurrency group (rust-lang/cargo#12835)
- ci/contrib: do not fail on missing gh-pages (rust-lang/cargo#12834)
- Clarify flag behavior in `cargo remove --help` (rust-lang/cargo#12823)

r? ghost
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Oct 23, 2023
Update cargo

22 commits in 8eb8acbb116e7923ea2ce33a50109933ed5ab375..d2f6a048529eb8e9ebc55d793abd63456c98fac2
2023-10-17 11:55:04 +0000 to 2023-10-20 18:25:30 +0000
- chore(deps): bump rustix from 0.38.18 to 0.38.19 (rust-lang/cargo#12851)
- refactor: centralize logic of getting max resolve version (rust-lang/cargo#12860)
- If there's a version in the lock file only use that exact version (rust-lang/cargo#12772)
- Make the precise field of a source an Enum (rust-lang/cargo#12849)
- fix(cli): Provide next steps for bad -Z flag (rust-lang/cargo#12857)
- fix(remove): Preserve feature comments (rust-lang/cargo#12837)
- docs: fix typo (rust-lang/cargo#12844)
- chore(triagebot): auto label when PR review state changes (rust-lang/cargo#12856)
- fix(add): Preserve more comments (rust-lang/cargo#12838)
- ci: big ⚠️ to ensure the CNAME file is always there (rust-lang/cargo#12853)
- docs(cargo-bench): `--bench` is passed in unconditionally to bench harnesses (rust-lang/cargo#12850)
- docs(contrib): generate redirection HTML pages in CI (rust-lang/cargo#12846)
- docs: remove review capacity notice (rust-lang/cargo#12842)
- fix(help):Clarify install's positional (rust-lang/cargo#12841)
- Adjust `-Zcheck-cfg` for new rustc syntax and behavior (rust-lang/cargo#12845)
- fix(replace): Partial-version spec support (rust-lang/cargo#12806)
- Print environment variables for build script executions with `-vv` (rust-lang/cargo#12829)
- fix(cli): Suggest cargo-search on bad commands (rust-lang/cargo#12840)
- docs(contrib): Policy on manifest editing (rust-lang/cargo#12836)
- ci/contrib: use separate concurrency group (rust-lang/cargo#12835)
- ci/contrib: do not fail on missing gh-pages (rust-lang/cargo#12834)
- Clarify flag behavior in `cargo remove --help` (rust-lang/cargo#12823)

r? ghost
@ehuss ehuss added this to the 1.75.0 milestone Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-crate-dependencies Area: [dependencies] of any kind A-dependency-resolution Area: dependency resolution and the resolver A-git Area: anything dealing with git A-lockfile Area: Cargo.lock issues A-registries Area: registries Command-generate-lockfile Command-vendor S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants