Skip to content

fix(ci): force clang for Linux x86_64 native builds (#1054)#1059

Merged
carlos-alm merged 1 commit into
mainfrom
fix/issue-1054-rust-build-c-compiler
May 4, 2026
Merged

fix(ci): force clang for Linux x86_64 native builds (#1054)#1059
carlos-alm merged 1 commit into
mainfrom
fix/issue-1054-rust-build-c-compiler

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • The Linux x86_64 native binary built in CI silently drops the 4 HCL fixture files in `tests/benchmarks/resolution/fixtures/hcl/`. The drop triggers an engine-parity WASM backfill on every full `buildGraph` call, which the pre-publish gate detects as a perf regression (perf(native): ~2 second flat overhead added to rebuild operations in v3.10.0 #1054).
  • Verified locally that this is a C compiler issue — same Rust source, same Cargo.lock, same toolchain version: gcc 14.2.0 builds a dropping binary; clang 14 builds a working one. The bundled parser.c in tree-sitter-hcl is what `cc-rs` compiles via `CC`, and gcc apparently optimizes some part of it differently.
  • Sets `CC=clang`/`CXX=clang++` for the three places that build the host x86_64-linux-gnu artifact: `publish.yml` `build-native` matrix, `publish.yml` `preflight` job, `build-native.yml`, and `ci.yml`'s ubuntu-latest leg.

Why clang specifically

Reproduced both sides locally in Docker:

Compiler Image Binary parses HCL? noopRebuild
gcc 14.2.0 node:22-trixie ❌ drops 4 .tf files (warning fires)
clang 14 node:22-bookworm ✅ parses cleanly clean

Same Rust source (current main), same `napi build --release`, only `CC` differs.

Scope

  • Cross-compile targets (`aarch64-unknown-linux-gnu`, `x86_64-unknown-linux-musl`) are unaffected — they already pin their own toolchains via separate steps.
  • macOS and Windows builds are unaffected.
  • End users on Linux x86_64 GNU now get the clang-built binary, which fixes the silent HCL drop they would also have hit had they happened to install v3.10.0.

Companion PR

#1058 makes the WASM backfill cheap when it does fire (defense-in-depth). This PR removes the trigger condition. With both, `Native orchestrator dropped 4 file(s)` should disappear from gate logs entirely.

Test plan

  • Local Docker: gcc-14 build → drops files; clang build → no drops.
  • Pre-publish benchmark gate runs cleanly on dispatch.
  • CI `native-host-build` and parity tests pass on the new clang-built binary.

Refs #1054

Verified locally: gcc 14.2.0 (Debian Trixie) builds a binary whose
tree-sitter-hcl extractor silently drops the 4 HCL fixture files in
tests/benchmarks/resolution/fixtures/hcl/. Clang 14 (Debian Bookworm)
builds a binary that parses them correctly. Same Rust source, same
Cargo.lock — only the C compiler differs (cc-rs uses CC for the
bundled parser.c).

The drop triggers an engine-parity WASM backfill on every full-build
call (#1054), and the v3.10.0 pre-publish gate flagged the resulting
~1.7s overhead per call as a regression vs the v3.9.6 published binary
(which evidently was built before this gcc/parser-c interaction
manifested).

Forces `CC=clang`/`CXX=clang++` in three places that build the host
x86_64-unknown-linux-gnu artifact:

- publish.yml `build-native` matrix entry for the gate's binary
- publish.yml `preflight` job (so npm test parity matches published)
- build-native.yml's matching matrix entry (host build for PR checks)
- ci.yml `native-host-build` ubuntu-latest leg (so PR test runs match)

Cross-compile targets (aarch64-gnu, x86_64-musl) are unaffected — they
already pin their own toolchains. macOS and Windows are unaffected.

Refs #1054
@claude
Copy link
Copy Markdown

claude Bot commented May 4, 2026

Claude finished @carlos-alm's task —— View job


PR Review: fix(ci): force clang for Linux x86_64 native builds

Todo List

  • Read and analyze changed workflow files
  • Perform root cause analysis
  • Check for configuration change risks
  • Evaluate test integrity and approach
  • Assess backlog compliance
  • Provide final review recommendation

Root Cause Analysis

Problem Being Solved: The Linux x86_64 native binary built with gcc 14.2.0 silently drops 4 HCL fixture files in tests/benchmarks/resolution/fixtures/hcl/. This triggers WASM backfill on every build call, causing performance regression detected by pre-publish gates.

Why This Approach: The PR switches from gcc to clang for x86_64-linux-gnu builds based on local Docker verification showing gcc-built binaries drop .tf files while clang-built ones work correctly. This is a C compiler optimization issue in tree-sitter-hcl's bundled parser.c that cc-rs compiles.

Risk Assessment: Low risk. The change is targeted to the specific problematic target (x86_64-unknown-linux-gnu) and has been verified locally. Cross-compile targets already use their own toolchains and are unaffected.

Configuration Change Detection (LOW RISK)

This PR makes targeted CI configuration changes to address a specific C compiler bug:

  1. build-native.yml: Adds clang for matrix.target == 'x86_64-unknown-linux-gnu' only
  2. ci.yml: Adds clang for matrix.os == 'ubuntu-latest' in native-host-build job
  3. publish.yml: Adds clang in two places - preflight job (all Linux) and build-native matrix (x86_64-linux-gnu only)

No rules relaxed or thresholds lowered - this fixes a genuine compiler-level bug
Surgical scope - only affects specific Linux x86_64 targets, preserves existing cross-compile toolchains
Consistent application - applied to all jobs that build the affected target

Test Integrity (EXCELLENT)

This PR follows the project's strict engine parity policy - per CLAUDE.md: "Both engines must produce identical results. If they diverge, the less-accurate engine has a bug — fix it, don't document the gap"

Addresses root cause, not symptoms - fixes the gcc-compiled native engine to match WASM behavior, rather than documenting the divergence as acceptable

Backed by local verification - author reproduced issue in Docker with both gcc 14.2.0 (drops files) and clang 14 (works correctly)

Parity tests will validate the fix - existing tests/engines/parity.test.ts compares WASM vs native extraction of HCL files

Backlog Compliance

  • Zero-dep: ✓ - No new runtime dependencies, uses existing clang in CI runners
  • Foundation-aligned: ✓ - Maintains dual-engine architecture principle, fixes engine parity bug
  • Problem-fit: 4/5 - Directly prevents native/WASM engine divergence that would confuse AI agents with inconsistent results
  • Breaking: No - Pure CI change, no API or behavior changes for end users
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Critical Concerns

None identified. This is a well-researched fix for a compiler-specific bug.

Final Recommendation

  • Rating: ⭐⭐⭐⭐⭐ (5/5)
  • Action: APPROVE
  • Reasoning: Exemplary PR that identifies root cause, provides targeted fix, maintains project principles, and includes proper verification. The compiler switch is well-justified and addresses a genuine engine parity bug that would degrade AI agent experience.

Minor suggestion: Consider adding the companion PR #1058 mentioned in the description to make WASM backfill cheaper as defense-in-depth, but this PR stands on its own merit.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR fixes a silent HCL file drop in the Linux x86_64 native binary by forcing CC=clang/CXX=clang++ ahead of the napi build step in all four relevant CI jobs (build-native.yml, ci.yml native-host-build, publish.yml preflight, and publish.yml build-native). The fix is well-scoped: each job correctly guards the clang step to Linux x86_64 targets only (or omits the guard where the job is already ubuntu-only), and a clang --version preflight check ensures the step fails loudly if clang is absent rather than silently falling back to gcc.

Confidence Score: 5/5

Safe to merge — changes are limited to CI workflow files, correctly scoped to Linux x86_64 targets, and carry no risk of regression on other platforms.

No P0 or P1 findings. All four affected jobs are correctly guarded (or need no guard). The clang --version preflight check provides an explicit failure signal if clang is unavailable rather than silently falling back to gcc. Cross-compile and non-Linux targets are unaffected.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/build-native.yml Adds clang step guarded by matrix.target == 'x86_64-unknown-linux-gnu'; correctly placed before cross-compile tool steps; sanity-checks clang --version before setting env vars.
.github/workflows/ci.yml Adds clang step guarded by matrix.os == 'ubuntu-latest' for the native-host-build job; pattern is consistent with the other workflows.
.github/workflows/publish.yml Adds clang step in both preflight (no if guard needed — job is ubuntu-only) and build-native matrix (guarded by matrix.target == 'x86_64-unknown-linux-gnu'); correctly mirrors build-native.yml.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI / publish triggered] --> B{OS?}
    B -->|ubuntu-latest| C[Run: clang --version\nSet CC=clang / CXX=clang++]
    B -->|macOS / Windows| D[Use default toolchain\nNo CC override]
    C --> E{Target?}
    E -->|x86_64-unknown-linux-gnu| F[napi build --release\nwith clang]
    E -->|aarch64-unknown-linux-gnu| G[Set CC=aarch64-linux-gnu-gcc\nnapi build --release cross]
    E -->|x86_64-unknown-linux-musl| H[Set CC=musl-gcc\nnapi build --release cross]
    F --> I[✅ HCL parser works\nno file drops]
    G --> J[✅ Not affected\nown toolchain]
    H --> K[✅ Not affected\nown toolchain]
    D --> L[macOS/Windows build\nnot affected]
Loading

Reviews (1): Last reviewed commit: "fix(ci): force clang for Linux x86_64 na..." | Re-trigger Greptile

@carlos-alm carlos-alm merged commit 1e292ef into main May 4, 2026
23 checks passed
@carlos-alm carlos-alm deleted the fix/issue-1054-rust-build-c-compiler branch May 4, 2026 09:59
@github-actions github-actions Bot locked and limited conversation to collaborators May 4, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant