Skip to content

cargo add overwrites a symlink Cargo.toml #15241

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

Closed
Axlefublr opened this issue Feb 27, 2025 · 6 comments · Fixed by #15281
Closed

cargo add overwrites a symlink Cargo.toml #15241

Axlefublr opened this issue Feb 27, 2025 · 6 comments · Fixed by #15281
Labels
C-bug Category: bug Command-add E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@Axlefublr
Copy link

Axlefublr commented Feb 27, 2025

Problem

I'll start with a codeblock as imo it's the most straightforward explanation:

$ ls -l
drwxr-xr-x src
lrwxrwxrwx target -> /home/axlefublr/.cache/wks/hello-world.rs/target
lrwxrwxrwx Cargo.lock -> /home/axlefublr/.cache/wks/hello-world.rs/Cargo.lock
lrwxrwxrwx Cargo.toml -> /home/axlefublr/.cache/wks/hello-world.rs/Cargo.toml

$ cargo --version
cargo 1.87.0-nightly (1d1d646c0 2025-02-21)

$ rustup run stable cargo --version
cargo 1.85.0 (d73d2caf9 2024-12-31)

$ rustup run stable cargo add clap
    Updating crates.io index
      Adding clap v4.5.31 to dependencies
			// --SNIP--

$ ls -l
drwxr-xr-x src
lrwxrwxrwx target -> /home/axlefublr/.cache/wks/hello-world.rs/target
lrwxrwxrwx Cargo.lock -> /home/axlefublr/.cache/wks/hello-world.rs/Cargo.lock
.rw-r--r-- Cargo.toml

$ rm -fr Cargo.toml

$ ln -sf ~/.cache/wks/hello-world.rs/Cargo.toml ./Cargo.toml

$ cargo add clap
    Updating crates.io index
      Adding clap v4.5.31 to dependencies
			// --SNIP--

$ ls -l
drwxr-xr-x src
lrwxrwxrwx target -> /home/axlefublr/.cache/wks/hello-world.rs/target
lrwxrwxrwx Cargo.lock -> /home/axlefublr/.cache/wks/hello-world.rs/Cargo.lock
.rw-r--r-- Cargo.toml

I have Cargo.toml symlinked to the real Cargo.toml somewhere else. When using cargo add, I expect the symlink to not be broken. However, cargo add overrides the symlink into being a normal file, breaking the symlink tie to that other Cargo.toml.
I don't think that's intentional.

Steps

mkdir -p test/src
cd test
touch src/main.rs
ln -sf ~/.cache/wks/hello-world.rs/Cargo.toml ./Cargo.toml # path to some other Cargo.toml goes here, instead of the first path
cargo add clap # doesn't matter which dependency you add

viola. stat the Cargo.toml to see that it's now a regular file, rather than a symlink.
the target Cargo.toml stays untouched, btw

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.85.0 (d73d2caf9 2024-12-31)
release: 1.85.0
commit-hash: d73d2caf9e41a39daf2a8d6ce60ec80bf354d2a7
commit-date: 2024-12-31
host: x86_64-unknown-linux-gnu
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: EndeavourOS Rolling Release (rolling) [64-bit]
@Axlefublr Axlefublr added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Feb 27, 2025
@Axlefublr
Copy link
Author

Axlefublr commented Feb 27, 2025

for those curious to know why I'm using a Cargo.toml symlink in the first place, read my blog post on that (I didn't make this issue for advertising; I'm just answering a very likely question in advance)

@epage
Copy link
Contributor

epage commented Feb 27, 2025

With #11386 we switched to atomic writes in #12744. We had permission issues with that in #13896 which we then fixed in #13898.

Maybe we should follow the symlinks and do the atomic writes in that location.

@epage
Copy link
Contributor

epage commented Feb 27, 2025

btw are you aware of native support for cargo script? I posted an update today on it at https://blog.rust-lang.org/inside-rust/2025/02/27/this-development-cycle-in-cargo-1.86.html#cargo-script

I would be interested in performance feedback. While we likely won't be doing any cargo-script-specific layers of caching, I'm hopefully we can get cargo-script itself to be "fast enough".

