mirror of
https://github.com/rust-lang/cargo.git
synced 2025-09-28 11:20:36 +00:00
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.
This commit is contained in:
commit
d7940042bd
@ -190,9 +190,20 @@ pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()>
|
||||
/// Writes a file to disk atomically.
|
||||
///
|
||||
/// This uses `tempfile::persist` to accomplish atomic writes.
|
||||
/// If the path is a symlink, it will follow the symlink and write to the actual target.
|
||||
pub fn write_atomic<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()> {
|
||||
let path = path.as_ref();
|
||||
|
||||
// Check if the path is a symlink and follow it if it is
|
||||
let resolved_path;
|
||||
let path = if path.is_symlink() {
|
||||
resolved_path = fs::read_link(path)
|
||||
.with_context(|| format!("failed to read symlink at `{}`", path.display()))?;
|
||||
&resolved_path
|
||||
} else {
|
||||
path
|
||||
};
|
||||
|
||||
// On unix platforms, get the permissions of the original file. Copy only the user/group/other
|
||||
// read/write/execute permission bits. The tempfile lib defaults to an initial mode of 0o600,
|
||||
// and we'll set the proper permissions after creating the file.
|
||||
@ -981,6 +992,33 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_atomic_symlink() {
|
||||
let tmpdir = tempfile::tempdir().unwrap();
|
||||
let target_path = tmpdir.path().join("target.txt");
|
||||
let symlink_path = tmpdir.path().join("symlink.txt");
|
||||
|
||||
// Create initial file
|
||||
write(&target_path, "initial").unwrap();
|
||||
|
||||
// Create symlink
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::symlink(&target_path, &symlink_path).unwrap();
|
||||
#[cfg(windows)]
|
||||
std::os::windows::fs::symlink_file(&target_path, &symlink_path).unwrap();
|
||||
|
||||
// Write through symlink
|
||||
write_atomic(&symlink_path, "updated").unwrap();
|
||||
|
||||
// Verify both paths show the updated content
|
||||
assert_eq!(std::fs::read_to_string(&target_path).unwrap(), "updated");
|
||||
assert_eq!(std::fs::read_to_string(&symlink_path).unwrap(), "updated");
|
||||
|
||||
// Verify symlink still exists and points to the same target
|
||||
assert!(symlink_path.is_symlink());
|
||||
assert_eq!(std::fs::read_link(&symlink_path).unwrap(), target_path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(windows)]
|
||||
fn test_remove_symlink_dir() {
|
||||
|
@ -150,6 +150,7 @@ mod script_bare;
|
||||
mod script_frontmatter;
|
||||
mod script_shebang;
|
||||
mod sorted_table_with_dotted_item;
|
||||
mod symlink;
|
||||
mod target;
|
||||
mod target_cfg;
|
||||
mod unknown_inherited_feature;
|
||||
|
54
tests/testsuite/cargo_add/symlink.rs
Normal file
54
tests/testsuite/cargo_add/symlink.rs
Normal file
@ -0,0 +1,54 @@
|
||||
use cargo_test_support::prelude::*;
|
||||
use cargo_test_support::project;
|
||||
use cargo_test_support::registry;
|
||||
use std::fs;
|
||||
|
||||
#[cargo_test]
|
||||
fn symlink_case() {
|
||||
if !cargo_test_support::symlink_supported() {
|
||||
return;
|
||||
}
|
||||
|
||||
registry::init();
|
||||
registry::Package::new("test-dep", "1.0.0").publish();
|
||||
|
||||
let project = project().file("src/lib.rs", "").build();
|
||||
|
||||
let target_dir = project.root().join("target_dir");
|
||||
fs::create_dir_all(&target_dir).unwrap();
|
||||
|
||||
fs::copy(
|
||||
project.root().join("Cargo.toml"),
|
||||
target_dir.join("Cargo.toml"),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
fs::remove_file(project.root().join("Cargo.toml")).unwrap();
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::symlink;
|
||||
symlink(
|
||||
target_dir.join("Cargo.toml"),
|
||||
project.root().join("Cargo.toml"),
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use std::os::windows::fs::symlink_file;
|
||||
symlink_file(
|
||||
target_dir.join("Cargo.toml"),
|
||||
project.root().join("Cargo.toml"),
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
project.cargo("add test-dep").run();
|
||||
|
||||
assert!(project.root().join("Cargo.toml").is_symlink());
|
||||
|
||||
let target_content = fs::read_to_string(target_dir.join("Cargo.toml")).unwrap();
|
||||
assert!(target_content.contains("test-dep"));
|
||||
}
|
Loading…
x
Reference in New Issue
Block a user