Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(package): Fix lookups to capitalized workspace member's index entry #15216

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

epage
Copy link
Contributor

@epage epage commented Feb 21, 2025

What does this PR try to resolve?

When investigating a report of a package-rename bug in -Zpackage-workspace, I found that we weren't correctly naming the file for index entries for generating Cargo.lock and verifying. We must first to_lowercase the name.

In fixing this, I also tried to clarify the API to reduce the chance of this happening in the future. Still not great that the caller is expected to handle this and know about it. The problem is make_dep_path is shared between the index and .crate files which are handled differently (and sharing of the to_lowercase would be nice). Maybe if made a make_index_path that had an assert and called make_dep_path. I held off on that for now.

How should we test and review this PR?

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Feb 21, 2025

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-registries Area: registries Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 21, 2025
@@ -3,6 +3,12 @@
/// - [index from of Cargo's index on filesystem][1], and
/// - [index from Crates.io][2].
///
/// <div class="warning">
///
/// Note: For index files, `dep_name` must have had `to_lowercase` called on it.
Copy link
Member

Choose a reason for hiding this comment

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

To be frank, I don't know why we have chosen to lower the case for index file queries. Apart from that, this looks good.

@weihanglo weihanglo added this pull request to the merge queue Feb 21, 2025
Merged via the queue into rust-lang:master with commit 1d1d646 Feb 21, 2025
20 of 21 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2025
Update cargo

16 commits in ce948f4616e3d4277e30c75c8bb01e094910df39..1d1d646c06a84c1aa53967b394b7f1218f85db82
2025-02-14 20:32:07 +0000 to 2025-02-21 21:38:53 +0000
- fix(package): Fix lookups to capitalized workspace member's index entry (rust-lang/cargo#15216)
- chore(ci): Visually group output in Github (rust-lang/cargo#15218)
- chore(ci): Auto-update cargo-semver-checks (rust-lang/cargo#15212)
- chore(deps): update msrv (3 versions) to v1.83 (rust-lang/cargo#15217)
- docs(ref): Shift focus to resolver v3 (rust-lang/cargo#15213)
- fix: mention "3" as a valid value for "resolver" field in error message (rust-lang/cargo#15215)
- chore(deps): update msrv (1 version) to v1.85 (rust-lang/cargo#15211)
- fix: build warning in windows_reserved_names_are_allowed (rust-lang/cargo#15206)
- Typo: "togother" -&gt; "together" (rust-lang/cargo#15204)
- chore: bump to 0.88.0; update changelog (rust-lang/cargo#15202)
- Typo: "explicitally" -&gt; "explicitly" (rust-lang/cargo#15201)
- fix(add): Focus on error, rather than large feature lists  (rust-lang/cargo#15200)
- docs: Improve comments (rust-lang/cargo#15197)
- fix(embedded): Handle more parsing corner cases (rust-lang/cargo#15187)
- docs: docs for `-Zfeature-unification` (rust-lang/cargo#15189)
- Fix man page with malformed `{{#options}}` block (rust-lang/cargo#15191)
@rustbot rustbot added this to the 1.87.0 milestone Feb 22, 2025
@epage epage deleted the rename branch February 24, 2025 15:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-registries Area: registries Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants