fix(package): check dirtiness of path fields in manifest #14966
Merged
Conversation
This file contains 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
3 tasks
a3c9f3d to
29ad71a
Compare
|
☔ The latest upstream changes (presumably 71678a4) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment has been minimized.
This comment has been minimized.
29ad71a to
0f33c52
Compare
0f33c52 to
2546939
Compare
epage
approved these changes
Dec 24, 2024
This is a test showing corner cases that dirty files outside the package being packaging actually made the `.crate` file dirty. However, `cargo package` and `.cargo_vcs_info.json` didn't capture it.
This adds a special case for `package.{readme,license-file}`
to Git VCS status check.
If they were specified with paths outside the current package root,
but still under git workdir, Cargo checks git status of those files
to determine if they were dirty.
We don't need to take care of other fields with path values because
* `PathSource` only list files under the package root.
Things like `target.path` works for `cargo build`, but won't be
included in `.crate` file from `cargo publish`.
* The only exceptions are `package.readme`/`package.license-file`.
Cargo would copy files over if they are outside package root.
2546939 to
863e1f4
Compare
This was referenced Dec 24, 2024
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 25, 2024
Update cargo 6 commits in 652623b779c88fe44afede28bf7f1c9c07812511..c86f4b3a1b153218e6e50861214b0b4b4e695f23 2024-12-20 15:44:42 +0000 to 2024-12-24 17:49:48 +0000 - fix(package): check dirtiness of path fields in manifest (rust-lang/cargo#14966) - test: make path arguments more generic and flexible (rust-lang/cargo#14979) - Moved manifest metadata tracking from fingerprint to dep info (rust-lang/cargo#14973) - fix: assure possibly blocking non-files (like FIFOs) won't be picked up for publishing. (rust-lang/cargo#14977) - simplify SourceID Hash (rust-lang/cargo#14800) - upgrade `gix` to the latest release 0.69. (rust-lang/cargo#14975)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 31, 2024
### What does this PR try to resolve? This adds a special case for checking source files are symlinks and have been modified when under a VCS control This is required because those paths may link to a file outside the current package root, but still under the git workdir, affecting the final packaged `.crate` file. ### How should we test and review this PR? Pretty similar to #14966, as a part of #14967. This may have potential performance issue. If a package contains thousands of symlinks, Cargo will fire `git status` for each of them. Not sure if we want to do anything proactively now. The introduction of the `PathEntry` struct gives us more room for storing file metadata to satisfiy use cases in the future. For instances, * Knowing a source file is a symlink and resolving it when packaging on Windows * #5664 * #14965 * Knowing more about a file's metadata (e.g. permission bits from Git) * #4413 * #8006 * Provide richer information for `cargo package --list`, for example JSON output mode * #11666 * #13331 * #13953
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?
This adds a special case for
package.{readme,license-file}to Git VCS status check.
If they were specified with paths outside the current package root,
but still under git workdir, Cargo checks git status of those files
to determine if they were dirty.
We don't need to take care of other fields with path values because
PathSourceonly list files under the package root.Things like
target.pathworks forcargo build, but won't beincluded in
.cratefile fromcargo publish.package.readme/package.license-file.Cargo would copy files over if they are outside package root.
How should we test and review this PR?
While this doesn't fix ever case listed in #14967,
it at least fixes one of them.