Skip to content

revert(ci): undo "force clang for Linux x86_64 native builds" (#1059)#1061

Merged
carlos-alm merged 2 commits into
mainfrom
revert/1059-clang
May 4, 2026
Merged

revert(ci): undo "force clang for Linux x86_64 native builds" (#1059)#1061
carlos-alm merged 2 commits into
mainfrom
revert/1059-clang

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

Reverts #1059 (1e292ef). The premise was that switching CC to clang prevented the HCL .tf drops in #1054. After tighter verification it's clear that both gcc and clang produce binaries that drop .tf files — the earlier observation that "clang doesn't drop" was an output-filtering artifact in my local benchmark grep, not a real behavior difference.

What's actually going on

The real cause is a tree-sitter ABI mismatch (`tree-sitter-hcl` 1.1.0 ships ABI 15; runtime is pinned at `tree-sitter = "0.24"` / ABI 14). `Parser::set_language` rejects the grammar at runtime regardless of compiler. The proper fix bumps `tree-sitter` to 0.25 — that's #1060.

Why revert instead of letting it sit

#1059 isn't actively harmful — clang vs gcc produces functionally equivalent binaries here — but it adds CI configuration that solves nothing real and that future readers will be misled by. Better to keep the workflow clean and the diff history honest.

Test plan

  • Diff equals the inverse of 1e292ef (3 workflow files, -42 lines).
  • CI `native-host-build` matrix passes on Linux/macOS/Windows with the runner-default `cc`.

Refs #1054

Reverts 1e292ef. The premise of #1059 was that gcc-built binaries
silently dropped HCL files where clang-built ones did not — making the
v3.10.0 pre-publish gate fail. Closer empirical verification (running
both compilers against the same source in identical Docker containers,
counting "Native orchestrator dropped" warnings explicitly) showed that
both compilers produce binaries that drop .tf files. The earlier
"clang doesn't drop" reading was an output-filtering artifact in my
local benchmark grep.

The real cause is a tree-sitter ABI version mismatch — `tree-sitter-hcl`
1.1.0 ships ABI 15, runtime is pinned at ABI 14. `Parser::set_language`
rejects the grammar at runtime regardless of which compiler built the
binary. The proper fix is to bump `tree-sitter` to 0.25; that is being
delivered separately.

Reverting the clang change keeps Linux x86_64 GNU users on the
runner-default `cc` (gcc), matching every release prior to v3.10.0
and avoiding compiler-specific dead weight in our CI config that
fixes nothing.

Refs #1054
@claude
Copy link
Copy Markdown

claude Bot commented May 4, 2026

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


Reviewing PR: revert(ci): undo "force clang for Linux x86_64 native builds"

Todo List

  • Read and analyze changed workflow files
  • Verify root cause analysis and revert justification
  • Assess configuration change risks
  • Evaluate backlog compliance
  • Provide comprehensive review feedback

Root Cause Analysis

Problem Being Solved: Removing unnecessary compiler forcing that was added to address HCL file dropping issue, which has been determined to be caused by tree-sitter ABI mismatch rather than compiler choice.

Why This Approach: The PR author discovered through empirical verification that both gcc and clang produce binaries that drop .tf files due to tree-sitter ABI version mismatch (tree-sitter-hcl 1.1.0 ships ABI 15, runtime pinned at ABI 14). The original "fix" in #1059 was based on a misdiagnosis.

Risk Assessment: ✅ LOW RISK - This is a clean revert that removes configuration changes that provided no actual benefit. The change returns Linux builds to using the runner's default cc (gcc), which matches all releases prior to v3.10.0.

Backlog Compliance

  • Zero-dep: ✓ - No new dependencies added, pure configuration revert
  • Foundation-aligned: ✓ - Removes configuration complexity with no functional impact
  • Problem-fit: 4 - Maintains CI cleanliness and avoids misleading future developers about non-existent compiler-specific behavior differences
  • Breaking: No - No API, CLI, or functional changes
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Technical Analysis

Revert Scope Verification ✅

The revert cleanly removes 42 lines across 3 workflow files:

  • .github/workflows/build-native.yml: Removes clang forcing for x86_64-unknown-linux-gnu target
  • .github/workflows/ci.yml: Removes clang forcing for ubuntu-latest native builds
  • .github/workflows/publish.yml: Removes clang forcing for both preflight and build-native jobs

This matches exactly the inverse of commit 1e292ef from PR #1059.

Root Cause Evidence ✅

The PR description provides credible evidence that the original premise was wrong:

  1. Empirical verification: Author tested both compilers in identical Docker containers
  2. ABI mismatch identified: tree-sitter-hcl 1.1.0 ships ABI 15, runtime pinned at ABI 14
  3. Runtime rejection: Parser::set_language rejects the grammar regardless of compiler
  4. Proper fix referenced: PR fix(native): bump tree-sitter to 0.25 + ABI regression test (#1054) #1060 addresses the real issue by bumping tree-sitter to 0.25

Current state confirms ABI mismatch: crates/codegraph-core/Cargo.toml:15 shows tree-sitter = "0.24" while tree-sitter-hcl = "1" (line 31).

Configuration Change Assessment ✅

This is the good kind of configuration change - removing unnecessary complexity rather than adding it. The revert:

  • ✅ Removes configuration that solved no real problem
  • ✅ Returns to well-tested default behavior (all releases before v3.10.0)
  • ✅ Simplifies CI configuration
  • ✅ Avoids misleading future developers about non-existent compiler differences

Test Plan Evaluation ✅

The test plan is minimal but appropriate:

  • Diff equals the inverse of 1e292efVERIFIED
  • CI native-host-build matrix passes on Linux/macOS/Windows with runner-default cc - PENDING CI

This is sufficient for a clean revert that returns to well-established defaults.

Minor Observations

  1. Clean commit message: The commit message accurately describes the revert and provides good context
  2. Proper referencing: Correctly references related issues perf(native): ~2 second flat overhead added to rebuild operations in v3.10.0 #1054, fix(native): bump tree-sitter to 0.25 + ABI regression test (#1054) #1060
  3. No side effects: Pure revert with no additional changes or cleanup needed

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a well-justified revert that removes configuration complexity without any functional benefit. The root cause analysis is sound, the revert is clean, and it returns the codebase to a simpler, well-tested state. The only reason this isn't 5-star is that the original misdiagnosis could have been avoided with more thorough initial investigation.

The proper fix for the HCL parsing issue (tree-sitter version bump) should be handled separately in PR #1060 as mentioned.


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This is a clean mechanical revert of #1059, removing the CC=clang / CXX=clang++ overrides from all three workflow files (build-native.yml, ci.yml, publish.yml). The original change was based on a misattribution — the real HCL .tf drop is caused by a tree-sitter ABI 14/15 mismatch, addressed separately in #1060, not by the choice of C compiler.

Confidence Score: 5/5

Safe to merge — purely removes incorrect CI configuration with no functional impact on build correctness.

All three changes are exact inverses of #1059 steps with no logic differences; the root-cause is documented and addressed in a companion PR (#1060).

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/build-native.yml Removes the "Use clang for x86_64-linux-gnu" step that was incorrectly added in #1059; restores runner-default compiler.
.github/workflows/ci.yml Removes the "Use clang on Linux" step (with its now-accurate self-deprecating NOTE comment) from the native build job.
.github/workflows/publish.yml Removes two clang-forcing steps (preflight job and x86_64-linux-gnu matrix job), restoring parity with the reverted state.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI / build-native / publish trigger] --> B{Linux x86_64 runner?}
    B -- "Before revert (#1059)" --> C[Set CC=clang / CXX=clang++]
    C --> D[Rust build with clang]
    B -- "After revert (this PR)" --> D2[Rust build with runner-default cc/gcc]
    D --> E[Binary produced]
    D2 --> E
    E --> F{HCL .tf extraction}
    F -- "Real fix: tree-sitter 0.25 (#1060) ABI 14->15 resolved" --> G[.tf files parsed correctly]
    F -- "ABI mismatch unresolved" --> H[.tf files silently dropped]
Loading

Reviews (2): Last reviewed commit: "fix: resolve merge conflicts with main" | Re-trigger Greptile

Impact: 3 functions changed, 0 affected
@carlos-alm carlos-alm merged commit a28b6c2 into main May 4, 2026
21 checks passed
@carlos-alm carlos-alm deleted the revert/1059-clang branch May 4, 2026 22:03
@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