ext-cli: resolve $CC from the cross-canadian bindir, not the target sysroot - #200
ext-cli: resolve $CC from the cross-canadian bindir, not the target sysroot#200mobileoverlord wants to merge 5 commits into
Conversation
…ysroot
avocado-cli-compile.sh prepended $SDKTARGETSYSROOT/usr/bin to PATH so the
`cc` crate (via aws-lc-sys) could find the compiler $CC names. That bindir
holds the target-*native* toolchain, so "<triple>-gcc" resolved to a target
ELF; binfmt_misc handed it to qemu-user, which cannot resolve the target
loader and failed every compiler probe with exit 255:
qemu-aarch64: Could not open '/usr/lib/ld-linux-aarch64.so.1'
Compiler family detection failed ... exit status: 255
The cross-canadian compiler the SDK actually means lives under the SDK
*native* sysroot, at $OECORE_NATIVE_SYSROOT/usr/bin/<triple>/. Point PATH
there and fail loudly if it is absent.
This shipped because both ext workflows only build qemux86-64, where the
target gcc is same-arch and runs natively, so the wrong PATH entry is
harmless. Add a raspberrypi4 leg to ext-test.yml, which reproduces the
failure and now guards the fix.
Reported via avocado-linux/references#23, where the rubicon reference
cannot install avocado-ext-cli for raspberrypi4.
There was a problem hiding this comment.
Pull request overview
This PR fixes cross-compilation of avocado-ext-cli by ensuring the Rust cc crate resolves the correct cross-canadian compiler from the SDK native sysroot (instead of accidentally picking up a target-native gcc from the target sysroot), and adds CI coverage to catch cross-arch regressions.
Changes:
- Update
avocado-cli-compile.shto prepend the cross-canadian bindir under$OECORE_NATIVE_SYSROOTtoPATH, with a loud failure if the expected compiler binary is missing. - Add a
raspberrypi4(cross-arch) leg toext-test.ymlto reproduce and prevent the original failure mode.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| avocado-cli-compile.sh | Switch PATH setup to use the SDK native sysroot cross-canadian bindir and validate the cross compiler is present. |
| .github/workflows/ext-test.yml | Add a cross-arch CI matrix entry (raspberrypi4) to detect host/target toolchain mix-ups. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
packagegroup-rust-cross-canadian-avocado-<target> brings rust-cross-canadian plus nativesdk-gcc -- a HOST compiler. It does not bring a C cross-compiler, so gcc-cross-canadian-<arch> was never installed and $OECORE_NATIVE_SYSROOT/usr/bin/<triple>/<triple>-gcc did not exist. That is why the target-sysroot PATH hack appeared to work: with no cross compiler installed, the only <triple>-gcc anywhere was the target-native one in $SDKTARGETSYSROOT, which happens to be host-executable when target arch == host arch (qemux86-64). On a cross-arch target it is a foreign ELF and every compiler probe died under qemu-user. Pull packagegroup-cross-canadian-avocado-<target> so the real cross compiler is present for the PATH fix to find.
If OECORE_NATIVE_SYSROOT and CROSS_COMPILE expand empty, CROSS_BINDIR collapses to /usr/bin and the -x check finds the host gcc, so the build silently produces a native binary packaged as a target extension -- the same silently-wrong class of failure this branch is fixing. The RUST_TARGET_PATH loop above already aborts when the env is entirely unsourced, but not when it is partially set, and the script runs with set -e and not set -u. Reported by Copilot on #200.
jetm
left a comment
jetm
left a comment
There was a problem hiding this comment.
The core fix is right and the evidence for it is solid. I verified the parts that could have been hand-waved: ${CROSS_COMPILE%-} is the correct bindir derivation for every triple, the deleted $SDKTARGETSYSROOT/usr/bin entry took nothing away ($AR/$RANLIB/$LD/$OBJCOPY/$STRIP are all ${TARGET_PREFIX}-prefixed and live in the new dir), prepending cannot shadow host cargo/rustc, and packagegroup-cross-canadian-avocado-* really does resolve — there is a built RPM in the meta-avocado feed snapshot, packagegroup-rust-cross-canadian does not RDEPEND on it, so that line is load-bearing rather than belt-and-braces. shellcheck -s bash and bash -n are clean, and I simulated the new guard across unset / empty / missing-binary / happy paths and it fires correctly in each.
One thing I would want answered before this merges.
The new sdk.packages entry feeds no stamp hash, so a warm SDK never installs it. compute_sdk_input_hash (stamps.rs:968) reads only top-level sdk.*, and ext_build_hash_data (stamps.rs:1168) omits ext.sdk.packages. So an existing install stays cached, never picks up packagegroup-cross-canadian-avocado-*, and lands on the new hard exit 1 at avocado-cli-compile.sh:68 — whose message does not mention avocado sdk install --force. That turns an upgrade into a dead end for anyone who already has the SDK. Either fold ext.sdk.packages into the hash, or at minimum name the recovery in the abort message.
Second, and I could not verify it from here: .github/workflows/ext-release.yml:51 keeps the publish matrix qemux86-64-only, so no raspberrypi4 artifact ships. If the motivating report is the rubicon reference not being able to install avocado-ext-cli for raspberrypi4, this merge does not resolve it — but references#23 and avocado-linux/actions are not readable from here, so treat that as a question rather than a finding.
Nits, all optional:
avocado-cli-compile.sh:64—$SDKTARGETSYSROOTis the one SDK var the script actually consumes and it is the one left out of the new:?guards. Same silently-wrong class the commit body names.avocado-cli-compile.sh:41— both abort paths run after.cargo/config.tomlis written, leaving a stale gitignored file behind. I reproduced it. Moving the guards above the write fails before mutating the tree.avocado-cli-compile.sh:68— the guard proves${CROSS_COMPILE}gccexists, but thecccrate resolves the first token of$CC, which is never checked. It also aborts a working clang SDK.avocado-cli-compile.sh:8— pre-existing, not yours: the"${OECORE_TARGET_ARCH}-"*glob cannot match armv7 (armvsarmv7triple), sosupported_targets: '*'is not truly arch-agnostic.avocado-cli-install.sh:4-18is a byte-identical copy of the same block.- Bisectability: 03a770c adds the hard
exit 1and the rpi4 leg, but the packagegroup arrives in 215e2f3, so both legs are red at 03a770c. Cosmetic if this squash-merges.
CI legs stay on qemu targets — swap the raspberrypi4 leg for qemuarm64. It is aarch64/cortexa57, so it reproduces the host/target toolchain mix-up exactly the same way, and packagegroup-cross-canadian-avocado-qemuarm64 is in the 2024 feed. Review feedback on the guard, all in avocado-cli-compile.sh: - Move the env guards above the .cargo/config.toml write. Both abort paths used to run after the file was created, leaving a stale gitignored file behind. - Guard $SDKTARGETSYSROOT too. It is the one SDK var the script actually consumes, and an empty one bakes a bogus --sysroot into the cargo config — the same silently-wrong class the other two guards exist to prevent. - Check the binary `cc` will really invoke: the first token of $CC, falling back to "<triple>-gcc" only when $CC is unset. The old check hardcoded gcc, so it verified a binary the build might not use and aborted a working clang SDK. - Name the recovery in the abort message. An SDK installed before the packagegroup was added to avocado.yaml does not have it, and nothing invalidates that install (the sdk-install stamp's config hash is written but never compared — no caller passes SDK CurrentInput), so the message has to say `avocado sdk install --force`.
|
Thanks — the verification work here saved me time, particularly confirming The stamp findingThe symptom is real and I hit exactly the dead end you describe. But I don't think folding
So a warm SDK's install stamp never goes stale on a config change today, whatever we put in the hash. Adding Took your second option instead, since it's the part that reaches the user: Worth noting NitsAll three taken, and 1 and 2 turned out to be the same fix — moving the guards above the : "${OECORE_NATIVE_SYSROOT:?not set -- SDK environment-setup was not sourced}"
: "${SDKTARGETSYSROOT:?not set -- SDK environment-setup was not sourced}"
: "${CROSS_COMPILE:?not set -- SDK environment-setup was not sourced}"For nit 3, the check now resolves what CC_BIN="${CC:-${CROSS_COMPILE}gcc}"
CC_BIN="${CC_BIN%% *}"Simulated all of it against a fake SDK layout: unset env, partially-set env, missing compiler, happy path with a flag-carrying The armv7 glob is a real bug and I'd rather fix it where both copies live — TargetsSwapped the On the release matrix: you're right that this merge doesn't unblock rubicon. |
jetm
left a comment
jetm
left a comment
There was a problem hiding this comment.
Re-reviewed the delta since 60953ec (the compiler-guard hardening and the qemuarm64 leg). Three blocking findings, all inline on the same 6-line hunk in avocado-cli-compile.sh and all sharing one fix.
Six advisory notes were withheld rather than appended here, so the blocking ones stay readable.
| # unset. Hardcoding gcc here would also abort a working clang SDK. | ||
| CC_BIN="${CC:-${CROSS_COMPILE}gcc}" | ||
| CC_BIN="${CC_BIN%% *}" | ||
| if [ ! -x "$CROSS_BINDIR/$CC_BIN" ]; then |
There was a problem hiding this comment.
Empty CC_BIN makes -x test the bindir dir, so the guard passes
When $CC's first token resolves to empty, [ ! -x "$CROSS_BINDIR/$CC_BIN" ] tests $CROSS_BINDIR itself, and -x is true for any searchable directory - so the guard silently passes on exactly the case it exists to catch.
Reproduced against this logic: with the toolchain genuinely absent and CC=" aarch64-avocado-linux-gcc -mcpu=cortex-a57" (leading space), ${CC:-...} does not fire because the value is non-empty, and ${CC_BIN%% *} yields the empty string. The control run prints GUARD FIRED; the leading-space run prints GUARD PASSED and proceeds to cargo build with CC_BIN=[]. The build then dies inside aws-lc-sys with the opaque ToolNotFound this block was added to pre-empt.
Worth noting cc-rs handles this input fine - cc-1.2.61 src/lib.rs:3224 trims and :3225 returns None on empty. Only the guard does not.
There was a problem hiding this comment.
Fixed in a9feb3a. This is the one that mattered most — a guard that reports success on the exact condition it was added to catch is worse than no guard.
Reproduced your leading-space case first (CC=" aarch64-avocado-linux-gcc -mcpu=cortex-a57", toolchain absent → PASSED), then confirmed it fires after the change. -x on a searchable directory being true is an easy thing to write twice, so it is worth having in the thread.
| # rest are target flags) and falls back to guessing "<triple>-gcc" when $CC is | ||
| # unset. Hardcoding gcc here would also abort a working clang SDK. | ||
| CC_BIN="${CC:-${CROSS_COMPILE}gcc}" | ||
| CC_BIN="${CC_BIN%% *}" |
There was a problem hiding this comment.
First-token CC_BIN falsely aborts on a wrapper or an absolute-path CC
CC_BIN="${CC_BIN%% *}" takes the first whitespace token unconditionally, but cc-rs passes an exact filesystem path through whole and treats a leading known wrapper as a wrapper. So a working SDK gets a hard abort whose message blames a missing toolchain.
Both reproduced with a working compiler present in CROSS_BINDIR: CC="ccache aarch64-avocado-linux-gcc ..." gives CC_BIN=ccache, and CC="/abs/path/aarch64-avocado-linux-gcc ..." gives "$CROSS_BINDIR//abs/path/...". Each prints GUARD FIRED, exits 1, and tells the user to run avocado sdk install --force - a multi-minute no-op. cc-rs handles both: lib.rs:3234 passes an exe path through, and :3262-3281 knows the wrapper list (ccache/distcc/sccache/icecc/cachepot/buildcache) and takes the second token.
There is a third divergence in the same line: %% * splits only on a literal space, so a tab-separated $CC keeps its flags attached and aborts, while cc-rs uses split_whitespace. The ${CROSS_COMPILE}gcc check this replaced was immune to all three.
There was a problem hiding this comment.
Fixed in a9feb3a by dropping the derivation entirely, which is the version of "resolve it the way cc-rs does" that requires no cc-rs-shaped parser in shell.
Both cases confirmed against a fake SDK with a working compiler present — CC="ccache aarch64-avocado-linux-gcc …" and CC="/abs/path/aarch64-avocado-linux-gcc …" each aborted before and pass now. Added the tab case to the same harness: also aborted before, passes now. Aborting a working SDK with a message telling the user to sit through sdk install --force was the worst of the three outcomes.
|
|
||
| # Check the binary `cc` will actually run: it takes the FIRST token of $CC (the | ||
| # rest are target flags) and falls back to guessing "<triple>-gcc" when $CC is | ||
| # unset. Hardcoding gcc here would also abort a working clang SDK. |
There was a problem hiding this comment.
The clang-SDK justification cannot occur in this SDK stack
The comment justifies reading $CC on the grounds that hardcoding gcc "would abort a working clang SDK". That case cannot arise here, so the stated benefit never materializes while the two divergences above are new costs.
openembedded-core/meta/classes-recipe/toolchain-scripts.bbclass:135 writes export CC="${TARGET_PREFIX}gcc ${TARGET_CC_ARCH} --sysroot=$SDKTARGETSYSROOT" unconditionally, and :158 writes export CROSS_COMPILE=${TARGET_PREFIX} - so $CC's first token is byte-identical to ${CROSS_COMPILE}gcc in every stock SDK. meta-clang/recipes-core/meta/clang-environment.inc only appends CLANGCC/CLANGCXX/CLANGCPP and never overrides CC, and no override of toolchain_shared_env_script exists in oe-core, meta-clang, meta-avocado or meta-peridio.
Reproduced: CC unset with only aarch64-avocado-linux-clang in the bindir still prints GUARD FIRED - it aborts the clang SDK exactly as the old check did.
All three findings sit in this one 6-line hunk and share a single fix: either resolve CC_BIN the way cc-rs does, or drop the derivation, since it is provably identical to ${CROSS_COMPILE}gcc here.
There was a problem hiding this comment.
Taken — the derivation is gone in a9feb3a, back to CC_BIN="${CROSS_COMPILE}gcc".
Your toolchain-scripts.bbclass:135/:158 reading is the part that decided it. If both come from ${TARGET_PREFIX} unconditionally then the derivation was computing a value it already had, and the clang case it was justified by aborts either way — I re-ran that one and confirmed it: clang-only bindir fires the guard identically before and after. So the stated benefit was never real and the two divergences were pure cost.
Parsing $CC's first token to find the compiler had three failure modes and
no upside. oe-core writes CC and CROSS_COMPILE from the same TARGET_PREFIX
(toolchain-scripts.bbclass), so the derived name is byte-identical to
${CROSS_COMPILE}gcc in every SDK this runs against, and meta-clang appends
CLANGCC rather than overriding CC -- so the clang SDK the derivation was
meant to accommodate aborted either way.
What it did do:
- an empty first token (a $CC with a leading space) made `-x` test the
bindir itself, which is executable, so the guard passed on exactly the
case it exists to catch
- a ccache/distcc wrapper resolved to "ccache" and hard-aborted a working
SDK, telling the user to run a multi-minute no-op reinstall
- an absolute-path $CC produced "$CROSS_BINDIR//abs/path/..." and aborted
- a tab-separated $CC kept its flags attached and aborted
Simulated all four against a fake SDK layout plus the happy path, a stock
flag-carrying $CC and a clang-only bindir: every case that regressed a
working SDK now passes, the empty-token case now fires, and the control
and clang cases are unchanged.
|
All three blocking findings are fixed in a9feb3a, and they did share one fix — the one you named second: drop the derivation. Replies are on each thread. The deciding argument was your I kept your reproductions as a harness rather than reasoning about them, running each case against both the old and new logic on a fake SDK layout:
The last row is the one worth pointing at: the clang SDK the derivation existed to protect aborts identically either way, so that justification never bought anything. Bolded cells are your three findings. On the six withheld advisory notes — please do append them. Holding them back kept this review readable and that was the right call for a set of blocking findings, but I would rather have them than not, and they can land as a plain list with no ceremony. Same for anything you set aside on #201 through #205. |
What
avocado-cli-compile.shprepended$SDKTARGETSYSROOT/usr/bintoPATHso thecccrate (pulled in byaws-lc-sys) could resolve the compiler$CCnames. That bindir holds the target-native toolchain, not the cross compiler — the targetgccpackage owns/usr/bin/<triple>-gccin the target sysroot. So<triple>-gccresolved to a target ELF,binfmt_mischanded it to qemu-user, and every compiler probe died:The cross-canadian compiler the SDK actually means lives under the SDK native sysroot:
Point
PATHat the former and fail loudly if it is missing.Why it shipped
Both
ext-test.ymlandext-release.ymlonly buildqemux86-64. On a same-arch target the targetgccruns natively, so picking the wrong one still builds — the bug is invisible. It only fires on a cross-arch target, and only sinceaws-lc-sysentered the dependency graph (reqwest 0.13+rustls→aws-lc-rs), because nothing before it had to invoke a C compiler.Regression test
Adds a
raspberrypi4leg toext-test.yml. That leg reproduces the failure onmainand passes with this fix. Verified against the live2024/edgefeed:gcc-cross-canadian-aarch64is installed in the SDK and provides$OECORE_NATIVE_SYSROOT/usr/bin/aarch64-avocado-linux/aarch64-avocado-linux-gcc.Blast radius
Every consumer of
avocado-ext-clion a cross-arch target is currently broken — the published0.41.2cannot build. Found via avocado-linux/references#23, where therubiconreference fails to install it forraspberrypi4. Needs a release to republish once merged.