Declare system LLVM as +python external instead of rebuilding it - #121
Open
TApplencourt wants to merge 11 commits into
Open
Declare system LLVM as +python external instead of rebuilding it#121TApplencourt wants to merge 11 commits into
TApplencourt wants to merge 11 commits into
Conversation
h2yaml needs llvm@18:+clang+python, where +python means the prefix ships clang's Python bindings (clang/cindex.py) next to libclang. `spack external find` picks variants from executables and never looks for the bindings, so it always records LLVM as ~python -- bindings present or not. Such an entry can never satisfy h2yaml, so a usable system LLVM goes unused and Spack builds one from source, by far the longest step of a THAPI install. Add scripts/gen-llvm-external.py, which prints the packages.yaml config for a given LLVM prefix. If the bindings are already there it points the external at the prefix directly; otherwise it builds an overlay (symlinks to the real install, since site LLVMs are read-only) and supplies the bindings, either from where the distribution put them or by downloading them from the matching LLVM release -- they are pure Python, so nothing is compiled. It verifies clang/cindex.py is reachable before emitting, so a wrong +python claim fails during setup rather than mid-build. Stdlib only, no dependencies. The entry carries extra_attributes:compilers. llvm+clang provides c/cxx, so Spack may select the external as the compiler; spack's own llvm package treats that attribute as mandatory, and without it the build runs with SPACK_CC=None and every compile fails with "C compiler cannot create executables". Use it from both the README and CI, replacing the sed that rewrote packages.yaml in place, and drop the separate symlink workaround it needed. h2yaml: search for the bindings rather than assuming lib/python<spack's python>/site-packages. That path is named for whichever python LLVM was built against, which need not be the one Spack runs h2yaml with, and externals may use lib64 or Debian's dist-packages. Also stop re-joining an already-absolute libclang path onto prefix.lib, and raise a pointed error naming the searched prefix instead of failing later with a bare ModuleNotFoundError: clang. cron.yml: keep the from-scratch install (the only job checking THAPI builds on a bare machine) and add a job exercising the fast path PR CI uses, which would otherwise go untested during a quiet week. It was also missing the env block, so SPACK_OPT expanded empty and failures produced no log. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TApplencourt
force-pushed
the
improve_clang_binding_detection
branch
from
September 8, 2026 23:39
0bbe6ac to
1f58414
Compare
`spack external find` and `spack compiler find` both register the system llvm without `+python`, shadowing the entry gen-llvm-external.py writes. Exclude it from the former; `compiler find` has no such flag, so remove what it added. The mirror is created with `--autopush`, which uploads packages but never writes an index, and spack skips an unindexed mirror during concretization. Index it before upload so downstream jobs reuse it. Move the cron fast-path check ahead of the install: it only needs concretization, and running it afterwards means paying for the llvm build it exists to catch. upload/download-artifact v6->v7, delete-artifact v5->v6, ruff-action v3->v4.1.0, all now on node24. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`spack spec -j` gives structured output, so version numbers and the external marker come from jq rather than sed/awk over the rendered tree. `.external.path` is null for a non-external node, which is what `jq -e` needs to set its exit code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only status lines start with the marker; it can appear anywhere inside the json itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A spec can carry more than one llvm or python node. Selecting them node-by-node yields a newline-separated list, which lands in the apt sources line as "llvm-toolchain-noble-22\n22 main" and apt rejects the file. Reduce to the distinct values and stop if more than one survives, so an ambiguous spec is a clear error rather than a malformed repo entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The runner pool mixes cpu generations. Spack defaults to concretizer:targets:granularity:microarchitectures, which puts the microarchitecture in the dag hash, so a package built on one runner does not match the same package on another. Every consumer job was rebuilding its whole dependency tree from source: the thapi@0.0.12 (legacy) log reported "no binary available" for all of protobuf, abseil-cpp, elfutils, babeltrace2 and lttng-tools, each of which the dependencies job had just built and pushed. Targeting x86_64_v3 keeps hashes stable across the pool. Binaries lose cpu-specific tuning, which does not matter for a build test. from_scratch installs without a build cache, so it keeps the default. Also drop --concurrent-packages 2. It dates from the old installer, where 0 meant serial. The new installer, the default since v1.0, reads 0 as no package limit and bounds concurrency with a jobserver sized to the core count, so the flag only caps parallelism below what the hardware allows.
Both cost ~17 min in the dependency jobs and are already installed by apt, but `spack external find` cannot see them: neither recipe declares `executables` or `libraries`. Add scripts/gen-external.py, which reads the version from the installed tool and prints the packages.yaml entry, and use it for both. Same shape as gen-llvm-external.py. This pins PR CI to Noble's older protobuf; the weekly cron `from_scratch` job installs with no externals and covers the current one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
line-length is 150, so the two wrapped calls fit on one line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same treatment as re2c and protobuf: ~6 min in the dependency jobs, apt already ships it, and the recipe has no detectors so `spack external find` cannot see it. Version comes from `eu-readelf --version`. Unlike the other two this is a link dependency (of babeltrace2, and of glib through the `elf` virtual), but `libelf.pc` lives in pkg-config's default search path, so it does not need the PKG_CONFIG_PATH handling that keeps libxml2 excluded from `spack external find`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
h2yaml needs llvm@18:+clang+python, where +python means the prefix ships clang's Python bindings (clang/cindex.py) next to libclang.
spack external findpicks variants from executables and never looks for the bindings, so it always records LLVM as ~python -- bindings present or not. Such an entry can never satisfy h2yaml, so a usable system LLVM goes unused and Spack builds one from source, by far the longest step of a THAPI install.Add scripts/gen-llvm-external.py, which prints the packages.yaml config for a given LLVM prefix. If the bindings are already there it points the external at the prefix directly; otherwise it builds an overlay (symlinks to the real install, since site LLVMs are read-only) and supplies the bindings, either from where the distribution put them or by downloading them from the matching LLVM release -- they are pure Python, so nothing is compiled. It verifies clang/cindex.py is reachable before emitting, so a wrong +python claim fails during setup rather than mid-build. Stdlib only, no dependencies.
Use it from both the README and CI, replacing the sed that rewrote packages.yaml in place, and drop the separate symlink workaround it needed.
h2yaml: search for the bindings rather than assuming lib/python<spack's python>/site-packages. That path is named for whichever python LLVM was built against, which need not be the one Spack runs h2yaml with, and externals may use lib64 or Debian's dist-packages. Also stop re-joining an already-absolute libclang path onto prefix.lib, and raise a pointed error naming the searched prefix instead of failing later with a bare ModuleNotFoundError: clang.
cron.yml: keep the from-scratch install (the only job checking THAPI builds on a bare machine) and add a job exercising the fast path PR CI uses, which would otherwise go untested during a quiet week. It was also missing the env block, so SPACK_OPT expanded empty and failures produced no log.