Skip to content

fix(build): guard dev wrapper scripts against cross/OE env + document 3rd-party integration - #628

Merged
Ulrond merged 7 commits into
developfrom
feature/624-build-binder-oe-cross
Jul 5, 2026
Merged

fix(build): guard dev wrapper scripts against cross/OE env + document 3rd-party integration#628
Ulrond merged 7 commits into
developfrom
feature/624-build-binder-oe-cross

Conversation

@Ulrond

@Ulrond Ulrond commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Refs #624

Background

#624 reports build_binder.sh failing in an OpenEmbedded ARM cross environment two ways: the host AIDL tool gets cross-compiled (can't run on the build host), and OECORE_*_SYSROOT makes the binder CMake set BUILD_ENV_YOCTO=ON, which disables every install() rule so out/target ends up empty.

Both behaviours are correct for a production build invoked directly through CMake. The actual problem is that the build_*.sh wrapper scripts — which are documented as developer/architecture-only tools (BUILD.md: "Yocto/BitBake recipes must NOT use these wrapper scripts") — were run in an environment they were never meant for. The HAL libraries build from committed generated C++, so the AIDL host tool is never needed on a target at all.

So this is not a code adaptation bug — the fix is to enforce and document the boundary.

Changes

  • dev_env_guard.sh (new) — shared guard sourced by build_binder.sh, build_modules.sh, build_interfaces.sh. If it detects a cross CC (target triple ≠ host) or OECORE_*_SYSROOT, it aborts with a clear message pointing to the direct-CMake recipe. clean/help paths bypass it so they stay usable anywhere; BINDER_ALLOW_CROSS_ENV=1 overrides.
  • docs/standards/build_integration.md (new, wired into the nav) — the integration contract for third-party/Yocto build systems: two-stage build via direct CMake, required variables, BitBake recipe pattern, and why the wrapper scripts are not used.
  • README — production-build section now states the contract and links the new doc.

Testing

  • bash -n on all four scripts — clean.
  • Guard unit checks: native (no CC/OECORE) → allowed; OECORE_* set → blocked; cross CC → blocked; BINDER_ALLOW_CROSS_ENV=1 → allowed.
  • Wrapper smoke tests: build_modules.sh --help bypasses the guard (exit 0); build_modules.sh <module> and build_interfaces.sh sdk in a simulated OE env abort before doing any work with the pointer message.

Update: reference recipes added

Folded in copy-me integration recipes so the guide ships instructions + examples:

  • docs/standards/examples/rdk-halif-aidl.bb — BitBake reference recipe.
  • docs/standards/examples/rdk-halif-aidl.yaml — Bob Build Tool reference recipe.

Both are thin wrappers over the same direct-CMake invocation, demonstrating the build is build-system-agnostic. Linked from build_integration.md; marked as reference templates (pin to a release tag, adapt toolchain/sysroot), not CI-verified — I have no Yocto/Bob environment to execute-test them.

@Ulrond Ulrond added bug Something isn't working scope:infrastructure Repo tooling, CI/CD, scripts, governance docs documentation Surface-untouched change (docs/comments/metadata) — bumps bugfix; auto-applied for doc-only PRs labels Jun 22, 2026
Copilot AI review requested due to automatic review settings June 22, 2026 13:08
@github-project-automation github-project-automation Bot moved this to Architecture Review Required in halif_aidl Jun 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the boundary between developer-only wrapper scripts and production/cross build integrations by adding an explicit environment guard and documenting the supported third-party (Yocto/BitBake) CMake-direct build contract (refs #624).

Changes:

  • Added a shared dev_env_guard.sh and sourced it from build_binder.sh, build_modules.sh, and build_interfaces.sh to abort early in cross/OpenEmbedded environments (with an override env var).
  • Added new third-party integration documentation and linked it from the README and MkDocs navigation.
  • Updated wrapper scripts to allow help/clean paths without requiring a host toolchain (with one exception noted in comments).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
README.md Adds an explicit statement that production/cross builds must invoke CMake directly and links the new integration doc.
mkdocs.yml Wires the new “Third-Party Build Integration” doc into the published nav.
docs/standards/build_integration.md New guide describing the two-stage production build contract and BitBake patterns.
dev_env_guard.sh New shared guard that detects OE/cross environments and aborts wrapper scripts with guidance.
build_modules.sh Sources the guard for non-clean/help invocations to prevent misuse in cross/OE shells.
build_interfaces.sh Sources the guard for non-clean/help invocations to prevent misuse in cross/OE shells.
build_binder.sh Sources the guard early to prevent staging the SDK in cross/OE shells.

Comment thread build_binder.sh
Comment thread dev_env_guard.sh Outdated
Comment thread dev_env_guard.sh Outdated
Comment thread docs/standards/build_integration.md Outdated
Comment thread docs/standards/build_integration.md Outdated
Comment thread docs/standards/build_integration.md
@Ulrond
Ulrond requested a review from a team June 22, 2026 13:34
Ulrond added a commit that referenced this pull request Jun 22, 2026
Per Copilot review on #624/#628:
- dev_env_guard.sh: name the wrapper via BASH_SOURCE[1] (not $0), so the
  message is correct when a wrapper is sourced; point users at the in-tree
  docs/standards/build_integration.md instead of build-tools/.../BUILD.md
  (which is a gitignored clone, absent from a fresh checkout).
- build_binder.sh: clarify that 'clean' force-rebuilds here, so it is a build
  operation and is intentionally not exempt from the guard.
- build_integration.md: clarify that versions_released.yaml is consumed by
  build_modules.sh, not the top-level CMake (a cohort build iterates versions
  itself); link the upstream linux_binder_idl BUILD guide; replace the BitBake
  recipe's incorrect ${B}/out/target install path with the cmake class
  (EXTRA_OECMAKE + class-managed install).
@Ulrond

Ulrond commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed Copilot review feedback (pushed):

  • Guard $0 when sourced → now uses BASH_SOURCE[1], so the message names the actual wrapper.
  • Guard doc pointer / BUILD.md links → repointed from build-tools/linux_binder_idl/BUILD.md (a gitignored clone, absent from a fresh checkout) to the in-tree docs/standards/build_integration.md and the upstream linux_binder_idl BUILD guide.
  • versions_released.yaml wording → clarified it's consumed by build_modules.sh, not the top-level CMake; a cohort build iterates versions itself.
  • BitBake do_install path → replaced the incorrect ${B}/out/target copy with the cmake class (EXTRA_OECMAKE + class-managed install).
  • build_binder.sh clean not bypassing the guard → intentional and now documented: unlike the sibling scripts' pure clean, clean here means force rebuild, so it's a build op that correctly needs a host toolchain.

@Ulrond Ulrond self-assigned this Jun 22, 2026
Copilot AI review requested due to automatic review settings June 22, 2026 16:36
@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/rdk-halif-aidl/628/rdkcentral/rdk-halif-aidl

  • Commit: 9e78915

Report detail: gist'

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Comment thread build_modules.sh Outdated
Comment thread build_modules.sh
Comment thread build_interfaces.sh Outdated
Comment thread build_interfaces.sh
Comment thread docs/standards/examples/rdk-halif-aidl.bb Outdated
@Ulrond Ulrond moved this from Architecture Review Required to Review Requested in halif_aidl Jun 23, 2026
Ulrond added a commit that referenced this pull request Jun 24, 2026
…sistency

- Remove the Apache header from the reference recipes per @mhughesacn / RDK
  meta-layer convention (recipes are licensed via the repo, not per-file).
- Point the guard comments at docs/standards/build_integration.md (not BUILD.md).
- Drop the cross-compile/Yocto examples from build_modules.sh & build_interfaces.sh
  help text — the host-toolchain guard blocks that path; cross/Yocto use direct
  CMake (build_integration.md).
@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/rdk-halif-aidl/628/rdkcentral/rdk-halif-aidl

  • Commit: 4f96d07

Report detail: gist'

Copilot AI review requested due to automatic review settings June 24, 2026 08:00
@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/rdk-halif-aidl/628/rdkcentral/rdk-halif-aidl

  • Commit: aa23947

Report detail: gist'

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread docs/standards/examples/rdk-halif-aidl.bb Outdated
Comment thread build_interfaces.sh
@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

  • Upvote reason: Thanks Gerald

  • Commit: aa23947
    '

@Ulrond Ulrond added this to the Next Release milestone Jul 3, 2026
@Ulrond Ulrond added Minor Change Additive, backwards-compatible interface change — bumps minor; the default for real work and removed documentation Surface-untouched change (docs/comments/metadata) — bumps bugfix; auto-applied for doc-only PRs labels Jul 4, 2026
Ulrond added a commit that referenced this pull request Jul 4, 2026
Copilot review: 'SRCREV = "0.21.0"' is misleading — BitBake SRCREV must be a
git commit SHA, not a tag name. Show the correct form: reference the tag in
SRC_URI (;tag=0.21.0) and set SRCREV to that tag's commit SHA.
Copilot AI review requested due to automatic review settings July 4, 2026 08:14
@Ulrond

Ulrond commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

Copilot review: fixed the SRCREV pinning example (SHA, not tag name). The build_interfaces.sh debug-flags example thread is outdated (line since changed) — resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread dev_env_guard.sh Outdated
Ulrond added a commit that referenced this pull request Jul 5, 2026
…iew)

Copilot review: the cross-toolchain heuristic derived host_machine from a
PATH-resolved cc/gcc, which a cross toolchain that prepends PATH could point
at the cross compiler — masking the host triple so the triple-mismatch check
wouldn't fire. Prefer /usr/bin/gcc|cc (fixed paths, immune to PATH prepend),
falling back to PATH cc/gcc only when absent. (OE shells are still primarily
caught by the OECORE_*_SYSROOT check above.)
Ulrond added 7 commits July 5, 2026 09:36
…3rd-party integration

build_binder.sh / build_modules.sh / build_interfaces.sh are developer and
architecture-team tools that assume a native host toolchain. Run inside an
OpenEmbedded cross shell they fail silently: the host AIDL tool is cross-
compiled (can't run on the build host) and the binder CMake auto-enables
BUILD_ENV_YOCTO from OECORE_*, disabling install() so out/target is empty.

That CMake behaviour is correct for production builds invoked directly; the
bug is only that the dev scripts were run in an environment they don't support.
Add a shared dev_env_guard.sh that fails fast (with a pointer to the direct-
CMake recipe) when a cross CC or OECORE_*_SYSROOT is detected; clean/help
paths stay usable, and BINDER_ALLOW_CROSS_ENV=1 overrides.

Document the contract: new docs/standards/build_integration.md (required CMake
variables, BitBake recipe pattern, why not the scripts) wired into the nav, and
a pointer from the README production-build section.

Refs #624
Per Copilot review on #624/#628:
- dev_env_guard.sh: name the wrapper via BASH_SOURCE[1] (not $0), so the
  message is correct when a wrapper is sourced; point users at the in-tree
  docs/standards/build_integration.md instead of build-tools/.../BUILD.md
  (which is a gitignored clone, absent from a fresh checkout).
- build_binder.sh: clarify that 'clean' force-rebuilds here, so it is a build
  operation and is intentionally not exempt from the guard.
- build_integration.md: clarify that versions_released.yaml is consumed by
  build_modules.sh, not the top-level CMake (a cohort build iterates versions
  itself); link the upstream linux_binder_idl BUILD guide; replace the BitBake
  recipe's incorrect ${B}/out/target install path with the cmake class
  (EXTRA_OECMAKE + class-managed install).
Fold copy-me integration recipes into the third-party build guide so it ships
instructions AND examples:
- docs/standards/examples/rdk-halif-aidl.bb   — BitBake reference recipe
- docs/standards/examples/rdk-halif-aidl.yaml — Bob Build Tool reference recipe

Both are thin wrappers over the same direct-CMake invocation (BINDER_SDK_DIR /
BINDER_SDK_INCLUDE_DIR / INTERFACE_TARGET), demonstrating that the build is
build-system-agnostic. Linked from build_integration.md; marked as reference
templates (pin to a release tag, adapt toolchain/sysroot), not CI-verified.
…sistency

- Remove the Apache header from the reference recipes per @mhughesacn / RDK
  meta-layer convention (recipes are licensed via the repo, not per-file).
- Point the guard comments at docs/standards/build_integration.md (not BUILD.md).
- Drop the cross-compile/Yocto examples from build_modules.sh & build_interfaces.sh
  help text — the host-toolchain guard blocks that path; cross/Yocto use direct
  CMake (build_integration.md).
…rence

Reframe the production section as 'Consuming the interfaces (build with CMake
directly)' and add a switch table (INTERFACE_TARGET / AIDL_SRC_VERSION /
BINDER_SDK_DIR / BINDER_SDK_INCLUDE_DIR) so integrators select what to build via
-D switches rather than the wrapper scripts. Mark the Scripts section as
developer/architecture-team-only (refuse cross/OE). Replace the two dead
TWO_STAGE_BUILD.md links with docs/standards/build_integration.md.
Copilot review: 'SRCREV = "0.21.0"' is misleading — BitBake SRCREV must be a
git commit SHA, not a tag name. Show the correct form: reference the tag in
SRC_URI (;tag=0.21.0) and set SRCREV to that tag's commit SHA.
…iew)

Copilot review: the cross-toolchain heuristic derived host_machine from a
PATH-resolved cc/gcc, which a cross toolchain that prepends PATH could point
at the cross compiler — masking the host triple so the triple-mismatch check
wouldn't fire. Prefer /usr/bin/gcc|cc (fixed paths, immune to PATH prepend),
falling back to PATH cc/gcc only when absent. (OE shells are still primarily
caught by the OECORE_*_SYSROOT check above.)
Copilot AI review requested due to automatic review settings July 5, 2026 08:39
@Ulrond
Ulrond force-pushed the feature/624-build-binder-oe-cross branch from 5ac42a0 to 78654ee Compare July 5, 2026 08:39
@Ulrond
Ulrond merged commit 0ea237c into develop Jul 5, 2026
1 check passed
@Ulrond
Ulrond deleted the feature/624-build-binder-oe-cross branch July 5, 2026 08:39
@github-project-automation github-project-automation Bot moved this from Review Requested to Resolved in halif_aidl Jul 5, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working Minor Change Additive, backwards-compatible interface change — bumps minor; the default for real work scope:infrastructure Repo tooling, CI/CD, scripts, governance docs

Projects

Status: Resolved

Development

Successfully merging this pull request may close these issues.

4 participants