bundle sandbox helper binaries in main zip, for winget. #9707
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -285,7 +285,28 @@ jobs: | |
| # Must run from inside the dest dir so 7z won't | ||
| # embed the directory path inside the zip. | ||
| if [[ "${{ matrix.runner }}" == windows* ]]; then | ||
| if [[ "$base" == "codex-${{ matrix.target }}.exe" ]]; then | ||
| # Bundle the sandbox helper binaries into the main codex zip so | ||
| # WinGet installs include the required helpers next to codex.exe. | ||
| # Fall back to the single-binary zip if the helpers are missing | ||
| # to avoid breaking releases. | ||
| bundle_dir="$(mktemp -d)" | ||
| runner_src="$dest/codex-command-runner-${{ matrix.target }}.exe" | ||
| setup_src="$dest/codex-windows-sandbox-setup-${{ matrix.target }}.exe" | ||
| if [[ -f "$runner_src" && -f "$setup_src" ]]; then | ||
| cp "$dest/$base" "$bundle_dir/$base" | ||
| cp "$runner_src" "$bundle_dir/codex-command-runner.exe" | ||
| cp "$setup_src" "$bundle_dir/codex-windows-sandbox-setup.exe" | ||
| (cd "$bundle_dir" && 7z a "$dest/${base}.zip" .) | ||
| else | ||
| echo "warning: missing sandbox binaries; falling back to single-binary zip" | ||
| echo "warning: expected $runner_src and $setup_src" | ||
| (cd "$dest" && 7z a "${base}.zip" "$base") | ||
|
Comment on lines
+302
to
+304
Collaborator
There was a problem hiding this comment. Why shouldn't this break the release build? Hasn't something gone horribly wrong if this is true?
Collaborator
Author
There was a problem hiding this comment. I mostly don't want to break the build while I am testing/iterating on this. Like making sure I don't have a typo or something. Once it's working, I would revert any fallbacks and then the build would break if something was missing |
||
| fi | ||
| rm -rf "$bundle_dir" | ||
| else | ||
| (cd "$dest" && 7z a "${base}.zip" "$base") | ||
|
Collaborator
There was a problem hiding this comment. So this step should just be run for If so, maybe we should make an
Collaborator
Author
There was a problem hiding this comment. the bundling part of it should happen in the loop for
Collaborator
There was a problem hiding this comment. Oh, it is both of these: Does each of these need to be in its own It feels like we should favor the big
Collaborator
Author
There was a problem hiding this comment. yeah, both of those are being included in
Collaborator
There was a problem hiding this comment. The existing build script for the npm package appears to rely on a codex/codex-cli/scripts/install_native_deps.py Lines 334 to 337 in c6ded0a so I guess we still need to publish both of these until we update that script. |
||
| fi | ||
| fi | ||
|
|
||
| # Also create .zst (existing behaviour) *and* remove the original | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow: are we talking about what happens on our CI builder machine? Shouldn't that be deterministic? I'm confused what "Fall back" situation we are designing for here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is me not wanting to break the release, and if it fails to find the binaries (like I'm constructing the filepaths wrong) falling back to what currently happens (each binary in its own zip file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I plan to remove this once I'm sure this is working.