@Axlefublr
Copy link
Author

@epage I'm glad that cargo-script is being worked on on the core side, but it's probably not going to plug all of my holes as well as my current system does

rust-analyzer support I heard is coming, which is great news!
but many of the rust scripts I write are more so unixy programs than traditional scripts. things like "print a range of lines of a file", "append a line onto a file correctly even if it has a missing final newline", "print the first n lines in a file and remove them from that file" — all of those things are practically missing gnu programs, and core functionality that becomes a part of my system I want to be indesputably fast
if I have that need for some of my scripts, I might as well use my system for all of my scripts, rather than jumping in and out of cargo-script

I avoid writing fish scripts and instead write fish functions specifically because of the startup cost; I can't see cargo-script not having that cost, unfortunately

@Axlefublr
Copy link
Author

better way to phrase things: I have a rust binary generating system, not a rust scripting system. so I think I'm just not the audience for cargo-script, lol

RaghavenderSingh added a commit to RaghavenderSingh/cargo that referenced this issue Mar 9, 2025
When Cargo.toml is a symlink, cargo add was overwriting it with a regular
file. This change follows the symlink and writes to the target file instead,
preserving the symlink structure.

Fixes rust-lang#15241
RaghavenderSingh added a commit to RaghavenderSingh/cargo that referenced this issue Mar 9, 2025
When Cargo.toml is a symlink, cargo add was overwriting it with a regular
file. This change follows the symlink and writes to the target file instead,
preserving the symlink structure.

Fixes rust-lang#15241
RaghavenderSingh added a commit to RaghavenderSingh/cargo that referenced this issue Mar 9, 2025
When Cargo.toml is a symlink, cargo add was overwriting it with a regular
file. This change follows the symlink and writes to the target file instead,
preserving the symlink structure.

Fixes rust-lang#15241
RaghavenderSingh added a commit to RaghavenderSingh/cargo that referenced this issue Mar 9, 2025
When Cargo.toml is a symlink, cargo add was overwriting it with a regular
file. This change follows the symlink and writes to the target file instead,
preserving the symlink structure.

Fixes rust-lang#15241
@weihanglo weihanglo added E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-triage Status: This issue is waiting on initial triage. labels May 29, 2025
RaghavenderSingh added a commit to RaghavenderSingh/cargo that referenced this issue May 29, 2025
When cargo add is used on a project where Cargo.toml is a symlink,
it now follows the symlink and writes to the target file instead of
replacing the symlink with a regular file.

This is achieved by updating write_atomic() to detect symlinks and
resolve them before writing.

Fixes rust-lang#15241
RaghavenderSingh added a commit to RaghavenderSingh/cargo that referenced this issue Jun 2, 2025
- Preserve symlinks when writing files atomically in write_atomic()
- Update test to verify correct symlink preservation behavior
- Apply rustfmt formatting

This fixes the issue where cargo add would replace symlinked Cargo.toml
files with regular files, breaking the symlink to the original target.

Fixes rust-lang#15241
github-merge-queue bot pushed a commit that referenced this issue Jun 2, 2025
### What does this PR try to resolve?

This PR fixes a bug where `cargo add` breaks symlinks to Cargo.toml
files. Currently, when Cargo.toml is a symlink and `cargo add` is used
to add a dependency, the symlink is replaced with a regular file,
breaking the link to the original target file.

This issue was reported in #15241 where a user who relies on symlinked
Cargo.toml files found that `cargo add` breaks their workflow.

Fixes #15241

### How should we test and review this PR?

I've modified `LocalManifest::write()` to check if the path is a
symlink, and if so, follow it to get the actual target path. This
ensures we write to the actual file rather than replacing the symlink.

I've also added a test in `tests/testsuite/cargo_add/symlink.rs` that:
1. Creates a symlinked Cargo.toml file
2. Runs `cargo add` to add a dependency
3. Verifies the symlink is preserved and the dependency is added to the
target file

I've manually tested this fix and confirmed it works correctly.
@Axlefublr
Copy link
Author

yaaaay! 💃💃💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-add E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants