mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Isolate executable fixture copies in CLI tests on Linux (#48213)
## Why Sibling tests can spawn children while an executable fixture is open for writing, allowing inherited writable descriptors to cause `ETXTBSY` when launching the fixture. ## What changed Add a shared `copy_executable` helper for the daemon, doctor path safety, and exec-server tests. On Linux, run `/bin/cp` in a separate process and wait for it to exit before launching the fixture, then restore the source permissions to account for restrictive umasks. Keep `std::fs::copy` on other platforms. GitOrigin-RevId: 88f36826bbc6ab4e16e28ff02d5e7d6bd8dbf793
This commit is contained in:
@@ -1,5 +1,8 @@
|
||||
#![cfg(unix)]
|
||||
|
||||
#[path = "support/executable.rs"]
|
||||
mod executable;
|
||||
|
||||
use std::path::PathBuf;
|
||||
use std::process::Child;
|
||||
use std::process::Command;
|
||||
@@ -10,6 +13,7 @@ use std::time::Instant;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::ensure;
|
||||
use executable::copy_executable;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use tempfile::TempDir;
|
||||
@@ -40,7 +44,7 @@ impl TestDaemon {
|
||||
// Preserve the installed path without invalidating the shared CLI's Rosetta cache.
|
||||
#[cfg(all(target_os = "macos", target_arch = "x86_64"))]
|
||||
{
|
||||
std::fs::copy(&codex_source, &managed)?;
|
||||
copy_executable(&codex_source, &managed)?;
|
||||
// Translate the fixture before timed daemon capability and readiness checks.
|
||||
ensure!(
|
||||
Command::new(&managed)
|
||||
@@ -53,8 +57,9 @@ impl TestDaemon {
|
||||
);
|
||||
}
|
||||
#[cfg(not(all(target_os = "macos", target_arch = "x86_64")))]
|
||||
std::fs::hard_link(&codex_source, &managed)
|
||||
.or_else(|_| std::fs::copy(&codex_source, managed).map(|_| ()))?;
|
||||
if std::fs::hard_link(&codex_source, &managed).is_err() {
|
||||
copy_executable(&codex_source, &managed)?;
|
||||
}
|
||||
std::fs::write(standalone.join("auto-update-version"), &release_name)?;
|
||||
std::os::unix::fs::symlink(
|
||||
PathBuf::from("releases").join(release_name),
|
||||
@@ -499,7 +504,7 @@ fn packaged_daemon_launch(action: &str, initial: InitialDaemon) -> Result<()> {
|
||||
for directory in ["bin", "codex-path", "codex-resources"] {
|
||||
std::fs::create_dir_all(package.join(directory))?;
|
||||
}
|
||||
std::fs::copy(&daemon.codex, package.join("bin/codex"))?;
|
||||
copy_executable(&daemon.codex, &package.join("bin/codex"))?;
|
||||
daemon.codex = package.join("bin/codex");
|
||||
for helper in [
|
||||
"bin/codex-code-mode-host",
|
||||
@@ -540,22 +545,11 @@ fn packaged_daemon_launch(action: &str, initial: InitialDaemon) -> Result<()> {
|
||||
br#"{"shutdownGraceSeconds":0}"#,
|
||||
)?;
|
||||
let cli_before = daemon.codex.canonicalize()?;
|
||||
let mut command = daemon.command();
|
||||
command.args(["app-server", "daemon", action]);
|
||||
// A freshly copied executable can briefly remain busy on Linux CI workers.
|
||||
let mut retries = 0;
|
||||
let result = loop {
|
||||
let result = command.output();
|
||||
if !result
|
||||
.as_ref()
|
||||
.is_err_and(|error| error.kind() == std::io::ErrorKind::ExecutableFileBusy)
|
||||
|| retries == 2
|
||||
{
|
||||
break result?;
|
||||
}
|
||||
retries += 1;
|
||||
std::thread::sleep(Duration::from_millis(/*millis*/ 10));
|
||||
};
|
||||
let result = daemon
|
||||
.command()
|
||||
.args(["app-server", "daemon", action])
|
||||
.output()
|
||||
.context("failed to launch packaged daemon fixture")?;
|
||||
ensure!(
|
||||
result.status.success(),
|
||||
"{}",
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
//! Black-box coverage for safe diagnostic execution and config error reporting.
|
||||
|
||||
#[path = "support/executable.rs"]
|
||||
mod executable;
|
||||
|
||||
use std::ffi::OsString;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
@@ -12,6 +15,7 @@ use codex_app_server_protocol::RequestId;
|
||||
use codex_config::loader::project_trust_key;
|
||||
use codex_state::SqliteConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use executable::copy_executable;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -45,7 +49,7 @@ impl Fixture {
|
||||
// Hard-link setup and teardown invalidate other tests' Rosetta translations.
|
||||
#[cfg(all(target_os = "macos", target_arch = "x86_64"))]
|
||||
{
|
||||
std::fs::copy(&source, &program)?;
|
||||
copy_executable(&source, &program)?;
|
||||
// Translate the fixture before the timed diagnostic command.
|
||||
anyhow::ensure!(
|
||||
std::process::Command::new(&program)
|
||||
@@ -59,7 +63,7 @@ impl Fixture {
|
||||
}
|
||||
#[cfg(not(all(target_os = "macos", target_arch = "x86_64")))]
|
||||
if std::fs::hard_link(&source, &program).is_err() {
|
||||
std::fs::copy(&source, &program)?;
|
||||
copy_executable(&source, &program)?;
|
||||
}
|
||||
let workspace = root.path().join("workspace");
|
||||
let bin = workspace.join("node_modules/.bin");
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
#[path = "support/executable.rs"]
|
||||
mod executable;
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[path = "exec_server/pid_namespace_tests.rs"]
|
||||
mod pid_namespace_tests;
|
||||
@@ -33,6 +36,7 @@ use codex_exec_server::NoiseRendezvousConnectBundle;
|
||||
use codex_exec_server::ProcessId;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use executable::copy_executable;
|
||||
use futures::SinkExt;
|
||||
use futures::StreamExt;
|
||||
use predicates::prelude::PredicateBooleanExt;
|
||||
@@ -185,7 +189,7 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr
|
||||
let bin_dir = package.path().join("bin");
|
||||
std::fs::create_dir(&bin_dir)?;
|
||||
let executable = bin_dir.join(format!("codex{}", std::env::consts::EXE_SUFFIX));
|
||||
std::fs::copy(codex_utils_cargo_bin::cargo_bin("codex")?, &executable)?;
|
||||
copy_executable(&codex_utils_cargo_bin::cargo_bin("codex")?, &executable)?;
|
||||
let manifest = package.path().join("codex-package.json");
|
||||
std::fs::write(&manifest, r#"{"version":"1.2.3-alpha.4"}"#)?;
|
||||
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
//! Copies executable fixtures without exposing writable descriptors to sibling test spawns.
|
||||
//! Linux copies finish in a separate process before the fixture can be launched.
|
||||
|
||||
use std::path::Path;
|
||||
#[cfg(target_os = "linux")]
|
||||
use std::process::Command;
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
#[cfg(target_os = "linux")]
|
||||
use anyhow::ensure;
|
||||
|
||||
pub(super) fn copy_executable(source: &Path, destination: &Path) -> Result<()> {
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
// Sibling tests can spawn children while a copy holds the executable open
|
||||
// for writing. Keep that descriptor in a separate process and wait for it
|
||||
// to exit, so inherited descriptors cannot cause ETXTBSY during launch.
|
||||
let output = Command::new("/bin/cp")
|
||||
.arg("--")
|
||||
.arg(source)
|
||||
.arg(destination)
|
||||
.output()
|
||||
.context("failed to start fixture executable copy")?;
|
||||
ensure!(
|
||||
output.status.success(),
|
||||
"failed to copy fixture executable {} to {}: {}",
|
||||
source.display(),
|
||||
destination.display(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
// Match fs::copy even when the test process has a restrictive umask.
|
||||
std::fs::set_permissions(destination, std::fs::metadata(source)?.permissions())?;
|
||||
}
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
std::fs::copy(source, destination)?;
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user