Skip to content

Commit 9191c77

Browse files
authored
Fix FileLock path tracking after rename in package operation (#16036)
### What does this PR try to resolve? Fixes #7863 - FileLock::rename bug where `cargo::ops::package()` returns a FileLock pointing to a non-existent temporary file. The issue occurs when using Cargo's programmatic API. The `cargo::ops::package()` function creates a temporary file, locks it with FileLock, then renames it to the final package location. However, it was using `std::fs::rename()` directly, which only moves the file on the filesystem but doesn't update the FileLock's internal path. This left the FileLock pointing to the old temporary path that no longer exists. **Root cause:** FileLock had no way to update its internal path after a filesystem rename operation. **Solution:** 1. Add a `FileLock::rename()` method that performs both the filesystem rename AND updates the internal path 2. Update the package operation to use this new method instead of `std::fs::rename()` ### How to test and review this PR? **Manual testing approach:** Since this bug only affects programmatic usage (not CLI), you can test it by creating a simple test that uses `cargo::ops::package()`: ```rust use std::env; use crate::prelude::*; use cargo::core::Workspace; use cargo::core::Shell; use cargo::util::GlobalContext; use cargo_test_support::{basic_manifest, paths, project}; #[cargo_test] fn filelock_path_after_rename_bug() { // This test shows the bug where cargo::ops::package returns a FileLock // pointing to a temporary file that no longer exists after the file is renamed. let p = project() .file("Cargo.toml", &basic_manifest("foo", "0.1.0")) .file("src/lib.rs", "") .build(); let shell = Shell::from_write(Box::new(Vec::new())); let gctx = GlobalContext::new(shell, env::current_dir().unwrap(), paths::home()); let ws = Workspace::new(&p.root().join("Cargo.toml"), &gctx).unwrap(); let package_opts = cargo::ops::PackageOpts { gctx: &gctx, list: false, fmt: cargo::ops::PackageMessageFormat::Human, check_metadata: false, allow_dirty: true, include_lockfile: false, verify: false, jobs: None, keep_going: false, to_package: cargo::ops::Packages::Default, targets: Vec::new(), cli_features: cargo::core::resolver::CliFeatures::new_all(false), reg_or_index: None, dry_run: false, }; let file_locks = cargo::ops::package(&ws, &package_opts).unwrap(); let file_lock = &file_locks[0]; // Before fix: this fails because FileLock points to deleted temp file // After fix: this succeeds because FileLock points to actual package assert!(file_lock.path().exists()); }
2 parents 17a4b39 + e3afb93 commit 9191c77

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

src/cargo/ops/cargo_package/mod.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::BTreeMap;
22
use std::collections::BTreeSet;
33
use std::collections::HashMap;
4-
use std::fs::{self, File};
4+
use std::fs::File;
55
use std::io::SeekFrom;
66
use std::io::prelude::*;
77
use std::path::{Path, PathBuf};
@@ -177,10 +177,8 @@ fn create_package(
177177
.context("failed to prepare local package for uploading")?;
178178

179179
dst.seek(SeekFrom::Start(0))?;
180-
let src_path = dst.path();
181180
let dst_path = dst.parent().join(&filename);
182-
fs::rename(&src_path, &dst_path)
183-
.context("failed to move temporary tarball into final location")?;
181+
dst.rename(&dst_path)?;
184182

185183
let dst_metadata = dst
186184
.file()

src/cargo/util/flock.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,29 @@ impl FileLock {
7676
}
7777
Ok(())
7878
}
79+
80+
/// Renames the file and updates the internal path.
81+
///
82+
/// This method performs a filesystem rename operation using [`std::fs::rename`]
83+
/// while keeping the FileLock's internal path synchronized with the actual
84+
/// file location.
85+
///
86+
/// ## Difference from `std::fs::rename`
87+
///
88+
/// - `std::fs::rename(old, new)` only moves the file on the filesystem
89+
/// - `FileLock::rename(new)` moves the file AND updates `self.path` to point to the new location
90+
pub fn rename<P: AsRef<Path>>(&mut self, new_path: P) -> CargoResult<()> {
91+
let new_path = new_path.as_ref();
92+
std::fs::rename(&self.path, new_path).with_context(|| {
93+
format!(
94+
"failed to rename {} to {}",
95+
self.path.display(),
96+
new_path.display()
97+
)
98+
})?;
99+
self.path = new_path.to_path_buf();
100+
Ok(())
101+
}
79102
}
80103

81104
impl Read for FileLock {

0 commit comments

Comments
 (0)