[buildmgr] Enforce toolchain registration environment variables

[brondani]

No description provided.

[github-actions]

Test Results

  7 files   53 suites   4m 41s ⏱️
185 tests 168 ✅ 17 💤 0 ❌
692 runs  624 ✅ 68 💤 0 ❌

Results for commit e816562.

♻️ This comment has been updated with latest results.

[brondani]

1) Findings (ordered by severity)

High — Incorrect/fragile CI toolchain env var wiring

• File path: .github/workflows/buildmgr.yml (both edited env blocks)
• Why it matters:
  New required vars are set to ${{ matrix.toolchain_root }} for both GCC and AC6. In jobs/matrix entries where matrix.toolchain_root is absent or points to only one toolchain, the other env var is wrong or empty. This can cause non-deterministic failures (or false registration with wrong path).
• Concrete fix guidance:
  • Define toolchain-specific matrix keys (for example gcc_root, ac6_root) and map each env var explicitly.
  • Avoid referencing undefined matrix keys in shared job env.
  • Add a guard step that fails fast if required env vars are empty for the active toolchain.

---

High — Default linker behavior changed unexpectedly across compilers

• File path: tools/projmgr/templates/cdefault.yml
• Why it matters:
  This PR also changes runtime/link behavior:
  • GCC: adds --specs=nosys.specs
  • CLANG: removes semihost libs, adds -lcrt0
  • IAR: removes --semihosting
    These are functional behavior changes unrelated to env-var enforcement and can break debug/runtime expectations or host I/O behavior.
• Concrete fix guidance:
  • Move these template changes to a separate PR with explicit rationale and compatibility notes.
  • If intentional here, gate by profile/target (for example “baremetal” vs “semihosting”) instead of changing defaults globally.
  • Add migration notes and example overrides in docs.

---

Medium — Potential stale model state after toolchain resolution failure

• File path: tools/buildmgr/cbuild/src/CbuildModel.cpp (GetCompatibleToolchain)
• Why it matters:
  On failure (selectedVersion.empty()), function returns false without explicitly clearing prior resolved members (m_toolchainRegisteredVersion, m_toolchainConfig, m_toolchainConfigVersion). Reusing the same model instance may leave stale values visible to later logic.
• Concrete fix guidance:
  • Before returning false, clear all toolchain-resolution outputs:
    • m_toolchainRegisteredVersion.clear();
    • m_toolchainConfig.clear();
    • m_toolchainConfigVersion.clear();
  • Keep this invariant: failed resolution leaves model in “no resolved toolchain” state.

---

Medium — Toolchain config robustness reduced by removing local defaults

• File paths:
  • tools/buildmgr/cbuildgen/config/AC6.6.16.2.cmake
  • tools/buildmgr/cbuildgen/config/CLANG.17.0.1.cmake
  • tools/buildmgr/cbuildgen/config/GCC.10.3.1.cmake
  • tools/buildmgr/cbuildgen/config/IAR.9.32.1.cmake
• Why it matters:
  Configs now unconditionally set TOOLCHAIN_ROOT/TOOLCHAIN_VERSION from registered vars. If those vars are missing (manual use, older generators, partial integration), values become empty and can fail later with less clear diagnostics.
• Concrete fix guidance:
  • Add explicit validation in each config:
    • if(NOT DEFINED REGISTERED_TOOLCHAIN_ROOT OR REGISTERED_TOOLCHAIN_ROOT STREQUAL "") message(FATAL_ERROR "...") endif()
    • same for REGISTERED_TOOLCHAIN_VERSION.
  • Keep strong failure mode but with immediate, precise error.

---

2) Suggested fixes per finding

1. Workflow wiring
   • Split env mapping per toolchain/matrix entry.
   • Add preflight shell step validating non-empty expected vars.
2. Template linker defaults
   • Revert in this PR or isolate behind selectable profiles.
   • Document behavior changes and compatibility impact.
3. Model state clearing
   • Clear resolved toolchain fields on all failure exits.
   • Add assertion-style unit coverage for state invariants.
4. Config validation
   • Add explicit CMake FATAL_ERROR checks for missing registration vars.
   • Keep messages aligned with M616 naming format.

---

3) Test gaps

• Missing unit test for reused CbuildModel instance: success call followed by failure call should leave no stale toolchain data.
• Missing CI/integration tests for each supported compiler registration var (GCC/AC6/CLANG/IAR).
• Missing negative tests for invalid env var path/version (empty, non-existent directory, malformed version).
• Missing regression tests for cdefault.yml runtime/link behavior changes (semihosting vs nosys outcomes).

---

4) Final summary

The PR correctly moves toward strict env-var-based toolchain registration, but it introduces high-risk regressions in CI env mapping and default linker behavior. There are also robustness gaps in failure-state handling and config-level validation. Address the two high-severity items before merge, then add targeted tests for failure paths and multi-toolchain coverage.