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

cargo package / publish don't handle symlink correctly on Windows #5664

Open
upsuper opened this issue Jun 28, 2018 · 4 comments
Open

cargo package / publish don't handle symlink correctly on Windows #5664

upsuper opened this issue Jun 28, 2018 · 4 comments
Labels
A-git Area: anything dealing with git C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-package O-windows OS: Windows S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@upsuper
Copy link
Contributor

upsuper commented Jun 28, 2018

If you package a crate with symlink, the file would not be followed but be packed as a file with relative path as content. For example, if you checkout serde-rs/serde, and run cargo package in serde directory, the license files in the package directory would only have content like ../LICENSE-APACHE.

It's probably actually a problem of Git for Windows which may not checkout symlink correctly. But maybe Cargo can try harder to query the VCS for whether it's a symlink or not (e.g. use git ls-files -s).

Related issues: alexcrichton/cargo-vendor#77, #3896, #2748.

@alexcrichton
Copy link
Member

According to https://stackoverflow.com/questions/5917249/git-symlinks-in-windows this may be an msys/MinGW thing. In that case this may be quite difficult to fix...

@alexcrichton alexcrichton added O-windows OS: Windows C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Jun 29, 2018
@mati865
Copy link
Contributor

mati865 commented Jul 4, 2018

Due to permission error (yes, one cannot simply create symlink without admin rights on Windows) ln -s was configured to do copy instead.
See here to disable it.

christophermaier added a commit to habitat-sh/habitat that referenced this issue Aug 6, 2019
Some of our tests use nightly Rust to help test for specific
things. The version of Rust used was simply piggybacking on the
version of nightly Rust we use for `rustfmt`, rather than being
specifically managed.

Previously, this wasn't an issue, but with the latest nightly Rust
that works for `rustfmt`, we apparently have a bug around cargo
dealing with crates that have symlinks in them (possibly related to
rust-lang/cargo#5664).

This commit introduces a `RUST_NIGHTLY_VERSION` file to explicitly
manage which version we're using in tests. For now, it also pins back
to the last known good nightly version for testing purposes.
christophermaier added a commit to habitat-sh/habitat that referenced this issue Aug 6, 2019
Some of our tests use nightly Rust to help test for specific
things. The version of Rust used was simply piggybacking on the
version of nightly Rust we use for `rustfmt`, rather than being
specifically managed.

Previously, this wasn't an issue, but with the latest nightly Rust
that works for `rustfmt`, we apparently have a bug around cargo
dealing with crates that have symlinks in them (possibly related to
rust-lang/cargo#5664).

This commit introduces a `RUST_NIGHTLY_VERSION` file to explicitly
manage which version we're using in tests. For now, it also pins back
to the last known good nightly version for testing purposes.

Signed-off-by: Christopher Maier <[email protected]>
christophermaier added a commit to habitat-sh/habitat that referenced this issue Aug 6, 2019
Some of our tests use nightly Rust to help test for specific
things. The version of Rust used was simply piggybacking on the
version of nightly Rust we use for `rustfmt`, rather than being
specifically managed.

Previously, this wasn't an issue, but with the latest nightly Rust
that works for `rustfmt`, we apparently have a bug around cargo
dealing with crates that have symlinks in them (possibly related to
rust-lang/cargo#5664).

This commit introduces a `RUST_NIGHTLY_VERSION` file to explicitly
manage which version we're using in tests. For now, it also pins back
to the last known good nightly version for testing purposes.

Signed-off-by: Christopher Maier <[email protected]>
@Tastaturtaste
Copy link

Since I lost some hours today to this problem I want to document what solved this problem for me here.
Basically just follow the instruction in this stackoverflow answer. This will not fix symlinks that are already broken, though maybe another answer or comment there will be helpful.

I will outline the general steps here, too:

  1. Enable developer mode or give your account symlink permissions
  2. Enable symlinks in git
    • git config core.symlinks should return true in your repo
  3. Make sure your global settings are not overwritten by your local settings

@weihanglo weihanglo added A-git Area: anything dealing with git S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. labels Jun 29, 2023
@vallentin
Copy link

Using git ls-tree HEAD [file-name] for a specific file or just git ls-files -s for all files, then the mode (i.e. the first number) will be 120000 if a file is symlink

github-merge-queue bot pushed a commit that referenced this issue 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.

[^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

### 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
github-merge-queue bot pushed a commit that referenced this issue 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
Labels
A-git Area: anything dealing with git C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-package O-windows OS: Windows S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

6 participants