From 80c4a978f8c955b6eb820953b0a3023e5596fc54 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 20 May 2026 14:51:46 -0700 Subject: [PATCH] release: package prebuilt resource binaries (#23759) ## Why Release packaging should be a staging step once release binaries have already been built and signed. The Windows release job was downloading and signing `codex-command-runner.exe` and `codex-windows-sandbox-setup.exe`, but `scripts/build_codex_package.py` still rebuilt those helpers while creating the package archives. That makes the package step slower and, more importantly, risks putting helper binaries in the archive that were produced after the signing step. Linux had the same shape for package resources: `bwrap` could be rebuilt by the package builder instead of being passed in as a prebuilt release artifact. This builds on #23752, which fixes `.tar.zst` creation when Windows runners rely on the repository DotSlash `zstd` wrapper. ## What changed - Add explicit prebuilt resource inputs to the Codex package builder: - `--bwrap-bin` - `--codex-command-runner-bin` - `--codex-windows-sandbox-setup-bin` - Make `.github/scripts/build-codex-package-archive.sh` pass resource binaries from the release output directory when they are already present. - Build Linux `bwrap` for app-server release jobs too, so app-server package creation does not invoke Cargo just to supply the package resource. - Keep macOS package creation as a no-Cargo path when `--entrypoint-bin` is provided, since macOS packages have no resource binaries. - Add unit coverage showing prebuilt macOS, Linux, and Windows package inputs result in no source-built binaries. ## Verification - `python3 -m unittest discover -s scripts/codex_package -p 'test_*.py'` - `python3 -m py_compile scripts/codex_package/*.py` - `bash -n .github/scripts/build-codex-package-archive.sh` - Dry-ran Linux and Windows package builds with fake prebuilt resources and a nonexistent Cargo path to verify the package builder did not invoke Cargo. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23759). * #23760 * __->__ #23759 --- .../scripts/build-codex-package-archive.sh | 72 ++++++++++++-- .github/workflows/rust-release.yml | 2 +- scripts/codex_package/README.md | 16 ++- scripts/codex_package/cargo.py | 78 +++++++++++---- scripts/codex_package/cli.py | 64 ++++++++++-- scripts/codex_package/test_archive.py | 2 - scripts/codex_package/test_cargo.py | 98 +++++++++++++++++++ 7 files changed, 291 insertions(+), 41 deletions(-) create mode 100644 scripts/codex_package/test_cargo.py diff --git a/.github/scripts/build-codex-package-archive.sh b/.github/scripts/build-codex-package-archive.sh index c475ffe95..80da4cf20 100644 --- a/.github/scripts/build-codex-package-archive.sh +++ b/.github/scripts/build-codex-package-archive.sh @@ -8,6 +8,9 @@ Usage: build-codex-package-archive.sh \ --bundle \ --entrypoint-dir \ --archive-dir \ + [--bwrap-bin ] \ + [--codex-command-runner-bin ] \ + [--codex-windows-sandbox-setup-bin ] \ [--target-suffixed-entrypoint] EOF } @@ -17,6 +20,10 @@ bundle="" entrypoint_dir="" archive_dir="" target_suffixed_entrypoint="false" +resource_args=() +bwrap_bin_provided="false" +command_runner_bin_provided="false" +sandbox_setup_bin_provided="false" while [[ $# -gt 0 ]]; do case "$1" in @@ -36,6 +43,27 @@ while [[ $# -gt 0 ]]; do archive_dir="${2:?--archive-dir requires a value}" shift 2 ;; + --bwrap-bin) + resource_args+=(--bwrap-bin "${2:?--bwrap-bin requires a value}") + bwrap_bin_provided="true" + shift 2 + ;; + --codex-command-runner-bin) + resource_args+=( + --codex-command-runner-bin + "${2:?--codex-command-runner-bin requires a value}" + ) + command_runner_bin_provided="true" + shift 2 + ;; + --codex-windows-sandbox-setup-bin) + resource_args+=( + --codex-windows-sandbox-setup-bin + "${2:?--codex-windows-sandbox-setup-bin requires a value}" + ) + sandbox_setup_bin_provided="true" + shift 2 + ;; --target-suffixed-entrypoint) target_suffixed_entrypoint="true" shift @@ -86,6 +114,25 @@ if [[ "$target_suffixed_entrypoint" == "true" ]]; then entrypoint_name="${entrypoint_name}-${target}" fi +case "$target" in + *linux*) + bwrap_bin="${entrypoint_dir%/}/bwrap" + if [[ "$bwrap_bin_provided" == "false" && -f "$bwrap_bin" ]]; then + resource_args+=(--bwrap-bin "$bwrap_bin") + fi + ;; + *windows*) + command_runner_bin="${entrypoint_dir%/}/codex-command-runner.exe" + sandbox_setup_bin="${entrypoint_dir%/}/codex-windows-sandbox-setup.exe" + if [[ "$command_runner_bin_provided" == "false" && -f "$command_runner_bin" ]]; then + resource_args+=(--codex-command-runner-bin "$command_runner_bin") + fi + if [[ "$sandbox_setup_bin_provided" == "false" && -f "$sandbox_setup_bin" ]]; then + resource_args+=(--codex-windows-sandbox-setup-bin "$sandbox_setup_bin") + fi + ;; +esac + repo_root="${GITHUB_WORKSPACE:-}" if [[ -z "$repo_root" ]]; then repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" @@ -107,12 +154,19 @@ gzip_archive_path="${archive_dir}/${archive_stem}-${target}.tar.gz" zstd_archive_path="${archive_dir}/${archive_stem}-${target}.tar.zst" rm -rf "$package_dir" -"$python_bin" "${repo_root}/scripts/build_codex_package.py" \ - --target "$target" \ - --variant "$variant" \ - --entrypoint-bin "${entrypoint_dir%/}/${entrypoint_name}${exe_suffix}" \ - --cargo-profile release \ - --package-dir "$package_dir" \ - --archive-output "$gzip_archive_path" \ - --archive-output "$zstd_archive_path" \ - --force +python_args=( + "${repo_root}/scripts/build_codex_package.py" + --target "$target" + --variant "$variant" + --entrypoint-bin "${entrypoint_dir%/}/${entrypoint_name}${exe_suffix}" + --cargo-profile release + --package-dir "$package_dir" + --archive-output "$gzip_archive_path" + --archive-output "$zstd_archive_path" +) +if ((${#resource_args[@]} > 0)); then + python_args+=("${resource_args[@]}") +fi +python_args+=(--force) + +"$python_bin" "${python_args[@]}" diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 9be5106bb..c88fede7f 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -346,7 +346,7 @@ jobs: with: target: ${{ matrix.target }} - - if: ${{ contains(matrix.target, 'linux') && matrix.bundle == 'primary' }} + - if: ${{ contains(matrix.target, 'linux') }} name: Build bwrap and export digest shell: bash run: | diff --git a/scripts/codex_package/README.md b/scripts/codex_package/README.md index c1674a8b7..f53f1a41a 100644 --- a/scripts/codex_package/README.md +++ b/scripts/codex_package/README.md @@ -34,12 +34,14 @@ read from `[workspace.package].version` in `codex-rs/Cargo.toml`. ## Source-built artifacts -Artifacts built from this repository are always built by the package builder in -one grouped `cargo build` command per package when they are needed: +Artifacts built from this repository are built by the package builder in one +grouped `cargo build` command per package when they are needed and no prebuilt +override was provided: - all targets: the selected entrypoint, unless `--entrypoint-bin` is provided -- Linux targets: `bwrap` -- Windows targets: `codex-command-runner` and `codex-windows-sandbox-setup` +- Linux targets: `bwrap`, unless `--bwrap-bin` is provided +- Windows targets: `codex-command-runner` and `codex-windows-sandbox-setup`, + unless the corresponding prebuilt helper flags are provided The default cargo profile is `dev-small` because local iteration should favor fast, small builds. Release jobs should pass `--cargo-profile release` and an @@ -47,6 +49,12 @@ explicit target. Release jobs that already built and signed/notarized the entrypoint should pass `--entrypoint-bin` so the package contains that exact binary instead of rebuilding it. +Release jobs that already built package resource binaries should also pass the +corresponding resource flags: `--bwrap-bin` for Linux packages, and +`--codex-command-runner-bin` plus `--codex-windows-sandbox-setup-bin` for +Windows packages. This keeps package archive creation as a pure staging step +after signing instead of rebuilding resources. + `rg` is not built from this repository, so the builder fetches it from the DotSlash manifest at `codex-cli/bin/rg`. Downloaded archives are cached under `$TMPDIR/codex-package/-rg` and are reused only after the recorded size diff --git a/scripts/codex_package/cargo.py b/scripts/codex_package/cargo.py index c265808b1..f7fbf0f9a 100644 --- a/scripts/codex_package/cargo.py +++ b/scripts/codex_package/cargo.py @@ -28,11 +28,25 @@ def build_source_binaries( cargo: str, profile: str, entrypoint_bin: Path | None, + bwrap_bin: Path | None, + codex_command_runner_bin: Path | None, + codex_windows_sandbox_setup_bin: Path | None, ) -> SourceBuildOutputs: + validate_prebuilt_resource_inputs( + spec, + bwrap_bin=bwrap_bin, + codex_command_runner_bin=codex_command_runner_bin, + codex_windows_sandbox_setup_bin=codex_windows_sandbox_setup_bin, + ) binaries = source_binaries_for_target( spec, variant, build_entrypoint=entrypoint_bin is None, + build_bwrap=spec.is_linux and bwrap_bin is None, + build_codex_command_runner=spec.is_windows + and codex_command_runner_bin is None, + build_codex_windows_sandbox_setup=spec.is_windows + and codex_windows_sandbox_setup_bin is None, ) if binaries: cmd = [ @@ -51,17 +65,21 @@ def build_source_binaries( output_dir = cargo_profile_output_dir(spec, profile) outputs = SourceBuildOutputs( - entrypoint_bin=( - entrypoint_bin.resolve() - if entrypoint_bin is not None - else output_dir / variant.entrypoint_name(spec) + entrypoint_bin=resolve_output_path( + entrypoint_bin, + output_dir / variant.entrypoint_name(spec), ), - bwrap_bin=output_dir / "bwrap" if spec.is_linux else None, - codex_command_runner_bin=( - output_dir / "codex-command-runner.exe" if spec.is_windows else None + bwrap_bin=resolve_output_path( + bwrap_bin, + output_dir / "bwrap" if spec.is_linux else None, ), - codex_windows_sandbox_setup_bin=( - output_dir / "codex-windows-sandbox-setup.exe" if spec.is_windows else None + codex_command_runner_bin=resolve_output_path( + codex_command_runner_bin, + output_dir / "codex-command-runner.exe" if spec.is_windows else None, + ), + codex_windows_sandbox_setup_bin=resolve_output_path( + codex_windows_sandbox_setup_bin, + output_dir / "codex-windows-sandbox-setup.exe" if spec.is_windows else None, ), ) validate_source_outputs(outputs) @@ -73,22 +91,48 @@ def source_binaries_for_target( variant: PackageVariant, *, build_entrypoint: bool, + build_bwrap: bool, + build_codex_command_runner: bool, + build_codex_windows_sandbox_setup: bool, ) -> list[str]: binaries = [] if build_entrypoint: binaries.append(variant.cargo_bin) - if spec.is_linux: + if build_bwrap: binaries.append("bwrap") - if spec.is_windows: - binaries.extend( - [ - "codex-command-runner", - "codex-windows-sandbox-setup", - ] - ) + if build_codex_command_runner: + binaries.append("codex-command-runner") + if build_codex_windows_sandbox_setup: + binaries.append("codex-windows-sandbox-setup") return binaries +def validate_prebuilt_resource_inputs( + spec: TargetSpec, + *, + bwrap_bin: Path | None, + codex_command_runner_bin: Path | None, + codex_windows_sandbox_setup_bin: Path | None, +) -> None: + if bwrap_bin is not None and not spec.is_linux: + raise RuntimeError("--bwrap-bin is only supported for Linux targets.") + if codex_command_runner_bin is not None and not spec.is_windows: + raise RuntimeError( + "--codex-command-runner-bin is only supported for Windows targets." + ) + if codex_windows_sandbox_setup_bin is not None and not spec.is_windows: + raise RuntimeError( + "--codex-windows-sandbox-setup-bin is only supported for Windows targets." + ) + + +def resolve_output_path(explicit_path: Path | None, default_path: Path | None) -> Path | None: + if explicit_path is not None: + return explicit_path.resolve() + + return default_path + + def cargo_profile_output_dir(spec: TargetSpec, profile: str) -> Path: target_dir = cargo_target_dir() return target_dir / spec.target / cargo_profile_dirname(profile) diff --git a/scripts/codex_package/cli.py b/scripts/codex_package/cli.py index 0ca9f5d35..36ceda589 100644 --- a/scripts/codex_package/cli.py +++ b/scripts/codex_package/cli.py @@ -83,6 +83,32 @@ def parse_args() -> argparse.Namespace: "variant. If omitted, the entrypoint is built with Cargo." ), ) + parser.add_argument( + "--bwrap-bin", + type=Path, + help=( + "Optional prebuilt Linux bwrap executable. If omitted for Linux " + "targets, bwrap is built with Cargo." + ), + ) + parser.add_argument( + "--codex-command-runner-bin", + type=Path, + help=( + "Optional prebuilt Windows codex-command-runner.exe executable. " + "If omitted for Windows targets, codex-command-runner is built " + "with Cargo." + ), + ) + parser.add_argument( + "--codex-windows-sandbox-setup-bin", + type=Path, + help=( + "Optional prebuilt Windows codex-windows-sandbox-setup.exe " + "executable. If omitted for Windows targets, " + "codex-windows-sandbox-setup is built with Cargo." + ), + ) parser.add_argument( "--rg-bin", type=Path, @@ -110,14 +136,25 @@ def main() -> int: variant, cargo=args.cargo, profile=args.cargo_profile, - entrypoint_bin=( - resolve_input_path( - args.entrypoint_bin, - "prebuilt entrypoint executable", - "--entrypoint-bin", - ) - if args.entrypoint_bin is not None - else None + entrypoint_bin=resolve_optional_input_path( + args.entrypoint_bin, + "prebuilt entrypoint executable", + "--entrypoint-bin", + ), + bwrap_bin=resolve_optional_input_path( + args.bwrap_bin, + "prebuilt Linux bwrap executable", + "--bwrap-bin", + ), + codex_command_runner_bin=resolve_optional_input_path( + args.codex_command_runner_bin, + "prebuilt Windows codex-command-runner.exe executable", + "--codex-command-runner-bin", + ), + codex_windows_sandbox_setup_bin=resolve_optional_input_path( + args.codex_windows_sandbox_setup_bin, + "prebuilt Windows codex-windows-sandbox-setup.exe executable", + "--codex-windows-sandbox-setup-bin", ), ) version = read_workspace_version() @@ -139,3 +176,14 @@ def main() -> int: print(f"Built Codex package directory at {package_dir}") return 0 + + +def resolve_optional_input_path( + explicit_path: Path | None, + description: str, + flag_name: str, +) -> Path | None: + if explicit_path is None: + return None + + return resolve_input_path(explicit_path, description, flag_name) diff --git a/scripts/codex_package/test_archive.py b/scripts/codex_package/test_archive.py index cade7d12f..ae3902e21 100644 --- a/scripts/codex_package/test_archive.py +++ b/scripts/codex_package/test_archive.py @@ -1,7 +1,5 @@ #!/usr/bin/env python3 -from __future__ import annotations - from pathlib import Path import sys import tempfile diff --git a/scripts/codex_package/test_cargo.py b/scripts/codex_package/test_cargo.py new file mode 100644 index 000000000..d090bfd1e --- /dev/null +++ b/scripts/codex_package/test_cargo.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 + +from pathlib import Path +import sys +import tempfile +import unittest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from codex_package.cargo import build_source_binaries +from codex_package.cargo import source_binaries_for_target +from codex_package.targets import PACKAGE_VARIANTS +from codex_package.targets import TARGET_SPECS + + +class SourceBinariesForTargetTest(unittest.TestCase): + def test_macos_package_with_prebuilt_entrypoint_builds_nothing(self) -> None: + self.assertEqual( + source_binaries_for_target( + TARGET_SPECS["aarch64-apple-darwin"], + PACKAGE_VARIANTS["codex"], + build_entrypoint=False, + build_bwrap=False, + build_codex_command_runner=False, + build_codex_windows_sandbox_setup=False, + ), + [], + ) + + def test_linux_package_with_prebuilt_entrypoint_and_bwrap_builds_nothing(self) -> None: + self.assertEqual( + source_binaries_for_target( + TARGET_SPECS["x86_64-unknown-linux-musl"], + PACKAGE_VARIANTS["codex"], + build_entrypoint=False, + build_bwrap=False, + build_codex_command_runner=False, + build_codex_windows_sandbox_setup=False, + ), + [], + ) + + def test_windows_package_with_prebuilt_entrypoint_and_helpers_builds_nothing(self) -> None: + self.assertEqual( + source_binaries_for_target( + TARGET_SPECS["x86_64-pc-windows-msvc"], + PACKAGE_VARIANTS["codex"], + build_entrypoint=False, + build_bwrap=False, + build_codex_command_runner=False, + build_codex_windows_sandbox_setup=False, + ), + [], + ) + + def test_missing_windows_helpers_are_built(self) -> None: + self.assertEqual( + source_binaries_for_target( + TARGET_SPECS["x86_64-pc-windows-msvc"], + PACKAGE_VARIANTS["codex"], + build_entrypoint=False, + build_bwrap=False, + build_codex_command_runner=True, + build_codex_windows_sandbox_setup=True, + ), + ["codex-command-runner", "codex-windows-sandbox-setup"], + ) + + def test_build_uses_prebuilt_windows_helpers_without_running_cargo(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + entrypoint = touch_file(root / "codex.exe") + command_runner = touch_file(root / "codex-command-runner.exe") + sandbox_setup = touch_file(root / "codex-windows-sandbox-setup.exe") + + outputs = build_source_binaries( + TARGET_SPECS["x86_64-pc-windows-msvc"], + PACKAGE_VARIANTS["codex"], + cargo=str(root / "cargo-that-should-not-run"), + profile="release", + entrypoint_bin=entrypoint, + bwrap_bin=None, + codex_command_runner_bin=command_runner, + codex_windows_sandbox_setup_bin=sandbox_setup, + ) + + self.assertEqual(outputs.entrypoint_bin, entrypoint) + self.assertEqual(outputs.codex_command_runner_bin, command_runner) + self.assertEqual(outputs.codex_windows_sandbox_setup_bin, sandbox_setup) + + +def touch_file(path: Path) -> Path: + path.write_text("", encoding="utf-8") + return path.resolve() + + +if __name__ == "__main__": + unittest.main()