Skip to content

Commit d794004

Browse files
authored
Fix cargo add overwriting symlinked Cargo.toml files (#15281)
### 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.
2 parents 21a67e2 + ecfe3a9 commit d794004

File tree

3 files changed

+93
-0
lines changed

3 files changed

+93
-0
lines changed

crates/cargo-util/src/paths.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,20 @@ pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()>
190190
/// Writes a file to disk atomically.
191191
///
192192
/// This uses `tempfile::persist` to accomplish atomic writes.
193+
/// If the path is a symlink, it will follow the symlink and write to the actual target.
193194
pub fn write_atomic<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()> {
194195
let path = path.as_ref();
195196

197+
// Check if the path is a symlink and follow it if it is
198+
let resolved_path;
199+
let path = if path.is_symlink() {
200+
resolved_path = fs::read_link(path)
201+
.with_context(|| format!("failed to read symlink at `{}`", path.display()))?;
202+
&resolved_path
203+
} else {
204+
path
205+
};
206+
196207
// On unix platforms, get the permissions of the original file. Copy only the user/group/other
197208
// read/write/execute permission bits. The tempfile lib defaults to an initial mode of 0o600,
198209
// and we'll set the proper permissions after creating the file.
@@ -981,6 +992,33 @@ mod tests {
981992
}
982993
}
983994

995+
#[test]
996+
fn write_atomic_symlink() {
997+
let tmpdir = tempfile::tempdir().unwrap();
998+
let target_path = tmpdir.path().join("target.txt");
999+
let symlink_path = tmpdir.path().join("symlink.txt");
1000+
1001+
// Create initial file
1002+
write(&target_path, "initial").unwrap();
1003+
1004+
// Create symlink
1005+
#[cfg(unix)]
1006+
std::os::unix::fs::symlink(&target_path, &symlink_path).unwrap();
1007+
#[cfg(windows)]
1008+
std::os::windows::fs::symlink_file(&target_path, &symlink_path).unwrap();
1009+
1010+
// Write through symlink
1011+
write_atomic(&symlink_path, "updated").unwrap();
1012+
1013+
// Verify both paths show the updated content
1014+
assert_eq!(std::fs::read_to_string(&target_path).unwrap(), "updated");
1015+
assert_eq!(std::fs::read_to_string(&symlink_path).unwrap(), "updated");
1016+
1017+
// Verify symlink still exists and points to the same target
1018+
assert!(symlink_path.is_symlink());
1019+
assert_eq!(std::fs::read_link(&symlink_path).unwrap(), target_path);
1020+
}
1021+
9841022
#[test]
9851023
#[cfg(windows)]
9861024
fn test_remove_symlink_dir() {

tests/testsuite/cargo_add/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ mod script_bare;
150150
mod script_frontmatter;
151151
mod script_shebang;
152152
mod sorted_table_with_dotted_item;
153+
mod symlink;
153154
mod target;
154155
mod target_cfg;
155156
mod unknown_inherited_feature;

tests/testsuite/cargo_add/symlink.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
use cargo_test_support::prelude::*;
2+
use cargo_test_support::project;
3+
use cargo_test_support::registry;
4+
use std::fs;
5+
6+
#[cargo_test]
7+
fn symlink_case() {
8+
if !cargo_test_support::symlink_supported() {
9+
return;
10+
}
11+
12+
registry::init();
13+
registry::Package::new("test-dep", "1.0.0").publish();
14+
15+
let project = project().file("src/lib.rs", "").build();
16+
17+
let target_dir = project.root().join("target_dir");
18+
fs::create_dir_all(&target_dir).unwrap();
19+
20+
fs::copy(
21+
project.root().join("Cargo.toml"),
22+
target_dir.join("Cargo.toml"),
23+
)
24+
.unwrap();
25+
26+
fs::remove_file(project.root().join("Cargo.toml")).unwrap();
27+
28+
#[cfg(unix)]
29+
{
30+
use std::os::unix::fs::symlink;
31+
symlink(
32+
target_dir.join("Cargo.toml"),
33+
project.root().join("Cargo.toml"),
34+
)
35+
.unwrap();
36+
}
37+
38+
#[cfg(windows)]
39+
{
40+
use std::os::windows::fs::symlink_file;
41+
symlink_file(
42+
target_dir.join("Cargo.toml"),
43+
project.root().join("Cargo.toml"),
44+
)
45+
.unwrap();
46+
}
47+
48+
project.cargo("add test-dep").run();
49+
50+
assert!(project.root().join("Cargo.toml").is_symlink());
51+
52+
let target_content = fs::read_to_string(target_dir.join("Cargo.toml")).unwrap();
53+
assert!(target_content.contains("test-dep"));
54+
}

0 commit comments

Comments
 (0)