Skip to content

fix(build): flexible binder header path for Yocto/sysroot (#644) - #670

Merged
Ulrond merged 4 commits into
developfrom
feature/644-flexible-binder-include
Jul 5, 2026
Merged

fix(build): flexible binder header path for Yocto/sysroot (#644)#670
Ulrond merged 4 commits into
developfrom
feature/644-flexible-binder-include

Conversation

@Ulrond

@Ulrond Ulrond commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Closes #644. Sibling of #635 (both Yocto CMake portability; different root cause).

Problem

Each component's CMakeLists.txt resolved binder headers only under a fixed include/binder_sdk subdir:

set(_BINDER_INC "${BINDER_SDK_DIR}/include/binder_sdk")
if (NOT IS_DIRECTORY ...) ... "${BINDER_SDK_INCLUDE_DIR}/include/binder_sdk" ... else() FATAL

Yocto/cross builds stage headers flat (<sysroot>/usr/include/...), so they hit the FATAL. The reporter's proposed fix (treat BINDER_SDK_INCLUDE_DIR as a direct path) would have broken local dev (which needs the subdir).

Fix

A candidate list — first existing dir wins, dev layout tried first (no regression):

foreach(_cand
    "${BINDER_SDK_DIR}/include/binder_sdk"          # local dev
    "${BINDER_SDK_INCLUDE_DIR}/include/binder_sdk"  # dev split
    "${BINDER_SDK_INCLUDE_DIR}"                     # Yocto: direct include dir
    "${BINDER_SDK_DIR}/include/binder")             # alt sysroot name
  if (IS_DIRECTORY "${_cand}") set(_BINDER_INC "${_cand}") break() endif()
endforeach()
if (_BINDER_INC STREQUAL "") message(FATAL_ERROR ...) endif()

Applied across 52 CMakeLists.txt — all component current/ templates and the committed <version>/ snapshots. Snapshots are build-config only (the .hash covers the AIDL contract, not CMakeLists.txt; precedent: #629), so this doesn't touch any frozen contract.

Verified

  • Synthetic CMake test: local-dev subdir → resolves subdir; Yocto flat → resolves direct dir; nothing set → FATAL.
  • Real ./build_modules.sh common still builds (libcommon-vcurrent-cpp.so).

The component CMakeLists resolved binder headers only under a fixed
'include/binder_sdk' subdir, so Yocto/cross builds (headers staged flat under
<sysroot>/usr/include) failed. Replace the rigid 2-candidate check with a
candidate list that also accepts BINDER_SDK_INCLUDE_DIR as a direct include
dir; the local-dev subdir layout is tried first, so existing builds are
unaffected.

Applied across all component current/ templates AND the committed <version>/
snapshots (build config only — the .hash covers the AIDL contract, not the
CMakeLists; precedent: #629). 52 CMakeLists.txt updated.

Verified: local-dev subdir + Yocto flat both resolve; missing headers still
FATAL; a real 'build_modules.sh common' still builds. Sibling of #635.
Copilot AI review requested due to automatic review settings July 3, 2026 08:57
@github-project-automation github-project-automation Bot moved this to Architecture Review Required in halif_aidl Jul 3, 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 updates per-module CMake build logic to resolve Binder header include roots flexibly across local development layouts and Yocto/sysroot layouts, addressing Yocto build failures reported in #644 without regressing local builds.

Changes:

  • Replaced the fixed include/binder_sdk include-root assumption with a “first existing candidate wins” search across multiple possible header layouts.
  • Applied the updated Binder include-root resolution across both current/ module templates and versioned snapshot CMakeLists.txt files.

Reviewed changes

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

Show a summary per file
File Description
indicator/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
indicator/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
hdmiinput/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
hdmiinput/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
avclock/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
avclock/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
avclock/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
common/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
common/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
common/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
hdmicec/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
hdmicec/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
videosink/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
videosink/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
videosink/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiosink/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiosink/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiosink/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
firmwareupdate/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
firmwareupdate/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
deepsleep/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
deepsleep/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
sensor/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
sensor/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
sensor/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
hdmioutput/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
hdmioutput/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiodecoder/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiodecoder/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiodecoder/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
panel/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
panel/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
bootreason/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
bootreason/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
drm/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
drm/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
videodecoder/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
videodecoder/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
videodecoder/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiomixer/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiomixer/0.1.0.1/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiomixer/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
audiomixer/0.3.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
compositeinput/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
compositeinput/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
planecontrol/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
planecontrol/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
avbuffer/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
avbuffer/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
avbuffer/0.2.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
deviceinfo/current/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts
deviceinfo/0.1.0.0/CMakeLists.txt Binder include-root resolution updated to support Yocto/sysroot layouts

Comment thread common/current/CMakeLists.txt
Comment thread common/0.1.0.0/CMakeLists.txt
Tasks 1-4 stage headers under include/binder_sdk (dev subdir). Add Task 5: a
flat Yocto-style sysroot (headers directly under <sysroot>/usr/include) and a
configure-only check that a component (common) resolves them via
BINDER_SDK_INCLUDE_DIR as a direct include dir. Absence of the 'Binder SDK
headers not found' FATAL == resolved.

Verified: passes with the #644 fix; FATALs against develop's old component
CMake — so it regression-tests the flexible-include fix.
@Ulrond
Ulrond requested a review from a team July 3, 2026 10:47
@Ulrond Ulrond self-assigned this Jul 3, 2026
@Ulrond Ulrond moved this from Architecture Review Required to Review Requested in halif_aidl Jul 3, 2026
@Ulrond Ulrond moved this from Review Requested to Under Review in halif_aidl Jul 4, 2026
@Ulrond Ulrond added Minor Change Additive, backwards-compatible interface change — bumps minor; the default for real work scope:infrastructure Repo tooling, CI/CD, scripts, governance docs labels Jul 4, 2026
…review)

Copilot review: the candidate list could pick an unintended include root —
when BINDER_SDK_INCLUDE_DIR is unset, "${BINDER_SDK_INCLUDE_DIR}/include/binder_sdk"
expands to an absolute "/include/binder_sdk" that might match a host path; and
"${BINDER_SDK_DIR}/include/binder" is not a valid include root for <binder/...>.

Now each candidate is only added when its base variable is set (via list(APPEND)
guarded by if(BINDER_SDK_DIR)/if(BINDER_SDK_INCLUDE_DIR)), and the bogus
.../include/binder candidate is corrected to .../include (the actual root under
which binder/ lives). Priority order preserved. Applied across all 52 component
CMakeLists (templates + snapshots).
Copilot AI review requested due to automatic review settings July 4, 2026 08:19

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 53 out of 53 changed files in this pull request and generated 2 comments.

Comment thread common/current/CMakeLists.txt
Comment thread tests/fake-yocto/run-fake-yocto.sh
Copilot review: the bare ${BINDER_SDK_INCLUDE_DIR} candidate could match the
Yocto staging *prefix* (e.g. /usr) — which exists but is not a valid include
root — and the fake-yocto test passed BINDER_SDK_INCLUDE_DIR as a direct
include dir, not the prefix the README documents.

Unify on 'BINDER_SDK_INCLUDE_DIR = staging prefix': replace the bare candidate
with ${BINDER_SDK_INCLUDE_DIR}/include across all 52 component CMakeLists, so
both the namespaced (prefix/include/binder_sdk) and real-Yocto flat
(prefix/include, binder/ directly) layouts resolve. Fix the fake-yocto Task 5
to pass the prefix (mirroring the README). Verified: fake-yocto passes.
@Ulrond
Ulrond merged commit 61649c4 into develop Jul 5, 2026
4 checks passed
@Ulrond
Ulrond deleted the feature/644-flexible-binder-include branch July 5, 2026 08:30
@github-project-automation github-project-automation Bot moved this from Under Review to Resolved in halif_aidl Jul 5, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 5, 2026
@Ulrond Ulrond added documentation Surface-untouched change (docs/comments/metadata) — bumps bugfix; auto-applied for doc-only PRs and removed Minor Change Additive, backwards-compatible interface change — bumps minor; the default for real work labels Jul 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Surface-untouched change (docs/comments/metadata) — bumps bugfix; auto-applied for doc-only PRs scope:infrastructure Repo tooling, CI/CD, scripts, governance docs

Projects

Status: Resolved

Development

Successfully merging this pull request may close these issues.

CMake improvement: Allow flexible binder include path

2 participants