fix: do not borrow shell across registry query by weihanglo · Pull Request #1364...
source link: https://github.com/rust-lang/cargo/pull/13647
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
fix: do not borrow shell across registry query #13647
Merged
Conversation
Member
What does this PR try to resolve?
Fixes #13646
How should we test and review this PR?
$ git clone https://github.com/taiki-e/easytime
$ cd easytime
$ cargo generate-lockfile
$ cargo update -Z minimal-versions --verbose
Additional information
A better and safer way is always calling gctx.shell() instead.
added Command-generate-lockfile
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
labels
Member
Author
|
Talked with @Muscraft and agree that it's safer to use |
Thanks for this! gctx.shell() is tricky, and I have shot myself in the foot with it many times (most recently when working on the linting system). I think removing bindings from it everywhere is a good idea.
Do you think it would be a good idea to add a test for this?
Member
Author
I lean toward not. Such a test cannot systematically prevent this kind of runtime borrow checking bug. |
Feel free to r= me when CI passes if I don't get to it first
Contributor
|
@bors r+ I tested this change locally against https://github.com/taiki-e/easytime and it fixes the panic |
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
Collaborator
|
☀️ Test successful - checks-actions |
Collaborator
|
👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Successfully merging this pull request may close these issues.
Panic with "already borrowed: BorrowMutError" since nightly-2024-03-26
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK