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.

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

Skip extracting .cargo-ok files from packages #8835

Merged
merged 1 commit into from
Nov 9, 2020

Conversation

steven-joruk
Copy link
Contributor

This is for #8816

I'll look into adding a unit test tomorrow, I'm still familiarising myself with the project.

@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 Nov 5, 2020
@alexcrichton
Copy link
Member

Looks pretty reasonable to me, thanks! I think you can write a test for this with Package which allows specifying arbitrary files to include as well. I'd be sure to double-check that the test fails before this fix but there should be plenty of examples in tests/testsuite/*.rs about how to publish packages in the tests.

This fixes the case where a package contained an empty .cargo-ok file
and was mounted in a read only file system. This lead to attempting to
download the package again, which failed due to write permissions.
@steven-joruk
Copy link
Contributor Author

I also changed the fix when adding the tests, Alex. Now it just creates the lock file after unpacking, rather than skipping extracting a lock file from the package.

@alexcrichton
Copy link
Member

@bors: r+

Looks great to me, thanks!

@bors
Copy link
Contributor

bors commented Nov 9, 2020

📌 Commit 05a8127 has been approved by alexcrichton

@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 Nov 9, 2020
@bors
Copy link
Contributor

bors commented Nov 9, 2020

⌛ Testing commit 05a8127 with merge 03976af...

@bors
Copy link
Contributor

bors commented Nov 9, 2020

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 03976af to master...

@bors bors merged commit 03976af into rust-lang:master Nov 9, 2020
@ehuss
Copy link
Contributor

ehuss commented Nov 9, 2020

FWIW, I was intending for this to skip the .cargo-ok file during extraction. That would ensure that if extraction is interrupted, and the package contained a .cargo-ok file, that it would still retry to extract it next time. I think that is extremely unlikely to ever happen, so I don't think this needs to change, but just wanted to leave a note about it.

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 12, 2020
Update cargo

5 commits in d5556aeb8405b1fe696adb6e297ad7a1f2989b62..8662ab427a8d6ad8047811cc4d78dbd20dd07699
2020-11-04 22:20:36 +0000 to 2020-11-12 03:47:53 +0000
- Check if rust-src contains a vendor dir, and patch it in (rust-lang/cargo#8834)
- Improve performance of almost fresh builds (rust-lang/cargo#8837)
- Use u32/64::to/from_le_bytes instead of bit fiddling (rust-lang/cargo#8847)
- Avoid constructing an anyhow::Error when not necessary (rust-lang/cargo#8844)
- Skip extracting .cargo-ok files from packages (rust-lang/cargo#8835)
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 12, 2020
Update cargo

5 commits in d5556aeb8405b1fe696adb6e297ad7a1f2989b62..8662ab427a8d6ad8047811cc4d78dbd20dd07699
2020-11-04 22:20:36 +0000 to 2020-11-12 03:47:53 +0000
- Check if rust-src contains a vendor dir, and patch it in (rust-lang/cargo#8834)
- Improve performance of almost fresh builds (rust-lang/cargo#8837)
- Use u32/64::to/from_le_bytes instead of bit fiddling (rust-lang/cargo#8847)
- Avoid constructing an anyhow::Error when not necessary (rust-lang/cargo#8844)
- Skip extracting .cargo-ok files from packages (rust-lang/cargo#8835)
@ehuss ehuss added this to the 1.49.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants