Conversation
…ang symlink Replace `ARG CI` with `ARG PHASED_BUILD=true` so the OOM-aware phased LLVM build is controlled explicitly rather than piggy-backing on the CI environment variable. Add newly discovered memory-hungry Flang/MLIR targets (flang driver, fir-opt, fir-lsp-server, tco, MLIRCAPIRegisterEverything, MLIRLinalgTransforms) to the Phase 2 list. Fix flang binary detection for LLVM >= 20 where `flang` is a symlink to `flang-20` by passing `-L` to find.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades the LLVM version from 19 to 20 in the Docker-based build infrastructure. The primary driver is the LLVM 20 rename of flang-new to flang, which requires adjustments across the Dockerfile build targets, binary verification, TAU installation, and CI tagging.
Changes:
- Bumps
LLVM_VERfrom 19 to 20 and adapts all flang binary/target references with version-conditional logic for backward compatibility - Replaces the
CIbuild-arg toggle withPHASED_BUILD(defaulting totrue) for OOM-aware phased builds, and expands the OOM-fragile target list with newly identified memory-hungry LLVM 20 targets - Adds LLVM-version-suffixed Docker image tags in CI and passes
LLVM_VERas a build argument
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Dockerfile | Upgrades LLVM_VER to 20; adds version-conditional flang binary/target names; replaces CI with PHASED_BUILD; expands OOM-fragile grep pattern; updates flang verification logic |
| .github/workflows/CI.yml | Adds LLVM_VER env var; passes it as Docker build-arg; generates LLVM-version-suffixed image tags; moves Checkout step before Docker meta |
| CLAUDE.md | Streamlines command block; updates gotchas for PHASED_BUILD; updates security convention note; removes stale gotchas |
| .claude/skills/build/SKILL.md | Adds --builder salt-8cpu to local build commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Seven new Claude Code hooks enforce conventions automatically:
- check-unicode-lookalikes: flag non-ASCII doppelgangers (em/en dashes, smart quotes, NBSP)
- check-trailing-whitespace: flag trailing whitespace (markdown line-break aware)
- check-lint-registration: warn on missing or stale lint.sh manifest entries
- check-shell-strict-mode: require set -euo pipefail in shell scripts
- check-dockerfile-heredoc-strict: require set -euo pipefail in Dockerfile RUN heredocs
- check-workflow-expressions: flag bare ${{ }} in GitHub Actions run blocks
- lint-changed-file: run single-file lint on save
Enhance lint.sh with --file single-file mode, --warn-untracked flag
for discovering unlisted lintable files, manifest validation that
errors on missing files, and a CONFIG_FILES array for tool configs.
Promote block-pipe-to-shell to user-level hook; remove from project.
Delete vestigial docker-cache action scripts and redundant lint skill.
Update README to use ./script instead of bash script.
Trim CLAUDE.md now that hooks enforce conventions directly.
Replace Unicode em dashes (—) with ASCII double-dashes (--) across all project files. Add .gitattributes with text=auto for consistent line-ending normalization. Remove trailing whitespace in .actionlint.yaml.
Add /.gitattributes to dev-tooling exclusions. Exclude *.tgz to prevent loose archives from inflating the build context.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add dorny/paths-filter to CI workflow so doc-only or tooling-only pushes skip the expensive LLVM Docker build. Lint and changes jobs run in parallel; build-base gates on both. Schedule and tag events bypass the filter and always build. Add ci_filter_check to lint.sh that verifies every Dockerfile COPY/ADD source, Dockerfile, and submodule gitlink is covered by the CI path-filter -- catches filter drift automatically. Fix missing 'patches' gitlink entry (patches/** alone misses submodule pointer updates).
Add missing final newline in .gitignore. Fix lost space in CLAUDE.md
em-dash conversion (--only -> -- only). Quote ${LLVM_VER} in
Dockerfile arithmetic comparisons to guard against empty values.
fix spacing Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fix indent Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fix space Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fix var name change Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add set -uo pipefail to lint.sh (-e omitted so all checks run before summary). Guard --file against missing argument to prevent infinite loop. Remove block-pipe-to-shell.sh from project hooks and lint manifest (promoted to user-level). Update strict-mode hook to accept set -uo pipefail for lint.sh while requiring set -euo pipefail everywhere else.
Use set -euo pipefail in lint-changed-file.sh (was set -o pipefail). Remove unused ARG CI=false from Dockerfile and update debug echo to print PHASED_BUILD. Quote $FLANG_CMD in installtau calls.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace CI=true with PHASED_BUILD=true in cache-warmup build step - Remove stale Source: CLAUDE.md references from hook comments
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade to LLVM 20