Read/write the encoded cargo update --precise in the same place
#12629
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-registries
epage
reviewed
Sep 6, 2023
src/cargo/core/source/source_id.rs
Outdated
| /// If so return the version currently in the lock file and the version to be updated to. | ||
| /// If specified, our own source will have a precise version listed of the form | ||
| // `<pkg>=<p_req>-><f_req>` where `<pkg>` is the name of a crate on | ||
| // this source, `<p_req>` is the version installed and `<f_req> is the |
There was a problem hiding this comment.
<f_req> doesn't have a closing back tick
epage
reviewed
Sep 6, 2023
src/cargo/core/source/source_id.rs
Outdated
Comment on lines
447
to
450
| /// Check if the precise data field stores information for this `name` | ||
| /// from a call to [with_precise_registry_version]. | ||
| /// If so return the version currently in the lock file and the version to be updated to. |
There was a problem hiding this comment.
nit: a newline between short description and the rest
epage
reviewed
Sep 6, 2023
src/cargo/core/source/source_id.rs
Outdated
Comment on lines
461
to
463
| .filter(|p| p.starts_with(name) && p[name.len()..].starts_with('=')) | ||
| .map(|p| { | ||
| let (current, requested) = p[name.len() + 1..].split_once("->").unwrap(); |
There was a problem hiding this comment.
nit: this could be simplified
.filter_map(|p|p.strip_prefix(name)?.strip_prefix('='))`
epage
approved these changes
Sep 6, 2023
Eh2406
force-pushed
the
WhyStringlyTyped
branch
from
4d3e007
to
6d56ac8
Compare
Eh2406
force-pushed
the
WhyStringlyTyped
branch
from
6d56ac8
to
4d63fbc
Compare
|
@bors r=epage |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
This was referenced Sep 6, 2023
bors
added a commit
that referenced
this pull request
Sep 7, 2023
Ues strip_prefix for cleaner code ### What does this PR try to resolve? In #12629 (review) Ed pointed out how much cleaner the code can be using `strip_prefix`, so I found a bunch more places where we should be using it. ### How should we test and review this PR? Internal refactor and test still pass.
bors
added a commit
that referenced
this pull request
Sep 8, 2023
Ues strip_prefix for cleaner code ### What does this PR try to resolve? In #12629 (review) Ed pointed out how much cleaner the code can be using `strip_prefix`, so I found a bunch more places where we should be using it. ### How should we test and review this PR? Internal refactor and test still pass.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 9, 2023
Update cargo 14 commits in d14c85f4e6e7671673b1a1bc87231ff7164761e1..2fc85d15a542bfb610aff7682073412cf635352f 2023-09-05 22:28:10 +0000 to 2023-09-09 01:49:46 +0000 - feat: Stabilize lints (rust-lang/cargo#12648) - Ues strip_prefix for cleaner code (rust-lang/cargo#12631) - fix: don't print _TOKEN suggestion when not applicable (rust-lang/cargo#12644) - Bump cargo-credential-1password to v0.4.0 (rust-lang/cargo#12641) - refactor: put `Source` trait under `cargo::sources` (rust-lang/cargo#12527) - Error out if `cargo clean --doc` is mixed with `-p`. (rust-lang/cargo#12637) - Add wrappers around std::fs::metadata (rust-lang/cargo#12636) - Add with_stdout_unordered. (rust-lang/cargo#12635) - Fix example for creating a git project test. (rust-lang/cargo#12632) - Read/write the encoded `cargo update --precise` in the same place (rust-lang/cargo#12629) - docs(guide): Apply feedback on CI (rust-lang/cargo#12630) - fix: improve warning for both token & credential-provider (rust-lang/cargo#12626) - Skip clean up `profile.release.package."*"` (rust-lang/cargo#12624) - Add MSRV validation GitHub Action for cargo-credential (rust-lang/cargo#12623)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR try to resolve?
There's a stringly typed interface between
cargo/src/cargo/ops/cargo_generate_lockfile.rs
Line 105 in de7537e
cargo/src/cargo/sources/registry/index.rs
Line 587 in de7537e
As far as I can tell, anyone could just create this internally meaningful
structurestring by passing it on the command line.This should get cleaned up, for now by moving the encoding and decoding in to the same file.
How should we test and review this PR?
Internal refactor and test still pass.
Additional information
I am hoping that in the redesign of
cargo updatewe can come up with a better design for smuggling this data from the API all the way to querying the registry. It seems like locking the dependency to the selected version would be conceptually simpler, or using the patch system, or something.