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.

Sign up for GitHub

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

Jump to bottom

do not try an exponential number of package names #12083

Merged
merged 1 commit into from
May 5, 2023
Merged

do not try an exponential number of package names #12083

merged 1 commit into from
May 5, 2023

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented May 3, 2023

re #11934, and as discussed in the cargo team meeting, this changes the strategy to "the original, all underscore, and all dashes".

I was excessively proud of the hyphen_combination_num based implementation when I came up with it. But it's always been a hack. I'm glad to be the one to remove it.

@rustbot
Copy link
Collaborator

rustbot commented May 3, 2023

r? @ehuss

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

@rustbot rustbot added A-registries Area: registries S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2023
weihanglo
weihanglo approved these changes May 5, 2023
View reviewed changes
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!

Just give a data point: The number of crate names on crates.io using a mix of dash and underscore grows from 623(#11934 (comment)) to 639 (2023-05-05-020041).

src/cargo/sources/registry/mod.rs Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member

Feel free to r=weihanglo or if you want to do an fcp.

@Eh2406
Copy link
Contributor Author

Eh2406 commented May 5, 2023

@bors r=weihanglo

@bors
Copy link
Contributor

bors commented May 5, 2023

📌 Commit d6021c9 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 May 5, 2023
@bors
Copy link
Contributor

bors commented May 5, 2023

⌛ Testing commit d6021c9 with merge e2d1488...

@bors
Copy link
Contributor

bors commented May 5, 2023

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

@bors bors merged commit e2d1488 into rust-lang:master May 5, 2023
@weihanglo
Copy link
Member

Is it worth a relnotes Release-note worthy for putting it in Compatibility Notes in RELEASE.md?

@Eh2406 Eh2406 added the relnotes Release-note worthy label May 6, 2023
@Eh2406 Eh2406 deleted the names branch May 6, 2023 15:02
@weihanglo weihanglo mentioned this pull request May 9, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2023
@bors
Auto merge of rust-lang#111409 - weihanglo:update-cargo, r=weihanglo
65dfca8 
Update cargo

10 commits in 569b648b5831ae8a515e90c80843a5287c3304ef..26b73d15a68fb94579f6d3590585ec0e9d81d3d5
2023-05-05 15:49:44 +0000 to 2023-05-09 20:28:03 +0000
- Update the semver-check script to be able to run in any directory. (rust-lang/cargo#12117)
- Semver: Note that it is not a breaking change to make an unsafe function safe (rust-lang/cargo#12116)
- Add more documentation for artifact-dependencies. (rust-lang/cargo#12110)
- changelog: move registry query fixes to the right place (rust-lang/cargo#12086)
- Disallow RUSTUP_TOOLCHAIN in the [env] table. (rust-lang/cargo#12107)
- Disallow RUSTUP_HOME in the [env] table. (rust-lang/cargo#12101)
- Fix redacting tokens in http debug. (rust-lang/cargo#12095)
- Fix self_signed_should_fail for macOS. (rust-lang/cargo#12097)
- Update git2 (rust-lang/cargo#12096)
- do not try an exponential number of package names (rust-lang/cargo#12083)

r? `@ghost`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request May 10, 2023
@bors
Auto merge of #111409 - weihanglo:update-cargo, r=weihanglo
e687794 
Update cargo

10 commits in 569b648b5831ae8a515e90c80843a5287c3304ef..26b73d15a68fb94579f6d3590585ec0e9d81d3d5
2023-05-05 15:49:44 +0000 to 2023-05-09 20:28:03 +0000
- Update the semver-check script to be able to run in any directory. (rust-lang/cargo#12117)
- Semver: Note that it is not a breaking change to make an unsafe function safe (rust-lang/cargo#12116)
- Add more documentation for artifact-dependencies. (rust-lang/cargo#12110)
- changelog: move registry query fixes to the right place (rust-lang/cargo#12086)
- Disallow RUSTUP_TOOLCHAIN in the [env] table. (rust-lang/cargo#12107)
- Disallow RUSTUP_HOME in the [env] table. (rust-lang/cargo#12101)
- Fix redacting tokens in http debug. (rust-lang/cargo#12095)
- Fix self_signed_should_fail for macOS. (rust-lang/cargo#12097)
- Update git2 (rust-lang/cargo#12096)
- do not try an exponential number of package names (rust-lang/cargo#12083)

r? `@ghost`
@weihanglo weihanglo mentioned this pull request May 13, 2023
Closed
@ehuss ehuss added this to the 1.71.0 milestone May 30, 2023
@qm3ster
Copy link

qm3ster commented Jul 18, 2023

I feel like the easiest to make mistake for mixed cases is inversion (my-rice_your-peas vs my_rice-your_peas), so perhaps that is one last case that needs to be included.
I initially thought it may actually be the only thing worth including, since if a user is using both symbols in one name they probably have different semantics in mind, while the substitution will also cover both non-mixed inversions.
However I now realize the crate author may still canonicalize both semantics to one or the other, so all are needed.

@epage epage mentioned this pull request Apr 29, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@ehuss ehuss

Labels
A-registries Area: registries relnotes Release-note worthy S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Eh2406 @rustbot @weihanglo @bors @qm3ster @ehuss