envoyproxy / envoyproxy/toolshed

compile: llvm_toolchain_alias fails to resolve minimal LLVM repos under external bzlmod

Open
#4,926 0 comments 1 reaction 2 assignees Claimed by @phlax View on GitHub
Dominant language
Python
Stars
12
Forks
24
Avg merge
6h 37m
Merged PRs (30d)
92

Description

## Summary

The `llvm_toolchain_alias_extension` in `bazel/compile/extensions.bzl` (backed by the `llvm_toolchain_alias` repository rule in `bazel/compile/llvm_minimal.bzl`) does not work when `envoy_toolshed` is consumed as an **external** bzlmod module (e.g. from the Envoy repo). It fails to locate the host-arch `llvm_minimal_*` repository.

The fix should be verified against **toolshed's own bzlmod build first** (`bazel/MODULE.bazel` already wires up all relevant extensions); the same fix unblocks external consumers (Envoy's bzlmod migration).

## Background: what the code does today

Two separate module extensions are defined in `bazel/compile/extensions.bzl`:

- `llvm_minimal_extension` → calls `setup_llvm_minimal()` (in `bazel/compile/llvm_minimal.bzl`), creating `@llvm_minimal_linux_x64`, `@llvm_minimal_linux_arm64`, `@llvm_minimal_macos_arm64` via the `llvm_minimal_repo` repo rule (each downloads a released minimal LLVM artifact and writes a `BUILD.bazel` from `LLVM_MINIMAL_LLVM_REPO_BUILD`).
- `llvm_toolchain_alias_extension` → creates a single `@llvm_toolchain_llvm` repo via the `llvm_toolchain_alias` repo rule, meant to point at whichever `llvm_minimal_*` repo matches the host OS/arch, exposing the same filegroup targets (`objcopy`, `clang`, `ld`, `nm`, etc.) so consumers can reference e.g. `@llvm_toolchain_llvm//:objcopy`.

`bazel/MODULE.bazel` wires both up (external consumers mirror this exactly):

```starlark
llvm_minimal_ext = use_extension("//compile:extensions.bzl", "llvm_minimal_extension", dev_dependency = True)
llvm_minimal_ext.setup()
use_repo(llvm_minimal_ext, "llvm_minimal_linux_x64", "llvm_minimal_linux_arm64", "llvm_minimal_macos_arm64")

llvm_toolchain_alias_ext = use_extension("//compile:extensions.bzl", "llvm_toolchain_alias_extension", dev_dependency = True)
use_repo(llvm_toolchain_alias_ext, "llvm_toolchain_llvm")
```

## Core problem: cross-extension repository visibility

The `llvm_toolchain_alias` repo rule must find the host-arch `llvm_minimal_*` repo, but those repos are created by a **different** module extension (`llvm_minimal_extension`). Under bzlmod, each `module_extension` has its own repo-visibility namespace. Repos created by `llvm_minimal_extension` are NOT visible from within `llvm_toolchain_alias_extension` (nor the alias repo it generates) by apparent name — `use_repo` only makes them visible to the root module, not to a sibling extension.

Two distinct failure forms observed while iterating:

**1. Bare apparent-name `Label` inside the repo rule** — `ctx.path(Label("@{}//:BUILD.bazel".format(minimal_repo)))`:
```
Error in path: Unable to load package for @@[unknown repo 'llvm_minimal_linux_x64' requested from @@envoy_toolshed+]//:BUILD.bazel: ... No repository visible as '@llvm_minimal_linux_x64' from repository '@@envoy_toolshed+'
```

**2. Passing `Label("@llvm_minimal_linux_x64//:BUILD.bazel")` (constructed in `extensions.bzl`) as an `attr.label`** — fails at extension-eval time:
```
Error in repository_rule: no repository visible as '@llvm_minimal_linux_x64' in the extension '...%llvm_toolchain_alias_extension', but referenced by label '@llvm_minimal_linux_x64//:BUILD.bazel' in attribute 'minimal_linux_x64' of llvm_toolchain_alias 'llvm_toolchain_llvm'.
```
i.e. constructing the `Label` in the same `.bzl` file is NOT sufficient — the repos still belong to the other extension's namespace.

### Suggested resolution approach

Make the **same extension** that creates the alias also create the `llvm_minimal_*` repos, so they are siblings in one visibility namespace:

- Have `_llvm_toolchain_alias_ext_impl` call `setup_llvm_minimal()` itself, then instantiate `llvm_toolchain_alias` passing the repos in as canonical `Label` attributes (`minimal_linux_x64` / `minimal_linux_arm64` / `minimal_macos_arm64` → `Label("@llvm_minimal_.../:BUILD.bazel")`). Now the labels reference repos this extension owns, so they resolve.
- The `llvm_toolchain_alias` repo rule selects the correct label by host OS/arch (`ctx.os.name` / `ctx.os.arch`) and resolves `ctx.path().dirname` — no bare apparent-name `Label(...)` string construction.

