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

Fix BorrowMutError when calling cargo doc --open #9531

Merged
merged 4 commits into from
Jul 1, 2021
Merged

Fix BorrowMutError when calling cargo doc --open #9531

merged 4 commits into from
Jul 1, 2021

Conversation

5225225
Copy link
Contributor

@5225225 5225225 commented Jun 1, 2021 •
edited
Loading

I'm not sure why the existing test suite didn't catch this, it definitely calls cargo doc --open.

I had

[doc.extern-map]
std = "local"

in my .cargo/config.toml. Will write a test case that sets that and then tries to run cargo doc --open.

Closes #9530

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 1, 2021
@5225225 5225225 marked this pull request as draft June 1, 2021 21:56
@ehuss
Copy link
Contributor

ehuss commented Jun 3, 2021

Thanks for opening the PR! Let me know if you have any questions on how to add a test. There's some documentation in the contributor guide for running and writing tests, and the doc.rs file contains some examples of testing --open and rustdoc_extern_html.rs contains the tests for extern-map.

@5225225
Copy link
Contributor Author

5225225 commented Jun 7, 2021

I can't seem to get the test to use doc.extern-map, it just always gives that warning that it's not being used. I did copy the usage from the extern-map test that works, that didn't seem to help anything.

I'm assuming that it's not used if the cargo version is stable, but I am calling .masquerade_as_nightly_cargo(), so that should be fixing it? And I'm running the tests using a nightly cargo, since it's not the assert at the top that's tripping.

Can someone who's more familiar with the codebase than I help me out here, because I'm lost as to why this test isn't working.

ehuss
ehuss reviewed Jun 7, 2021
View reviewed changes
src/cargo/ops/cargo_doc.rs Outdated Show resolved Hide resolved
@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2021
@5225225
Copy link
Contributor Author

5225225 commented Jun 19, 2021

Okay, the test looks to be running fine, and I've tested that the test actually works (fails when it's meant to) by checking out the cargo_doc.rs file from origin/master and ran the test, and it failed as expected.

@5225225 5225225 marked this pull request as ready for review June 19, 2021 20:38
ehuss
ehuss reviewed Jun 21, 2021
View reviewed changes
tests/testsuite/doc.rs Outdated Show resolved Hide resolved
tests/testsuite/doc.rs Outdated Show resolved Hide resolved
tests/testsuite/doc.rs Outdated Show resolved Hide resolved
@ehuss ehuss removed the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Jul 1, 2021
@ehuss
Copy link
Contributor

ehuss commented Jul 1, 2021

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 1, 2021

📌 Commit 950c415 has been approved by ehuss

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 1, 2021
@bors
Copy link
Collaborator

bors commented Jul 1, 2021

⌛ Testing commit 950c415 with merge 4952979...

@bors
Copy link
Collaborator

bors commented Jul 1, 2021

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 4952979 to master...

@bors bors merged commit 4952979 into rust-lang:master Jul 1, 2021
@ehuss ehuss mentioned this pull request Jul 1, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 1, 2021
@bors
Auto merge of rust-lang#86769 - ehuss:update-cargo, r=ehuss
64de497 
Update cargo

9 commits in 9233aa06c801801cff75df65df718d70905a235e..4952979031e2cf1d901c817a32e25a156a19db4c
2021-06-22 21:32:55 +0000 to 2021-07-01 01:14:50 +0000
- Fix `BorrowMutError` when calling `cargo doc --open` (rust-lang/cargo#9531)
- Exclude `target` from content-indexing on Windows (rust-lang/cargo#9635)
- Temporarily ignore 2021 edition fix. (rust-lang/cargo#9642)
- Temporarily disable future_incompat tests. (rust-lang/cargo#9638)
- Include toolchain specification in error message (rust-lang/cargo#9625)
- Error when packaging with git dependencies without version (rust-lang/cargo#9612)
- simply 'if' block (rust-lang/cargo#9615)
- tidy some closures and iterators (rust-lang/cargo#9614)
- use 'writeln' instead of appending newline character (rust-lang/cargo#9620)
@ehuss ehuss mentioned this pull request Jul 2, 2021
Merged
@ehuss ehuss added this to the 1.55.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss ehuss left review comments

Assignees

@ehuss ehuss

Labels
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.55.0
Development

Successfully merging this pull request may close these issues.

cargo doc --open fails with 'already borrowed: BorrowMutError'
4 participants
@5225225 @rust-highfive @ehuss @bors