Shijie/codesign binary #4899
Shijie/codesign binary #4899
Conversation
shijie-oai
force-pushed
the
shijie/codesign-binary
branch
2 times, most recently
from
8786553 to
46c4c5a
Compare
shijie-oai
force-pushed
the
shijie/codesign-binary
branch
4 times, most recently
from
a219c47 to
6c7d335
Compare
shijie-oai
force-pushed
the
shijie/codesign-binary
branch
3 times, most recently
from
333724a to
795ac1d
Compare
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Based on doc, a cancelled job still runs steps with condition eval to true and therefore the keychain cleanup is still executed because of always().
| codesign_hashes=() | ||
| # SO this is breaking because our cert (at least the testing one) is not generated as codesign | ||
| while IFS= read -r hash; do | ||
| [[ -n "$hash" ]] && codesign_hashes+=("$hash") | ||
| done < <(security find-identity -v -p codesigning "$keychain_path" \ | ||
| | sed -n 's/.*\([0-9A-F]\{40\}\).*/\1/p' \ | ||
| | sort -u) | ||
| if ((${#codesign_hashes[@]} == 0)); then | ||
| echo "No signing identities found in $keychain_path" | ||
| cleanup_keychain | ||
| rm -f "$cert_path" | ||
| exit 1 | ||
| fi | ||
| if ((${#codesign_hashes[@]} > 1)); then | ||
| echo "Multiple signing identities found in $keychain_path:" | ||
| printf ' %s\n' "${codesign_hashes[@]}" | ||
| cleanup_keychain | ||
| rm -f "$cert_path" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
We should have ONE AND ONLY cert that can be used for codesign - populated above via a temp keychain created with our Apple cert. If there is none or more than one, we terminate and cleanup.
shijie-oai
force-pushed
the
shijie/codesign-binary
branch
from
795ac1d to
72b1eda
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| build: | ||
| needs: tag-check | ||
| name: ${{ matrix.runner }} - ${{ matrix.target }} | ||
| name: Build - ${{ matrix.runner }} - ${{ matrix.target }} |
There was a problem hiding this comment.
Curious why we're only using macOS 14 and not 15? (And also not 26?)
There was a problem hiding this comment.
For better backward compatibility I assume - 15 should be relatively safe to bump but not sure about 26 yet. I can open a separate PR to include both 14 and 15 and making sure that nothing break and then we can drop 14.
There was a problem hiding this comment.
Also, we have to think about what this means for things like the npm package, which is already quite large since it contains 6 native binaries.
Today, if you care about a macOS-specific build of Codex CLI, brew is probably the better way to get it (though that won't have the signing, which is an issue we're discussing).
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
Summary
Before - UNSIGNED codex-aarch64
After - SIGNED codex-aarch64