Reserve filename Cargo.toml.orig in cargo package#10551
Merged
bors merged 3 commits intorust-lang:masterfrom Apr 12, 2022
Merged
Reserve filename Cargo.toml.orig in cargo package#10551bors merged 3 commits intorust-lang:masterfrom
Cargo.toml.orig in cargo package#10551bors merged 3 commits intorust-lang:masterfrom
Conversation
|
r? @ehuss (rust-highfive has picked a reviewer for you, use r? to override) |
weihanglo
reviewed
Apr 11, 2022
Member
weihanglo
left a comment
There was a problem hiding this comment.
Looks good!
One little suggestion: can you make Cargo.toml.orig a constant, like what VCS_INFO_FILE does? Line 222-233 also need to update accordingly. Thanks!
Contributor
Author
|
@weihanglo Sure, done. 🙂 |
Member
|
@bors r+ |
Contributor
|
📌 Commit e71fb81 has been approved by |
Contributor
Contributor
|
☀️ Test successful - checks-actions |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 14, 2022
Update cargo 11 commits in e2e2dddebe66dfc1403a312653557e332445308b..dba5baf4345858c591517b24801902a062c399f8 2022-04-05 17:04:53 +0000 to 2022-04-13 21:58:27 +0000 - Part 6 of RFC2906 - Switch the inheritance source from `workspace` to… (rust-lang/cargo#10564) - Part 5 of RFC2906 - Add support for inheriting `rust-version` (rust-lang/cargo#10563) - Add support for rustc --check-cfg well known names and values (rust-lang/cargo#10486) - Reserve filename `Cargo.toml.orig` in `cargo package` (rust-lang/cargo#10551) - Retry command invocation with argfile (rust-lang/cargo#10546) - Add a progress indicator for `cargo clean` (rust-lang/cargo#10236) - Ensure host units don't depend on Docscrape units, fixes rust-lang/cargo#10545 (rust-lang/cargo#10549) - Fix docs: Bindeps env vars are passed to build script at runtime (rust-lang/cargo#10550) - Part 4 of RFC2906 - Add support for inheriting `readme` (rust-lang/cargo#10548) - Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.path` (rust-lang/cargo#10538) - Bump to 0.63.0, update changelog (rust-lang/cargo#10544)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 14, 2022
Update cargo 11 commits in e2e2dddebe66dfc1403a312653557e332445308b..dba5baf4345858c591517b24801902a062c399f8 2022-04-05 17:04:53 +0000 to 2022-04-13 21:58:27 +0000 - Part 6 of RFC2906 - Switch the inheritance source from `workspace` to… (rust-lang/cargo#10564) - Part 5 of RFC2906 - Add support for inheriting `rust-version` (rust-lang/cargo#10563) - Add support for rustc --check-cfg well known names and values (rust-lang/cargo#10486) - Reserve filename `Cargo.toml.orig` in `cargo package` (rust-lang/cargo#10551) - Retry command invocation with argfile (rust-lang/cargo#10546) - Add a progress indicator for `cargo clean` (rust-lang/cargo#10236) - Ensure host units don't depend on Docscrape units, fixes rust-lang/cargo#10545 (rust-lang/cargo#10549) - Fix docs: Bindeps env vars are passed to build script at runtime (rust-lang/cargo#10550) - Part 4 of RFC2906 - Add support for inheriting `readme` (rust-lang/cargo#10548) - Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.path` (rust-lang/cargo#10538) - Bump to 0.63.0, update changelog (rust-lang/cargo#10544)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR try to resolve?
(previously discussed on Zulip)
Currently,
cargo packagewill detect if there is an existing file named.cargo_vcs_info.jsonin the project folder and raise an error. This is to avoid collisions with the file it generates of the same name.However, there is no such logic for the other file it generates,
Cargo.toml.orig. If such a file exists in the project folder, instead of an error, or one file taking precedence, cargo will generate a tarball containing two files with the same name. For example, from a package I uploaded as a test:This PR is a two-line change to add this filename to the existing check for
.cargo_vcs_info.json.How should we test and review this PR?
I have added a unit test to verify that the expected error is produced, copying the existing unit test for
.cargo_vcs_info.json. I have manually tested the change locally and confirmed that it raises an error if the file exists, and has no effect if it does not. Given the small size of this change, I think this is sufficient, but please let me know if anything else is expected.Additional information
This raises a separate question of whether Cargo or crates.io should prohibit tarballs with duplicate filenames. It seems like most (all?)
tarimplementations will give precedence to the last file (which will be the existing copy here, not the generated one), but I don't believe this is specified behaviour, and it's possible that scripts scanning through tarballs directly (without first extracting to the filesystem) may not handle this case correctly. For example, I was working on a rough script to compare packaged libraries to their corresponding Git commits where possible, and this wasn't a case I had anticipated.In any case, it seems like preventing such tarballs from being created by accident (this PR) is a good place to start.