mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
Clean up failed and abandoned marketplace staging directories (#55689)
## Why Failed marketplace adds can leave temporary checkouts behind, and interrupted adds or upgrades can leave abandoned staging directories. ## What changed - Share a staging guard across marketplace adds and upgrades so temporary checkouts are removed on completion or failure, including config-write rollback. - Reclaim up to four abandoned directories per cleanup pass during add and upgrade operations. Use file locks to preserve active checkouts and operating-system-specific names to avoid reclaiming another platform's staging directories. - Skip symlinks and unrelated entries, and report cleanup failures as warnings. ## Testing Add regression coverage for failed adds, CLI and automatic-upgrade cleanup, cross-process ownership and abandonment, lock contention, bounded cleanup, and symlink handling. GitOrigin-RevId: 20999da0eab861dfb8025aad5e9fba2bcaf818b8
This commit is contained in:
committed by
copyberry
parent
36512a34ff
commit
69727ae170
@@ -1,4 +1,4 @@
|
||||
//! Path normalization, replacement, symlink resolution, and atomic writes.
|
||||
//! Path normalization, replacement, metadata inspection, symlink resolution, and atomic writes.
|
||||
|
||||
pub(crate) mod env;
|
||||
pub use env::is_wsl;
|
||||
@@ -15,6 +15,16 @@ use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::NamedTempFile;
|
||||
|
||||
/// Read metadata without following the final symlink, returning `None` for a missing path.
|
||||
/// Other I/O errors are returned to the caller.
|
||||
pub fn symlink_metadata_if_exists(path: &Path) -> io::Result<Option<std::fs::Metadata>> {
|
||||
match std::fs::symlink_metadata(path) {
|
||||
Ok(metadata) => Ok(Some(metadata)),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(None),
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn normalize_for_path_comparison(path: impl AsRef<Path>) -> std::io::Result<PathBuf> {
|
||||
let canonical = path.as_ref().canonicalize()?;
|
||||
Ok(normalize_for_wsl(canonical))
|
||||
|
||||
@@ -1,7 +1,33 @@
|
||||
use super::replace_path_and_deduplicate;
|
||||
use super::symlink_metadata_if_exists;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
|
||||
#[test]
|
||||
fn symlink_metadata_if_exists_distinguishes_files_directories_and_missing_paths()
|
||||
-> std::io::Result<()> {
|
||||
let directory = tempfile::tempdir()?;
|
||||
let file = directory.path().join("file");
|
||||
std::fs::write(&file, "contents")?;
|
||||
|
||||
let directory_metadata =
|
||||
symlink_metadata_if_exists(directory.path())?.expect("directory exists");
|
||||
let file_metadata = symlink_metadata_if_exists(&file)?.expect("file exists");
|
||||
assert_eq!(
|
||||
(directory_metadata.is_dir(), file_metadata.is_file()),
|
||||
(true, true)
|
||||
);
|
||||
assert!(symlink_metadata_if_exists(&directory.path().join("missing"))?.is_none());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn symlink_metadata_if_exists_preserves_io_errors() {
|
||||
let error = symlink_metadata_if_exists(Path::new("invalid\0path"))
|
||||
.expect_err("an invalid path must not be treated as missing");
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replace_path_and_deduplicate_preserves_other_paths_and_order() {
|
||||
let old_path = Path::new("old");
|
||||
@@ -22,9 +48,27 @@ fn replace_path_and_deduplicate_preserves_other_paths_and_order() {
|
||||
#[cfg(unix)]
|
||||
mod symlinks {
|
||||
use super::super::resolve_symlink_write_paths;
|
||||
use super::super::symlink_metadata_if_exists;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
#[test]
|
||||
fn symlink_metadata_if_exists_preserves_links_with_and_without_targets() -> std::io::Result<()>
|
||||
{
|
||||
let directory = tempfile::tempdir()?;
|
||||
let target = directory.path().join("target");
|
||||
let link = directory.path().join("link");
|
||||
std::fs::write(&target, "contents")?;
|
||||
symlink(&target, &link)?;
|
||||
|
||||
let linked_metadata = symlink_metadata_if_exists(&link)?.expect("symlink exists");
|
||||
std::fs::remove_file(target)?;
|
||||
let dangling_metadata = symlink_metadata_if_exists(&link)?.expect("symlink still exists");
|
||||
assert!(linked_metadata.is_symlink());
|
||||
assert_eq!(dangling_metadata.file_type(), linked_metadata.file_type());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn symlink_cycles_fall_back_to_root_write_path() -> std::io::Result<()> {
|
||||
let dir = tempfile::tempdir()?;
|
||||
|
||||
Reference in New Issue
Block a user