No repo-name collision even though `llvm_minimal_extension` also calls `setup_llvm_minimal()` (separate namespaces). The only cost is the artifact may be fetched under both extensions. If that duplication can be cleanly avoided (alias extension becomes sole owner, exports the `llvm_minimal_*` repos, `bazel/MODULE.bazel` updated to `use_repo` them from the alias extension, dropping the redundant separate setup), prefer that — but ONLY if it keeps toolshed's own build green and preserves the public repo/extension names. A correct working build matters more than eliminating the double fetch.

## Second, separate bug in the same code path: whole-directory symlinks break filegroup staging

An in-progress version of `_llvm_toolchain_alias_impl` replaced per-child symlinking with whole-directory symlinks:

```starlark
ctx.symlink(minimal_root.get_child("bin"), "bin")
ctx.symlink(minimal_root.get_child("lib"), "lib")
ctx.symlink(minimal_root.get_child("include"), "include")
ctx.symlink(minimal_root.get_child("BUILD.bazel"), "BUILD.bazel")
```

This is a regression. The original code deliberately symlinked each child individually (helpers `_symlink_dir_children` / `_ensure_repo_dir`), per the existing docstring:

> A directory symlink (ctx.symlink on the dir itself) is NOT followed by Bazel when sourcing individual files for a filegroup src, so bzlmod reports "missing input file '...//:bin/llvm-nm'".

The alias repo's `BUILD.bazel` (`LLVM_MINIMAL_LLVM_REPO_BUILD`) defines filegroups whose `srcs` are individual files like `bin/llvm-objcopy`, `bin/llvm-nm`. With whole-directory symlinks these fail to stage at execution (analysis may pass, build fails when an action needs the tool).

### Required fix for the symlink bug

Restore per-child symlinking: populate `bin/`, `include/`, `lib/` using the existing `_ensure_repo_dir` (which calls `_symlink_dir_children` and `mkdir`s the dir if the source is absent — e.g. a macOS tarball lacking a dir), then write the alias repo's own `BUILD.bazel` explicitly with `ctx.file("BUILD.bazel", LLVM_MINIMAL_LLVM_REPO_BUILD)` (do NOT symlink the minimal repo's BUILD.bazel). Keep the docstring explaining the per-child rationale.

## Also: remove dead / duplicated code

`bazel/compile/llvm_minimal.bzl` accumulated duplicates/dead code while iterating:
- `_normalize_llvm_toolchain_alias_os` and `_normalize_llvm_toolchain_alias_arch` are defined **twice** (one pair using exact `== "linux"` / `"mac os x"`/`"darwin"`, another using `startswith(...)`). Keep exactly ONE pair — prefer the more robust `startswith`-style, mapping to the same canonical tokens (`linux`, `macos`, `x86_64`, `aarch64`) and preserving an "unsupported host platform" `fail()` for anything outside linux/x86_64, linux/aarch64, macos/arm64.
- `_get_llvm_toolchain_alias_platform_info` becomes dead once selection is via label attrs; remove it if unused.

## Files in scope
- `bazel/compile/extensions.bzl`
- `bazel/compile/llvm_minimal.bzl`
- `bazel/MODULE.bazel` (only if choosing the single-owner cleanup; otherwise leave as-is)
- `bazel/MODULE.bazel.lock` may need regenerating

## Acceptance criteria (MUST verify against toolshed's own build)

1. Toolshed's own bzlmod build works: `llvm_toolchain_alias_extension` evaluates without "no repository visible as '@llvm_minimal_linux_x64'", and `@llvm_toolchain_llvm//:objcopy` (plus `clang`, `ld`, `nm`, `ar`, `strip`, `symbolizer`, etc.) resolve to host-arch minimal LLVM tools.
2. A **real build/execution** consuming a tool from `@llvm_toolchain_llvm` (not just analysis) succeeds — per-child symlinks stage correctly, no "missing input file '...//:bin/llvm-...'" error.
3. Public API unchanged for external consumers: still `llvm_toolchain_alias_extension`, still generates a repo named `llvm_toolchain_llvm` exposing the same filegroup target names. (If taking the single-owner cleanup, ensure external consumers can still `use_repo` the same repo names; document any required consumer-side change.)
4. No duplicate helper definitions; no dead code.
5. `fail()` still fires for unsupported host platforms.

## Notes
- This is the reference/first-party fix; downstream Envoy's `MODULE.bazel` wires the extensions the same way and consumes toolshed via a local path override, so keeping repo/extension public names stable is important.
- Do NOT change the set of tools in `LLVM_MINIMAL_BINS` or the filegroup schema in `LLVM_MINIMAL_LLVM_REPO_BUILD`.

## Context
Prior attempts: abandoned PR `phlax/toolshed` #4923 (against a fork branch) and two coding-agent runs that stalled before pushing. This issue captures the full context so a fresh agent/contributor can implement it end to end.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.