Fix cargo add behaving different when translating package name #13765

dohse · 2024-04-17T06:44:52Z

TODOs

~~Fuzzy match registry dependencies in cargo remove as well~~
cargo remove does not need fuzzy matching, because there is no unexpected behavior for the user
~~Don't duplicate name permutation generation~~
Unsure whether this is worth it
Shall we reject cases where multiple different permutations match?
Add comments that explain the behavior

rustbot · 2024-04-17T06:44:57Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/mod.rs

src/cargo/ops/cargo_add/mod.rs

torhovland · 2024-08-23T13:34:17Z

This PR looks to be basically ready. @dohse, is there anything else you need to do before a final review? I see an unfinished TODO list in the description and one review comment that may not be resolved yet.

dohse · 2024-08-23T16:16:02Z

Yes, this PR should be ready. I need feedback to my responses (@epage). I would take another look at my TODOs when the general direction of the fix is clear.

epage · 2024-08-27T20:33:11Z

Yes, this PR should be ready. I need feedback to my responses (@epage). I would take another look at my TODOs when the general direction of the fix is clear.

In looking at the responses, its not clear to me what I needed responding to.

dohse · 2024-08-28T04:34:59Z

I responded to your comments - I will also rebase this PR.

In looking at the responses, its not clear to me what I needed responding to.

Sorry - this PR became confusing, because it contains two possible solutions. I was mostly referring to this comment, that I broke out a function as suggested.

dohse · 2024-08-29T13:59:05Z

I hope that I addressed the comment.

I am also now only check permutations that are unique

dohse · 2024-08-30T10:30:06Z

I changed the comments according to the discussion.

dohse · 2024-09-01T13:55:05Z

I updated the comment.

I also removed the .dedup() of the combinations - it would not prevent all lookups of the unmodified original basename.

epage · 2024-09-03T21:14:37Z

Thanks!

@bors r+

bors · 2024-09-03T21:14:39Z

📌 Commit 9391bfb has been approved by epage

It is now in the queue for this repository.

Fix cargo add behaving different when translating package name Fixes #13702 Fixes #10680 TODOs - [x] ~Fuzzy match registry dependencies in `cargo remove` as well~ `cargo remove` does not need fuzzy matching, because there is no unexpected behavior for the user - [x] ~Don't duplicate name permutation generation~ Unsure whether this is worth it - [ ] Shall we reject cases where multiple different permutations match? - [x] Add comments that explain the behavior

bors · 2024-09-03T21:15:46Z

⌛ Testing commit 9391bfb with merge 1ed3596...

bors · 2024-09-03T21:43:30Z

💔 Test failed - checks-actions

epage · 2024-09-04T00:16:40Z

@bors retry

bors · 2024-09-04T00:17:49Z

⌛ Testing commit 9391bfb with merge 40b6638...

bors · 2024-09-04T00:48:03Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing 40b6638 to master...

Update cargo 24 commits in c1fa840a85eca53818895901a53fae34247448b2..468f1500bdca6591555b204ef31f92d725053190 2024-08-29 21:03:53 +0000 to 2024-09-14 19:24:54 +0000 - Disable the shell_completions tests (rust-lang/cargo#14546) - fix(vendor): trust crate version only when coming from registries (rust-lang/cargo#14530) - docs: Feature resolver version 2: clarify use of 'target' (rust-lang/cargo#14540) - Update docs for how cargo is published (rust-lang/cargo#14539) - feat: Add native comlpetion with CompleteEnv under the nightly (rust-lang/cargo#14493) - fix(new): Add to workspace relative to manifest, not current-dir (rust-lang/cargo#14505) - Fix parsing of comma separated values in --crate-type flag (rust-lang/cargo#14499) - Include public/private dependency status in `cargo metadata` (rust-lang/cargo#14504) - Remove unnecessary symbols (rust-lang/cargo#14519) - docs: bin source can be `src/main.rs` and/or in `src/bin/` (rust-lang/cargo#14515) - fix(toml): Don't require MSRV bump for pub/priv (rust-lang/cargo#14507) - bail before packaging on same version (rust-lang/cargo#14448) - Implement path-bases (RFC 3529) 2/n: `cargo [add|remove|update]` support (rust-lang/cargo#14427) - Publish workspace (rust-lang/cargo#14433) - Bump ci's version of cargo-semver-version (rust-lang/cargo#14503) - Document -Zpackage-workspace (rust-lang/cargo#14496) - uplift windows gnullvm import libraries (rust-lang/cargo#14451) - Bump to 0.84.0; update changelog (rust-lang/cargo#14495) - Fix cargo add behaving different when translating package name (rust-lang/cargo#13765) - chore(deps): update rust crate core-foundation to 0.10.0 (rust-lang/cargo#14475) - feat(resolve): Report MSRV compatible version instead of incomptible (rust-lang/cargo#14471) - Don't automatically include the current crate when packaging (rust-lang/cargo#14488) - Fix elided lifetime (rust-lang/cargo#14487) - chore(deps): update rust crate pasetors to 0.7.0 (rust-lang/cargo#14478) r? ghost

Update cargo 25 commits in c1fa840a85eca53818895901a53fae34247448b2..a9a418d1a22f29e7dfd034e3b93f15657e608a29 2024-08-29 21:03:53 +0000 to 2024-09-15 19:13:12 +0000 - chore: revert change to Cargo.lock in f25806c (rust-lang/cargo#14547) - Disable the shell_completions tests (rust-lang/cargo#14546) - fix(vendor): trust crate version only when coming from registries (rust-lang/cargo#14530) - docs: Feature resolver version 2: clarify use of 'target' (rust-lang/cargo#14540) - Update docs for how cargo is published (rust-lang/cargo#14539) - feat: Add native comlpetion with CompleteEnv under the nightly (rust-lang/cargo#14493) - fix(new): Add to workspace relative to manifest, not current-dir (rust-lang/cargo#14505) - Fix parsing of comma separated values in --crate-type flag (rust-lang/cargo#14499) - Include public/private dependency status in `cargo metadata` (rust-lang/cargo#14504) - Remove unnecessary symbols (rust-lang/cargo#14519) - docs: bin source can be `src/main.rs` and/or in `src/bin/` (rust-lang/cargo#14515) - fix(toml): Don't require MSRV bump for pub/priv (rust-lang/cargo#14507) - bail before packaging on same version (rust-lang/cargo#14448) - Implement path-bases (RFC 3529) 2/n: `cargo [add|remove|update]` support (rust-lang/cargo#14427) - Publish workspace (rust-lang/cargo#14433) - Bump ci's version of cargo-semver-version (rust-lang/cargo#14503) - Document -Zpackage-workspace (rust-lang/cargo#14496) - uplift windows gnullvm import libraries (rust-lang/cargo#14451) - Bump to 0.84.0; update changelog (rust-lang/cargo#14495) - Fix cargo add behaving different when translating package name (rust-lang/cargo#13765) - chore(deps): update rust crate core-foundation to 0.10.0 (rust-lang/cargo#14475) - feat(resolve): Report MSRV compatible version instead of incomptible (rust-lang/cargo#14471) - Don't automatically include the current crate when packaging (rust-lang/cargo#14488) - Fix elided lifetime (rust-lang/cargo#14487) - chore(deps): update rust crate pasetors to 0.7.0 (rust-lang/cargo#14478)

rustbot assigned weihanglo Apr 17, 2024

rustbot added Command-add S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2024

dohse force-pushed the fix-13702 branch 2 times, most recently from b7d4b79 to 8340710 Compare April 17, 2024 07:24

epage reviewed Apr 17, 2024

View reviewed changes

tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/mod.rs Outdated Show resolved Hide resolved

dohse force-pushed the fix-13702 branch from 8340710 to 72af7aa Compare April 17, 2024 16:19

epage reviewed Apr 17, 2024

View reviewed changes

src/cargo/ops/cargo_add/mod.rs Outdated Show resolved Hide resolved

weihanglo assigned epage and unassigned weihanglo Apr 18, 2024

dohse force-pushed the fix-13702 branch 2 times, most recently from 44abbff to d24d92e Compare May 1, 2024 13:32

epage reviewed May 1, 2024

View reviewed changes

src/cargo/ops/cargo_add/mod.rs Outdated Show resolved Hide resolved

epage reviewed May 1, 2024

View reviewed changes

src/cargo/ops/cargo_add/mod.rs Outdated Show resolved Hide resolved

epage reviewed May 1, 2024

View reviewed changes

src/cargo/ops/cargo_add/mod.rs Outdated Show resolved Hide resolved

dohse force-pushed the fix-13702 branch 2 times, most recently from 6e1e369 to 543a138 Compare May 2, 2024 10:34

epage force-pushed the fix-13702 branch 2 times, most recently from 8eac893 to acd05f0 Compare August 28, 2024 13:51

dohse force-pushed the fix-13702 branch 5 times, most recently from 1569c49 to adf38c6 Compare August 29, 2024 13:57

Add test case for feature adding with fuzzy name rust-lang#10680

140911c

dohse force-pushed the fix-13702 branch 2 times, most recently from 9e079de to 5963260 Compare August 30, 2024 10:29

Fix rust-lang#10680, Fix rust-lang#13702

9391bfb

dohse force-pushed the fix-13702 branch from 5963260 to 9391bfb Compare September 1, 2024 13:49

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 Sep 3, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 3, 2024

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 Sep 4, 2024

bors merged commit 40b6638 into rust-lang:master Sep 4, 2024
24 checks passed

dohse deleted the fix-13702 branch September 5, 2024 14:43

weihanglo mentioned this pull request Sep 15, 2024

Update cargo rust-lang/rust#130377

Merged

rustbot added this to the 1.83.0 milestone Sep 16, 2024

weihanglo mentioned this pull request Sep 20, 2024

Inconsistent behavior when adding dependency tracked in workspace to member #14574

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cargo add behaving different when translating package name #13765

Fix cargo add behaving different when translating package name #13765

dohse commented Apr 17, 2024 •

edited

Loading

rustbot commented Apr 17, 2024

torhovland commented Aug 23, 2024

dohse commented Aug 23, 2024

epage commented Aug 27, 2024

dohse commented Aug 28, 2024

dohse commented Aug 29, 2024

dohse commented Aug 30, 2024

dohse commented Sep 1, 2024

epage commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

epage commented Sep 4, 2024

bors commented Sep 4, 2024

bors commented Sep 4, 2024

Fix cargo add behaving different when translating package name #13765

Fix cargo add behaving different when translating package name #13765

Conversation

dohse commented Apr 17, 2024 • edited Loading

rustbot commented Apr 17, 2024

torhovland commented Aug 23, 2024

dohse commented Aug 23, 2024

epage commented Aug 27, 2024

dohse commented Aug 28, 2024

dohse commented Aug 29, 2024

dohse commented Aug 30, 2024

dohse commented Sep 1, 2024

epage commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

epage commented Sep 4, 2024

bors commented Sep 4, 2024

bors commented Sep 4, 2024

dohse commented Apr 17, 2024 •

edited

Loading