mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Remove deterministic_process_ids feature to avoid duplicate codex-core builds (#11393)
## Why `codex-core` enabled `deterministic_process_ids` through a self dev-dependency. That forced a second feature-resolved build of the same crate, which increased compile time and test latency. ## What Changed - Removed the `deterministic_process_ids` feature from `codex-rs/core/Cargo.toml`. - Removed the self dev-dependency on `codex-core` that enabled that feature. - Removed the Bazel `deterministic_process_ids` crate feature for `codex-core`. - Added a test-only `AtomicBool` override in unified exec process-id allocation. - Added a test-support setter for that override and re-exported it from `codex-core`. - Enabled deterministic process IDs in integration tests via `core_test_support` ctor. ## Behavior - Production behavior remains random process IDs. - Unit tests remain deterministic via `cfg(test)`. - Integration tests remain deterministic via explicit test-support initialization. ## Validation - `just fmt` - `cargo test -p codex-core unified_exec::` - `cargo test -p codex-core --test all unified_exec -- --test-threads=1` - `cargo tree -p codex-core -e features` (verified the removed feature path)
This commit is contained in:
committed by
GitHub
Unverified
parent
8b46c0ce00
commit
b68a84ee8e
@@ -42,6 +42,11 @@ mod head_tail_buffer;
|
||||
mod process;
|
||||
mod process_manager;
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub fn set_deterministic_process_ids_for_tests(enabled: bool) {
|
||||
process_manager::set_deterministic_process_ids_for_tests(enabled);
|
||||
}
|
||||
|
||||
pub(crate) use errors::UnifiedExecError;
|
||||
pub(crate) use process::UnifiedExecProcess;
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::collections::HashSet;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tokio::sync::Notify;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::time::Duration;
|
||||
@@ -61,6 +62,32 @@ const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [
|
||||
("CODEX_CI", "1"),
|
||||
];
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
/// Test-only override for deterministic unified exec process IDs.
|
||||
///
|
||||
/// In production builds this value should remain at its default (`false`) and
|
||||
/// must not be toggled.
|
||||
static FORCE_DETERMINISTIC_PROCESS_IDS: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub(super) fn set_deterministic_process_ids_for_tests(enabled: bool) {
|
||||
FORCE_DETERMINISTIC_PROCESS_IDS.store(enabled, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
fn deterministic_process_ids_forced_for_tests() -> bool {
|
||||
FORCE_DETERMINISTIC_PROCESS_IDS.load(Ordering::Relaxed)
|
||||
}
|
||||
|
||||
#[cfg(not(any(test, feature = "test-support")))]
|
||||
fn deterministic_process_ids_forced_for_tests() -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
fn should_use_deterministic_process_ids() -> bool {
|
||||
cfg!(test) || deterministic_process_ids_forced_for_tests()
|
||||
}
|
||||
|
||||
fn apply_unified_exec_env(mut env: HashMap<String, String>) -> HashMap<String, String> {
|
||||
for (key, value) in UNIFIED_EXEC_ENV {
|
||||
env.insert(key.to_string(), value.to_string());
|
||||
@@ -85,10 +112,7 @@ impl UnifiedExecProcessManager {
|
||||
loop {
|
||||
let mut store = self.process_store.lock().await;
|
||||
|
||||
let process_id = if !cfg!(test) && !cfg!(feature = "deterministic_process_ids") {
|
||||
// production mode → random
|
||||
rand::rng().random_range(1_000..100_000).to_string()
|
||||
} else {
|
||||
let process_id = if should_use_deterministic_process_ids() {
|
||||
// test or deterministic mode
|
||||
let next = store
|
||||
.reserved_process_ids
|
||||
@@ -99,6 +123,9 @@ impl UnifiedExecProcessManager {
|
||||
.unwrap_or(1000);
|
||||
|
||||
next.to_string()
|
||||
} else {
|
||||
// production mode → random
|
||||
rand::rng().random_range(1_000..100_000).to_string()
|
||||
};
|
||||
|
||||
if store.reserved_process_ids.contains(&process_id) {
|
||||
|
||||
Reference in New Issue
Block a user