Skip to content

BUILD-11297 Simplify CI_METRICS_ENABLED gate — strict 'true', no bash step#68

Merged
julien-carsique-sonarsource merged 1 commit into
masterfrom
fix/jcarsique/BUILD-11297-macos-bash
May 19, 2026
Merged

BUILD-11297 Simplify CI_METRICS_ENABLED gate — strict 'true', no bash step#68
julien-carsique-sonarsource merged 1 commit into
masterfrom
fix/jcarsique/BUILD-11297-macos-bash

Conversation

@julien-carsique-sonarsource
Copy link
Copy Markdown
Contributor

@julien-carsique-sonarsource julien-carsique-sonarsource commented May 19, 2026

Summary

Fix a regression introduced by #62 (BUILD-11294 — cache metrics): the metrics-flag shell step crashed on macOS runners with bad substitution because ${VAR,,} lowercase substitution is bash 4+ only (Apple ships bash 3.2). The crash cascaded into a template-validity error for downstream build-maven if: expressions in consumer workflows.

Observed in: sonar-dummy#592 Build macOS, run 26091741374, step 5 line 884.

Fix

Drop the bash step entirely; check env.CI_METRICS_ENABLED == 'true' directly in the cache-metrics-prep step's if: expression.

  • No more bash — zero portability surface across runner platforms.
  • Strict literal 'true' — matches the convention of the existing env vars in this action (CACHE_BACKEND, CACHE_IMPORT_GITHUB). The earlier case-insensitive logic (${VAR,,} == 'true' accepting True / TRUE / true) was an artifact of bash 4 only; GHA expressions don't have tolower, so encoding the same semantic in YAML would require listing variants explicitly. Strict 'true' is simpler and consistent.
  • Linux-only scope preserved — the combined if: runner.os == 'Linux' && env.CI_METRICS_ENABLED == 'true' keeps the metrics flow Linux-only. The downstream cache-metrics step continues to chain on steps.cache-metrics-prep.outputs.prepared == 'true', so when the flag is unset or non-'true' the entire pipeline is skipped without warning.

Why ship this now (separate from M2 macOS compliance)

The metrics-flag bash step ran unconditionally on every consumer call to gh-action_cache@master. On macOS, bash 3.2 errored on ${RAW,,} regardless of whether CI_METRICS_ENABLED was set — i.e. this broke the base cache flow on macOS for everyone, not just metrics-pilot repos. Restoring base-flow correctness is independent of (and prerequisite to) the M2 work that will add macOS metrics emission.

Behavior on non-Linux after this fix

Test plan

  • Vitest suite green (48 cases)
  • All lines in action.yml, cache-metrics/action.yml ≤ 140 cols
  • Markdownlint clean on README
  • CI on this branch
  • Consumer dogfood: sonar-dummy macOS job no longer crashes

Related

@julien-carsique-sonarsource julien-carsique-sonarsource force-pushed the fix/jcarsique/BUILD-11297-macos-bash branch from 541f4d9 to facf38c Compare May 19, 2026 13:04
@julien-carsique-sonarsource julien-carsique-sonarsource changed the title BUILD-11297 Fix macOS bash 3.2 incompatibility in CI_METRICS_ENABLED resolution BUILD-11297 Simplify CI_METRICS_ENABLED gate — strict 'true', no bash step May 19, 2026
@julien-carsique-sonarsource julien-carsique-sonarsource force-pushed the fix/jcarsique/BUILD-11297-macos-bash branch from facf38c to 5674b1f Compare May 19, 2026 13:16
… step

Drop the dedicated `metrics-flag` shell step and check `env.CI_METRICS_ENABLED == 'true'` directly in the
`cache-metrics-prep` step's `if:` expression. This:

* Removes the bash 3.2 incompatibility that crashed macOS runners with `bad substitution` on `${RAW,,}`
  (observed in SonarSource/sonar-dummy#592 Build macOS, run 26091741374), which also cascaded into a
  template-validity error for downstream `build-maven` `if:` expressions.
* Removes the case-insensitive variant matching. Accepting only the literal `'true'` matches the convention
  of the existing env vars in this action (`CACHE_BACKEND`, `CACHE_IMPORT_GITHUB`) and avoids the GHA-
  expression equivalent of bash 4-only `${VAR,,}` (no `tolower` in GHA expressions).
* Eliminates the bash step entirely, so there's no portability surface to worry about across runner
  platforms.

The `cache-metrics-prep` step's combined `if: runner.os == 'Linux' && env.CI_METRICS_ENABLED == 'true'`
keeps the Linux-only scope. The downstream `cache-metrics` step continues to chain on
`steps.cache-metrics-prep.outputs.prepared == 'true'`, so when the flag is unset or non-`true` the entire
metrics flow is skipped without warning, matching the pre-#62 behaviour for non-pilot consumers.

README updated to document the strict `true` requirement instead of "case-insensitive".

Surfaced during the BUILD-11297 pilot rollout on sonar-dummy.
@sonarqubecloud
Copy link
Copy Markdown

@sonarqube-cloud-us
Copy link
Copy Markdown

@julien-carsique-sonarsource julien-carsique-sonarsource marked this pull request as ready for review May 19, 2026 13:20
@julien-carsique-sonarsource julien-carsique-sonarsource requested a review from a team as a code owner May 19, 2026 13:20
@sonar-review-alpha
Copy link
Copy Markdown

sonar-review-alpha Bot commented May 19, 2026

Summary

Problem: The bash 4+ feature ${VAR,,} (lowercase) in the "Resolve CI_METRICS_ENABLED flag" step crashes on macOS runners (bash 3.2), breaking the entire cache flow with a "bad substitution" error.

Solution: Eliminate the bash step entirely. Evaluate CI_METRICS_ENABLED directly in the downstream if: expression with a strict literal 'true' check. This removes the macOS portability issue while preserving the Linux-only metrics gate (runner.os == 'Linux').

Scope of changes:

  • action.yml: Removed 12-line bash step, changed if: condition from steps.metrics-flag.outputs.enabled == 'true' to env.CI_METRICS_ENABLED == 'true'
  • README.md: Updated CI_METRICS_ENABLED docs to specify strict 'true' literal (removed "case-insensitive" language)
  • cache-metrics/action.yml: Comment formatting only
  • All changes kept to ≤140 character line length per existing project convention

What reviewers should know

Key review points:

  1. The condition change is the crux — verify the if: runner.os == 'Linux' && env.CI_METRICS_ENABLED == 'true' logic matches the intended behavior. The step now runs when both: (a) runner is Linux, and (b) the env var is the literal string 'true'. Unset or any other value skips it silently (fail-open).

  2. Documentation change is semantic, not just cosmetic — the README now documents strict 'true' matching instead of case-insensitive. This is consistent with the behavior change and matches how other env vars in this action (CACHE_BACKEND, CACHE_IMPORT_GITHUB) work.

  3. Behavior preservation — non-Linux runners with CI_METRICS_ENABLED=true still skip metrics (no change from before #62). The next step's chaining on steps.cache-metrics-prep.outputs.prepared == 'true' ensures graceful skip when the condition is false or when the step itself fails.

  4. No functional change on Linux — Linux runners with the flag set to 'true' behave identically; only the path to evaluate the condition is different (expression vs. bash).

Testing impact: Vitest suite passes; CI/consumer dogfood (sonar-dummy macOS job) is the real validation.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

Copy link
Copy Markdown

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

Straightforward and correct fix. The removed metrics-flag step has no remaining references anywhere in the repo. The downstream "Emit cache metrics" step still chains on steps.cache-metrics-prep.outputs.prepared == 'true' (unchanged), keeping the fail-open behaviour intact. The env.CI_METRICS_ENABLED expression context is the same GHA env context that the old bash step already used (env: RAW: ${{ env.CI_METRICS_ENABLED }}), so variable resolution is equivalent — only the case-insensitivity is lost, which is intentional and fully documented. All "case-insensitive" language is consistently removed from the README with no stale mentions remaining.

🗣️ Give feedback

@julien-carsique-sonarsource julien-carsique-sonarsource merged commit c285839 into master May 19, 2026
25 checks passed
@julien-carsique-sonarsource julien-carsique-sonarsource deleted the fix/jcarsique/BUILD-11297-macos-bash branch May 19, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants