mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Avoid whole plugin cache replacement on Windows
This commit is contained in:
@@ -260,6 +260,10 @@ fn replace_plugin_root_atomically(
|
||||
target_root: &Path,
|
||||
plugin_version: &str,
|
||||
) -> Result<(), PluginStoreError> {
|
||||
if cfg!(windows) {
|
||||
return replace_plugin_root_on_windows(source, target_root, plugin_version);
|
||||
}
|
||||
|
||||
let Some(parent) = target_root.parent() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin cache path has no parent: {}",
|
||||
@@ -322,6 +326,133 @@ fn replace_plugin_root_atomically(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn replace_plugin_root_on_windows(
|
||||
source: &Path,
|
||||
target_root: &Path,
|
||||
plugin_version: &str,
|
||||
) -> Result<(), PluginStoreError> {
|
||||
let Some(parent) = target_root.parent() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin cache path has no parent: {}",
|
||||
target_root.display()
|
||||
)));
|
||||
};
|
||||
|
||||
fs::create_dir_all(parent)
|
||||
.map_err(|err| PluginStoreError::io("failed to create plugin cache directory", err))?;
|
||||
|
||||
let Some(plugin_dir_name) = target_root.file_name() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin cache path has no directory name: {}",
|
||||
target_root.display()
|
||||
)));
|
||||
};
|
||||
|
||||
let target_version_root = target_root.join(plugin_version);
|
||||
let staged_dir = tempfile::Builder::new()
|
||||
.prefix("plugin-install-")
|
||||
.tempdir_in(parent)
|
||||
.map_err(|err| {
|
||||
PluginStoreError::io("failed to create temporary plugin cache directory", err)
|
||||
})?;
|
||||
let staged_root = staged_dir.path().join(plugin_dir_name);
|
||||
let staged_version_root = staged_root.join(plugin_version);
|
||||
copy_dir_recursive(source, &staged_version_root)?;
|
||||
|
||||
if !target_root.exists() {
|
||||
fs::rename(&staged_root, target_root)
|
||||
.map_err(|err| PluginStoreError::io("failed to activate plugin cache entry", err))?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if !target_root.is_dir() {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin cache entry is not a directory: {}",
|
||||
target_root.display()
|
||||
)));
|
||||
}
|
||||
|
||||
replace_plugin_version_root_atomically_on_windows(&staged_version_root, &target_version_root)?;
|
||||
remove_stale_plugin_versions_best_effort(target_root, plugin_version);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn replace_plugin_version_root_atomically_on_windows(
|
||||
staged_version_root: &Path,
|
||||
target_version_root: &Path,
|
||||
) -> Result<(), PluginStoreError> {
|
||||
if !target_version_root.exists() {
|
||||
fs::rename(staged_version_root, target_version_root)
|
||||
.map_err(|err| PluginStoreError::io("failed to activate plugin cache version", err))?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(parent) = target_version_root.parent() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin cache version path has no parent: {}",
|
||||
target_version_root.display()
|
||||
)));
|
||||
};
|
||||
let backup_dir = tempfile::Builder::new()
|
||||
.prefix("plugin-version-backup-")
|
||||
.tempdir_in(parent)
|
||||
.map_err(|err| {
|
||||
PluginStoreError::io(
|
||||
"failed to create plugin cache version backup directory",
|
||||
err,
|
||||
)
|
||||
})?;
|
||||
let Some(version_dir_name) = target_version_root.file_name() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin cache version path has no directory name: {}",
|
||||
target_version_root.display()
|
||||
)));
|
||||
};
|
||||
let backup_version_root = backup_dir.path().join(version_dir_name);
|
||||
fs::rename(target_version_root, &backup_version_root)
|
||||
.map_err(|err| PluginStoreError::io("failed to back up plugin cache version", err))?;
|
||||
|
||||
if let Err(err) = fs::rename(staged_version_root, target_version_root) {
|
||||
let rollback_result = fs::rename(&backup_version_root, target_version_root);
|
||||
return match rollback_result {
|
||||
Ok(()) => Err(PluginStoreError::io(
|
||||
"failed to activate updated plugin cache version",
|
||||
err,
|
||||
)),
|
||||
Err(rollback_err) => {
|
||||
let backup_path = backup_dir.keep().join(version_dir_name);
|
||||
Err(PluginStoreError::Invalid(format!(
|
||||
"failed to activate updated plugin cache version at {}: {err}; failed to restore previous cache version (left at {}): {rollback_err}",
|
||||
target_version_root.display(),
|
||||
backup_path.display()
|
||||
)))
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_stale_plugin_versions_best_effort(target_root: &Path, active_version: &str) {
|
||||
let Ok(entries) = fs::read_dir(target_root) else {
|
||||
return;
|
||||
};
|
||||
|
||||
for entry in entries.filter_map(Result::ok) {
|
||||
let Ok(file_type) = entry.file_type() else {
|
||||
continue;
|
||||
};
|
||||
if !file_type.is_dir() {
|
||||
continue;
|
||||
}
|
||||
if entry.file_name() == std::ffi::OsStr::new(active_version) {
|
||||
continue;
|
||||
}
|
||||
let _ = fs::remove_dir_all(entry.path());
|
||||
}
|
||||
}
|
||||
|
||||
fn copy_dir_recursive(source: &Path, target: &Path) -> Result<(), PluginStoreError> {
|
||||
fs::create_dir_all(target)
|
||||
.map_err(|err| PluginStoreError::io("failed to create plugin target directory", err))?;
|
||||
|
||||
@@ -183,6 +183,75 @@ fn install_uses_manifest_version_when_present() {
|
||||
assert!(installed_path.join(".codex-plugin/plugin.json").is_file());
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_install_reuses_valid_existing_version_without_renaming_plugin_root() {
|
||||
let tmp = tempdir().unwrap();
|
||||
write_plugin_with_version(tmp.path(), "sample-plugin", "sample-plugin", Some("1.0.0"));
|
||||
let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
let store = PluginStore::new(tmp.path().to_path_buf());
|
||||
|
||||
let first_result = store
|
||||
.install(
|
||||
AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap(),
|
||||
plugin_id.clone(),
|
||||
)
|
||||
.unwrap();
|
||||
let plugin_base_root = tmp.path().join("plugins/cache/debug/sample-plugin");
|
||||
let previous_dir = std::env::current_dir().unwrap();
|
||||
std::env::set_current_dir(&plugin_base_root).unwrap();
|
||||
let second_result = store.install(
|
||||
AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap(),
|
||||
plugin_id,
|
||||
);
|
||||
std::env::set_current_dir(previous_dir).unwrap();
|
||||
let second_result = second_result.unwrap();
|
||||
|
||||
assert_eq!(second_result, first_result);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_install_adds_new_version_without_renaming_plugin_root() {
|
||||
let tmp = tempdir().unwrap();
|
||||
write_plugin_with_version(tmp.path(), "source-v1", "sample-plugin", Some("1.0.0"));
|
||||
write_plugin_with_version(tmp.path(), "source-v2", "sample-plugin", Some("2.0.0"));
|
||||
let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
let store = PluginStore::new(tmp.path().to_path_buf());
|
||||
|
||||
store
|
||||
.install(
|
||||
AbsolutePathBuf::try_from(tmp.path().join("source-v1")).unwrap(),
|
||||
plugin_id.clone(),
|
||||
)
|
||||
.unwrap();
|
||||
let plugin_base_root = tmp.path().join("plugins/cache/debug/sample-plugin");
|
||||
let previous_dir = std::env::current_dir().unwrap();
|
||||
std::env::set_current_dir(&plugin_base_root).unwrap();
|
||||
let result = store.install(
|
||||
AbsolutePathBuf::try_from(tmp.path().join("source-v2")).unwrap(),
|
||||
plugin_id.clone(),
|
||||
);
|
||||
std::env::set_current_dir(previous_dir).unwrap();
|
||||
let result = result.unwrap();
|
||||
|
||||
let installed_path = tmp.path().join("plugins/cache/debug/sample-plugin/2.0.0");
|
||||
assert_eq!(
|
||||
result,
|
||||
PluginInstallResult {
|
||||
plugin_id,
|
||||
plugin_version: "2.0.0".to_string(),
|
||||
installed_path: AbsolutePathBuf::try_from(installed_path.clone()).unwrap(),
|
||||
}
|
||||
);
|
||||
assert!(
|
||||
!tmp.path()
|
||||
.join("plugins/cache/debug/sample-plugin/1.0.0")
|
||||
.exists()
|
||||
);
|
||||
assert!(installed_path.join(".codex-plugin/plugin.json").is_file());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_rejects_blank_manifest_version() {
|
||||
let tmp = tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user