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

fix(package): warn if symlinks checked out as plain text files #14994

Merged
merged 4 commits into from
Dec 31, 2024

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Dec 31, 2024

What does this PR try to resolve?

cargo package will warn users when git core.symlinks is false
and some symlinks were checked out as plain files during packaging.

Git config core.symlinks defaults to true when unset.
In git-for-windows (and git as well),
the config should be set to false explicitly when the repo was created,
if symlink support wasn't detected 1.

We assume the config was always set at creation time and never changed.
So, if it is true, we don't bother users with any warning.

How should we test and review this PR?

CI passes.

This shares two commits 42dc4ef and c8c8223 with #14981.

I didn't commit to fix all symlink issues all at once.
This PR demonstrates how we could leverage metadata in PathEntry.
Maybe later we can really follow plain-text symlinks and resolve the issue.

Additional information

cc #5664

Footnotes

  1. https://github.com/git-for-windows/git/blob/f1241afcc7956918d5da33ef74abd9cbba369247/setup.c#L2394-L2403

@rustbot
Copy link
Collaborator

rustbot commented Dec 31, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-git Area: anything dealing with git Command-package Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 31, 2024
@weihanglo weihanglo force-pushed the git-core-symlinks-false branch 2 times, most recently from 72378eb to ffdab25 Compare December 31, 2024 04:23
This gives us more room to store file metadata.
For example,

* knowing a source file is a symlink and resolving it when packaging,
* providing a rich JSON output for `cargo package --list`
* enriching the `.cargo-vcs-info.json` with copied/symlinked file info
So that we can tell whether a path is a symlink and need to
traverse to the actual file to check dirtiness or copy real content.
@weihanglo weihanglo force-pushed the git-core-symlinks-false branch from ffdab25 to 06d7265 Compare December 31, 2024 16:54
@weihanglo weihanglo changed the title feat(package): warn if symlinks checked out as plain text files fix(package): warn if symlinks checked out as plain text files Dec 31, 2024
`cargo package` will warn users when git `core.symlinks` is `false`
and some symlinks were checked out as plain files during packaging.

Git config [`core.symlinks`] defaults to true when unset.
In git-for-windows (and git as well),
the config should be set to false explicitly when the repo was created,
if symlink support wasn't detected [^1].

We assume the config was always set at creation time and never changed.
So, if it is true, we don't bother users with any warning.

[^1]: <https://github.com/git-for-windows/git/blob/f1241afcc7956918d5da33ef74abd9cbba369247/setup.c#L2394-L2403>

[`core.symlinks`]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks
@weihanglo weihanglo force-pushed the git-core-symlinks-false branch from 06d7265 to 059fe16 Compare December 31, 2024 19:20
@weihanglo
Copy link
Member Author

All suggestions are resolved, and also slightly tweaked the message, as the code path is not specific to cargo package. Let me know if it is desired.

@ehuss ehuss assigned epage and unassigned ehuss Dec 31, 2024
@epage epage added this pull request to the merge queue Dec 31, 2024
Merged via the queue into rust-lang:master with commit d73d2ca Dec 31, 2024
20 checks passed
@weihanglo weihanglo deleted the git-core-symlinks-false branch December 31, 2024 21:21
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 31, 2024
Update cargo

5 commits in c86f4b3a1b153218e6e50861214b0b4b4e695f23..d73d2caf9e41a39daf2a8d6ce60ec80bf354d2a7
2024-12-24 17:49:48 +0000 to 2024-12-31 20:51:21 +0000
- fix(package): warn if symlinks checked out as plain text files (rust-lang/cargo#14994)
- test: track caller for `.crate` file publish verification (rust-lang/cargo#14992)
- test: relax panic output assertion (rust-lang/cargo#14989)
- test: relax `bad_crate_type` to only match error message prefix (rust-lang/cargo#14990)
- refactor(package): split `cargo_package` to modules (rust-lang/cargo#14982)

r? ghost
@rustbot rustbot added this to the 1.85.0 milestone Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-git Area: anything dealing with git Command-package Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants