Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9416d588
ℹ️ 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 "@codex address that feedback".
|
|
||
| # Allow pkg-config resolution during cross-compilation. | ||
| echo "PKG_CONFIG_ALLOW_CROSS=1" >> "$GITHUB_ENV" | ||
| echo "PKG_CONFIG_PATH=${libcap_pkgconfig_dir}\${PKG_CONFIG_PATH:+:\${PKG_CONFIG_PATH}}" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Expand PKG_CONFIG_PATH before writing to GITHUB_ENV
This writes a literal \${PKG_CONFIG_PATH:+:\${PKG_CONFIG_PATH}} into $GITHUB_ENV instead of expanding it, so subsequent GitHub Actions steps receive a malformed PKG_CONFIG_PATH containing ${...} text rather than a real colon-joined path. Because GITHUB_ENV values are imported as-is, plain pkg-config consumers in later steps can miss the staged musl libcap.pc (and any prior path entries), which undermines the cross-linking setup this change is trying to enforce.
Useful? React with 👍 / 👎.
Problem:
The `aarch64-unknown-linux-musl` release build was failing at link time with
`/usr/bin/ld: cannot find -lcap` while building binaries that transitively pull
in `codex-linux-sandbox`. CI did not include an arm64 linux musl representative
release build in `rust-ci`, so this release-only target regression was not
caught before release.
Why this is the right fix:
`codex-linux-sandbox` compiles vendored bubblewrap and links `libcap`. In the
musl jobs, we were installing distro `libcap-dev`, which provides host/glibc
artifacts. That is not a valid source of target-compatible static libcap for
musl cross-linking. The correct fix is to produce a target-compatible libcap
inside the musl tool bootstrap and point pkg-config at it.
Separately, adding arm64 linux musl to the representative release matrix in
`rust-ci` ensures this class of release-only breakage is caught by normal CI.
Using `thin` LTO for those representative release-profile checks keeps CI
feedback fast while still exercising release-profile behavior.
What changed:
- Updated `.github/scripts/install-musl-build-tools.sh` to install tooling
needed to fetch/build libcap sources (`curl`, `xz-utils`, certs).
- Added deterministic libcap bootstrap in the musl tool root:
- download `libcap-2.75` from kernel.org
- verify SHA256
- build with the target musl compiler (`*-linux-musl-gcc`)
- stage `libcap.a` and headers under the target tool root
- generate a target-scoped `libcap.pc`
- Exported target `PKG_CONFIG_PATH` so builds resolve the staged musl libcap
instead of host pkg-config/lib paths.
- Added `ubuntu-24.04-arm` + `aarch64-unknown-linux-musl` + `profile: release`
to the representative release builds in `.github/workflows/rust-ci.yml`.
- Configured `rust-ci` to use `CARGO_PROFILE_RELEASE_LTO=thin` for matrix
entries where `profile == release`.
Verification:
- Reproduced the original failure in CI-like containers:
- `aarch64-unknown-linux-musl` failed with `cannot find -lcap`.
- Verified the underlying mismatch by forcing host libcap into the link:
- link then failed with glibc-specific unresolved symbols
(`__isoc23_*`, `__*_chk`), confirming host libcap was unsuitable.
- Verified the fix in CI-like containers after this change:
- `cargo build -p codex-linux-sandbox --target aarch64-unknown-linux-musl --release` -> pass
- `cargo build -p codex-linux-sandbox --target x86_64-unknown-linux-musl --release` -> pass
Problem
rust-releasewas failing for two independent reasons:aarch64-unknown-linux-muslfailed at link time with/usr/bin/ld: cannot find -lcapwhen building binaries that transitively pull incodex-linux-sandbox.releasejob failed inCreate GitHub ReleasewithNot Foundfrom the release-assets API becausefiles: dist/**included many files namedcargo-timing.html, which triggered conflicting delete/upload operations for the same release asset name.Why this is the right fix
codex-linux-sandboxbuilds vendored bubblewrap and linkslibcap. In musl jobs, distrolibcap-devprovides host/glibc artifacts, which are not valid for musl cross-linking. Building/staging target-compatiblelibcapin the musl tool bootstrap and pointingpkg-configat it fixes the linker mismatch directly.For release assets,
softprops/action-gh-releaseuploads by basename. Allowing multiplecargo-timing.htmlfiles intodist/**creates asset-name collisions and racey API behavior. Removing those files fromdist/before release upload prevents the collision.What changed
.github/scripts/install-musl-build-tools.shto install tooling needed to fetch/build libcap sources (curl,xz-utils, certs).libcap-2.75from kernel.org*-linux-musl-gcc)libcap.aand headers under the target tool rootlibcap.pcPKG_CONFIG_PATHso builds resolve the staged musl libcap instead of host pkg-config/lib paths..github/workflows/rust-ci.ymlto add areleasematrix entry foraarch64-unknown-linux-muslon the ARM runner..github/workflows/rust-ci.ymlto setCARGO_PROFILE_RELEASE_LTO=thinforreleasematrix entries (and keepfatfor non-release entries), matching the release-build tradeoff already used inrust-release.ymlwhile reducing CI runtime..github/workflows/rust-release.ymlrelease cleanup to removecargo-timing.htmlfiles fromdist/beforeCreate GitHub Release(files: dist/**).Verification
aarch64-unknown-linux-muslfailed withcannot find -lcap.__isoc23_*,__*_chk), confirming host libcap was unsuitable.cargo build -p codex-linux-sandbox --target aarch64-unknown-linux-musl --release-> passcargo build -p codex-linux-sandbox --target x86_64-unknown-linux-musl --release-> passrust-cion this branch and confirmed the new job appears:Lint/Build — ubuntu-24.04-arm - aarch64-unknown-linux-musl (release)rust-releaserun21938547951(releasejob), where duplicatecargo-timing.htmlassets causedNot Foundresponses from release asset update/delete API calls.