mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
runtime: use install context for bundled bwrap (#23634)
## Summary The Linux sandbox should find bundled `bwrap` through the same package-layout abstraction as the rest of the runtime, instead of maintaining a separate standalone-specific lookup path. This adds an `InstallContext` helper for bundled resources and updates `codex-linux-sandbox` to ask the current install context for `codex-resources/bwrap` before falling back to the old executable-relative probes. The tests cover npm-style, standalone, and canonical package layouts so `bwrap` lookup follows the package structure introduced earlier in the stack. ## Test plan - `cargo test -p codex-install-context` - `cargo test -p codex-linux-sandbox --lib` - `just fix -p codex-install-context -p codex-linux-sandbox` - `just bazel-lock-check` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23634). * #23638 * #23637 * #23636 * #23635 * __->__ #23634
This commit is contained in:
committed by
GitHub
Unverified
parent
a52c91d8b5
commit
b0b383bea3
Generated
+1
@@ -3024,6 +3024,7 @@ version = "0.0.0"
|
||||
dependencies = [
|
||||
"clap",
|
||||
"codex-core",
|
||||
"codex-install-context",
|
||||
"codex-process-hardening",
|
||||
"codex-protocol",
|
||||
"codex-sandboxing",
|
||||
|
||||
@@ -124,7 +124,7 @@ impl InstallContext {
|
||||
&& let Some(path_dir) = &package_layout.path_dir
|
||||
{
|
||||
let bundled_rg = path_dir.join(default_rg_command());
|
||||
if bundled_rg.exists() {
|
||||
if bundled_rg.is_file() {
|
||||
return bundled_rg.into_path_buf();
|
||||
}
|
||||
}
|
||||
@@ -135,13 +135,37 @@ impl InstallContext {
|
||||
} = &self.method
|
||||
{
|
||||
let bundled_rg = resources_dir.join(default_rg_command());
|
||||
if bundled_rg.exists() {
|
||||
if bundled_rg.is_file() {
|
||||
return bundled_rg.into_path_buf();
|
||||
}
|
||||
}
|
||||
|
||||
default_rg_command()
|
||||
}
|
||||
|
||||
pub fn bundled_resource(&self, file_name: impl AsRef<Path>) -> Option<AbsolutePathBuf> {
|
||||
if let Some(package_layout) = &self.package_layout
|
||||
&& let Some(resources_dir) = &package_layout.resources_dir
|
||||
{
|
||||
let resource = resources_dir.join(file_name.as_ref());
|
||||
if resource.is_file() {
|
||||
return Some(resource);
|
||||
}
|
||||
}
|
||||
|
||||
if let InstallMethod::Standalone {
|
||||
resources_dir: Some(resources_dir),
|
||||
..
|
||||
} = &self.method
|
||||
{
|
||||
let resource = resources_dir.join(file_name);
|
||||
if resource.is_file() {
|
||||
return Some(resource);
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
impl CodexPackageLayout {
|
||||
@@ -242,6 +266,8 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
|
||||
const TEST_RESOURCE_NAME: &str = "codex-test-helper";
|
||||
|
||||
#[test]
|
||||
fn detects_standalone_install_from_release_layout() -> std::io::Result<()> {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
@@ -253,6 +279,7 @@ mod tests {
|
||||
let exe_path = release_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
|
||||
fs::write(&exe_path, "")?;
|
||||
fs::write(resources_dir.join(default_rg_command()), "")?;
|
||||
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
|
||||
let canonical_release_dir =
|
||||
AbsolutePathBuf::from_absolute_path(release_dir.canonicalize()?)?;
|
||||
let canonical_resources_dir =
|
||||
@@ -270,12 +297,16 @@ mod tests {
|
||||
InstallContext {
|
||||
method: InstallMethod::Standalone {
|
||||
release_dir: canonical_release_dir,
|
||||
resources_dir: Some(canonical_resources_dir),
|
||||
resources_dir: Some(canonical_resources_dir.clone()),
|
||||
platform: standalone_platform(),
|
||||
},
|
||||
package_layout: None,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
context.bundled_resource(TEST_RESOURCE_NAME),
|
||||
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -312,6 +343,7 @@ mod tests {
|
||||
fs::write(package_dir.path().join(PACKAGE_METADATA_FILENAME), "{}")?;
|
||||
let exe_path = bin_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
|
||||
fs::write(&exe_path, "")?;
|
||||
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
|
||||
fs::write(path_dir.join(default_rg_command()), "")?;
|
||||
let canonical_package_dir =
|
||||
AbsolutePathBuf::from_absolute_path(package_dir.path().canonicalize()?)?;
|
||||
@@ -322,7 +354,7 @@ mod tests {
|
||||
let package_layout = CodexPackageLayout {
|
||||
package_dir: canonical_package_dir,
|
||||
bin_dir: canonical_bin_dir,
|
||||
resources_dir: Some(canonical_resources_dir),
|
||||
resources_dir: Some(canonical_resources_dir.clone()),
|
||||
path_dir: Some(canonical_path_dir.clone()),
|
||||
};
|
||||
|
||||
@@ -346,6 +378,10 @@ mod tests {
|
||||
.join(default_rg_command())
|
||||
.into_path_buf()
|
||||
);
|
||||
assert_eq!(
|
||||
context.bundled_resource(TEST_RESOURCE_NAME),
|
||||
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -364,6 +400,7 @@ mod tests {
|
||||
fs::write(package_dir.join(PACKAGE_METADATA_FILENAME), "{}")?;
|
||||
let exe_path = bin_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
|
||||
fs::write(&exe_path, "")?;
|
||||
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
|
||||
fs::write(path_dir.join(default_rg_command()), "")?;
|
||||
let canonical_package_dir =
|
||||
AbsolutePathBuf::from_absolute_path(package_dir.canonicalize()?)?;
|
||||
@@ -390,7 +427,7 @@ mod tests {
|
||||
package_layout: Some(CodexPackageLayout {
|
||||
package_dir: canonical_package_dir,
|
||||
bin_dir: canonical_bin_dir,
|
||||
resources_dir: Some(canonical_resources_dir),
|
||||
resources_dir: Some(canonical_resources_dir.clone()),
|
||||
path_dir: Some(canonical_path_dir.clone()),
|
||||
}),
|
||||
}
|
||||
@@ -401,6 +438,10 @@ mod tests {
|
||||
.join(default_rg_command())
|
||||
.into_path_buf()
|
||||
);
|
||||
assert_eq!(
|
||||
context.bundled_resource(TEST_RESOURCE_NAME),
|
||||
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -455,6 +496,31 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bundled_file_lookups_ignore_directories() -> std::io::Result<()> {
|
||||
let package_dir = tempfile::tempdir()?;
|
||||
let bin_dir = package_dir.path().join(BIN_DIRNAME);
|
||||
let resources_dir = package_dir.path().join(RESOURCES_DIRNAME);
|
||||
let path_dir = package_dir.path().join(PATH_DIRNAME);
|
||||
fs::create_dir_all(&bin_dir)?;
|
||||
fs::create_dir_all(resources_dir.join(TEST_RESOURCE_NAME))?;
|
||||
fs::create_dir_all(path_dir.join(default_rg_command()))?;
|
||||
fs::write(package_dir.path().join(PACKAGE_METADATA_FILENAME), "{}")?;
|
||||
let exe_path = bin_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
|
||||
fs::write(&exe_path, "")?;
|
||||
|
||||
let context = InstallContext::from_exe_with_codex_home(
|
||||
/*is_macos*/ false,
|
||||
/*current_exe*/ Some(&exe_path),
|
||||
/*managed_by_npm*/ false,
|
||||
/*managed_by_bun*/ false,
|
||||
/*codex_home*/ None,
|
||||
);
|
||||
assert_eq!(context.rg_command(), default_rg_command());
|
||||
assert_eq!(context.bundled_resource(TEST_RESOURCE_NAME), None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn npm_and_bun_take_precedence() {
|
||||
let npm_context = InstallContext::from_exe_with_codex_home(
|
||||
|
||||
@@ -18,6 +18,7 @@ workspace = true
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dependencies]
|
||||
clap = { workspace = true, features = ["derive"] }
|
||||
codex-install-context = { workspace = true }
|
||||
codex-process-hardening = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-sandboxing = { workspace = true }
|
||||
|
||||
@@ -12,6 +12,7 @@ use std::sync::OnceLock;
|
||||
use crate::bazel_bwrap;
|
||||
use crate::exec_util::argv_to_cstrings;
|
||||
use crate::exec_util::make_files_inheritable;
|
||||
use codex_install_context::InstallContext;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use sha2::Digest as _;
|
||||
use sha2::Sha256;
|
||||
@@ -26,7 +27,9 @@ pub(crate) struct BundledBwrapLauncher {
|
||||
|
||||
pub(crate) fn launcher() -> Option<BundledBwrapLauncher> {
|
||||
let current_exe = std::env::current_exe().ok()?;
|
||||
find_for_exe(¤t_exe).map(|program| BundledBwrapLauncher { program })
|
||||
find_for_install_context(InstallContext::current())
|
||||
.or_else(|| find_legacy_for_exe(¤t_exe))
|
||||
.map(|program| BundledBwrapLauncher { program })
|
||||
}
|
||||
|
||||
impl BundledBwrapLauncher {
|
||||
@@ -66,8 +69,14 @@ impl BundledBwrapLauncher {
|
||||
}
|
||||
}
|
||||
|
||||
fn find_for_exe(exe: &Path) -> Option<AbsolutePathBuf> {
|
||||
candidates_for_exe(exe)
|
||||
fn find_for_install_context(context: &InstallContext) -> Option<AbsolutePathBuf> {
|
||||
context
|
||||
.bundled_resource("bwrap")
|
||||
.filter(|path| is_executable_file(path))
|
||||
}
|
||||
|
||||
fn find_legacy_for_exe(exe: &Path) -> Option<AbsolutePathBuf> {
|
||||
legacy_candidates_for_exe(exe)
|
||||
.into_iter()
|
||||
.find(|candidate| is_executable_file(candidate))
|
||||
.map(|path| {
|
||||
@@ -80,7 +89,7 @@ fn find_for_exe(exe: &Path) -> Option<AbsolutePathBuf> {
|
||||
})
|
||||
}
|
||||
|
||||
fn candidates_for_exe(exe: &Path) -> Vec<PathBuf> {
|
||||
fn legacy_candidates_for_exe(exe: &Path) -> Vec<PathBuf> {
|
||||
let Some(exe_dir) = exe.parent() else {
|
||||
return Vec::new();
|
||||
};
|
||||
@@ -180,13 +189,44 @@ fn bytes_to_hex(bytes: &[u8; 32]) -> String {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_install_context::CodexPackageLayout;
|
||||
use codex_install_context::InstallContext;
|
||||
use codex_install_context::InstallMethod;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use tempfile::NamedTempFile;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn finds_standalone_bundled_bwrap_next_to_exe_resources() {
|
||||
fn finds_package_layout_bwrap_from_install_context() {
|
||||
let temp_dir = tempdir().expect("temp dir");
|
||||
let package_dir = temp_dir.path();
|
||||
let bin_dir = package_dir.join("bin");
|
||||
let resources_dir = package_dir.join("codex-resources");
|
||||
let expected_bwrap = resources_dir.join("bwrap");
|
||||
fs::create_dir_all(&bin_dir).expect("create bin dir");
|
||||
write_executable(&expected_bwrap);
|
||||
|
||||
let context = InstallContext {
|
||||
method: InstallMethod::Other,
|
||||
package_layout: Some(CodexPackageLayout {
|
||||
package_dir: AbsolutePathBuf::from_absolute_path(package_dir).expect("absolute"),
|
||||
bin_dir: AbsolutePathBuf::from_absolute_path(&bin_dir).expect("absolute"),
|
||||
resources_dir: Some(
|
||||
AbsolutePathBuf::from_absolute_path(&resources_dir).expect("absolute"),
|
||||
),
|
||||
path_dir: None,
|
||||
}),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
find_for_install_context(&context),
|
||||
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn finds_legacy_standalone_bundled_bwrap_next_to_exe_resources() {
|
||||
let temp_dir = tempdir().expect("temp dir");
|
||||
let exe = temp_dir.path().join("codex");
|
||||
let expected_bwrap = temp_dir.path().join("codex-resources").join("bwrap");
|
||||
@@ -194,7 +234,7 @@ mod tests {
|
||||
write_executable(&expected_bwrap);
|
||||
|
||||
assert_eq!(
|
||||
find_for_exe(&exe),
|
||||
find_legacy_for_exe(&exe),
|
||||
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
|
||||
);
|
||||
}
|
||||
@@ -209,7 +249,7 @@ mod tests {
|
||||
write_executable(&expected_bwrap);
|
||||
|
||||
assert_eq!(
|
||||
find_for_exe(&exe),
|
||||
find_legacy_for_exe(&exe),
|
||||
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
|
||||
);
|
||||
}
|
||||
@@ -223,7 +263,7 @@ mod tests {
|
||||
write_executable(&expected_bwrap);
|
||||
|
||||
assert_eq!(
|
||||
find_for_exe(&exe),
|
||||
find_legacy_for_exe(&exe),
|
||||
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user