fix: small fixes so Codex compiles on Windows (#673)

Small fixes required:

* `ExitStatusExt` differs because UNIX expects exit code to be `i32`
whereas Windows does `u32`
* Marking a file "executable only by owner" is a bit more involved on
Windows. We just do something approximate for now (and add a TODO) to
get things compiling.

I created this PR on my personal Windows machine and `cargo test` and
`cargo clippy` succeed. Once this is in, I'll rebase
https://github.com/openai/codex/pull/665 on top so Windows stays fixed!
This commit is contained in:
Michael Bolin
2025-04-25 15:58:44 -07:00
committed by GitHub
parent ebd2ae4abd
commit c18f1689a9
2 changed files with 29 additions and 10 deletions

View File

@@ -322,7 +322,7 @@ fn synthetic_exit_status(code: i32) -> ExitStatus {
}
#[cfg(windows)]
fn synthetic_exit_status(code: u32) -> ExitStatus {
fn synthetic_exit_status(code: i32) -> ExitStatus {
use std::os::windows::process::ExitStatusExt;
std::process::ExitStatus::from_raw(code)
std::process::ExitStatus::from_raw(code.try_into().unwrap())
}

View File

@@ -13,7 +13,6 @@ use crate::Policy;
use crate::Result;
use crate::ValidExec;
use path_absolutize::*;
use std::os::unix::fs::PermissionsExt;
macro_rules! check_file_in_folders {
($file:expr, $folders:expr, $error:ident) => {
@@ -120,9 +119,20 @@ fn is_executable_file(path: &str) -> bool {
let file_path = Path::new(path);
if let Ok(metadata) = std::fs::metadata(file_path) {
let permissions = metadata.permissions();
// Check if the file is executable (by checking the executable bit for the owner)
return metadata.is_file() && (permissions.mode() & 0o111 != 0);
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let permissions = metadata.permissions();
// Check if the file is executable (by checking the executable bit for the owner)
return metadata.is_file() && (permissions.mode() & 0o111 != 0);
}
#[cfg(windows)]
{
// TODO(mbolin): Check against PATHEXT environment variable.
return metadata.is_file();
}
}
false
@@ -157,10 +167,19 @@ system_path=[{fake_cp:?}]
// Create an executable file that can be used with the system_path arg.
let fake_cp = temp_dir.path().join("cp");
let fake_cp_file = std::fs::File::create(&fake_cp).unwrap();
let mut permissions = fake_cp_file.metadata().unwrap().permissions();
permissions.set_mode(0o755);
std::fs::set_permissions(&fake_cp, permissions).unwrap();
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let fake_cp_file = std::fs::File::create(&fake_cp).unwrap();
let mut permissions = fake_cp_file.metadata().unwrap().permissions();
permissions.set_mode(0o755);
std::fs::set_permissions(&fake_cp, permissions).unwrap();
}
#[cfg(windows)]
{
std::fs::File::create(&fake_cp).unwrap();
}
// Create root_path and reference to files under the root.
let root_path = temp_dir.path().to_path_buf();