migrate workspace XLS selection into a module extension#52
Merged
dank-openai merged 13 commits intomainfrom Mar 13, 2026
Merged
migrate workspace XLS selection into a module extension#52dank-openai merged 13 commits intomainfrom
dank-openai merged 13 commits intomainfrom
Conversation
dank-openai
force-pushed
the
dank/xls-toolchain-toml
branch
from
c54d481 to
63aa861
Compare
Thread artifact-config TOML through Bazel actions, tighten env-helper flag handling, and gate enum-case naming for older drivers.
Add the bundle repository/materializer, wire workspace examples into presubmit, and fix runfile and staging fallout so the new bundle contract works end to end.
Make MODULE.bazel the only artifact-selection surface now that workspace bundle plumbing is in place.
Bootstrap download-backed driver installs, expose the runtime names consumers need, and extend explicit bundle selection across direct DSLX, ECO, and IR analysis rules.
Reuse validated download caches, package the xlsynth-sys and native runtime surfaces, normalize staged libxls identity, and hand installed-layout ownership to consumer-owned root prefixes.
dank-openai
force-pushed
the
dank/xls-toolchain-toml
branch
from
63aa861 to
030698b
Compare
This was referenced Mar 11, 2026
dank-openai
changed the title
Contributor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a139556ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dank-openai
added a commit
to xlsynth/xlsynth-crate
that referenced
this pull request
Mar 13, 2026
## Problem Solved
`xlsynth-sys` needed a Bazel-friendly way to receive the XLS DSO and
DSLX stdlib as one declared input instead of two separate environment
variables. It also needed a mode where `build.rs` records those artifact
paths without emitting its own native link directives, so an external
build system can own the final link step.
## Review Focus
`XLSYNTH_ARTIFACT_CONFIG` precedence, declared-link mode, and
compatibility with the legacy `XLS_DSO_PATH` + `DSLX_STDLIB_PATH`
override.
## Mental Model
`XLSYNTH_ARTIFACT_CONFIG` is now the preferred declared-input contract:
a TOML file with `dso_path` and `dslx_stdlib_path`. `build.rs` resolves
that config first, writes the selected paths into generated Rust
constants, and then either emits native link directives or only
publishes the DSO path depending on `XLSYNTH_SYS_LINK_MODE`.
If that config is absent, `build.rs` deliberately falls back to the
older paired `XLS_DSO_PATH` plus `DSLX_STDLIB_PATH` override, the local
workspace override, and the download fallback so existing users who
cannot or will not migrate immediately still keep working.
## Non-goals
- This does not change the public `xlsynth` Rust API.
- This does not remove the legacy paired environment-variable override.
- This does not make Cargo-only consumers adopt Bazel-specific wiring.
## Tradeoffs
- `build.rs` now owns more validation and precedence logic.
- `declared` link mode is stricter: it preserves artifact discovery but
leaves final native link ownership to the orchestrator.
## Architecture
- Add `XLSYNTH_ARTIFACT_CONFIG` parsing, precedence, and relative-path
resolution from an absolute config path.
- Keep generating `artifact_paths.rs` so Rust code still sees one
resolved artifact choice.
- Add `XLSYNTH_SYS_LINK_MODE={native,declared}` so managed and explicit
artifact paths can skip native `-l...` emission when the caller already
declares the link inputs.
- Continue emitting `cargo:DSO_PATH` so dependent build scripts can
consume the resolved runtime location through Cargo's normal
`DEP_XLSYNTH_DSO_PATH` mechanism.
- Preserve the existing paired env override and download-backed fallback
for non-Bazel callers.
## Observability
- `build.rs` now reports which artifact source won: config file, paired
env override, local workspace, or download.
- Bad config shapes fail with targeted messages, including
relative-config-path rejection and invalid shared-library-path
rejection.
- `XLSYNTH_SYS_LINK_MODE=declared` is explicit in the build output when
native link directives are intentionally skipped.
# Related PRs in the other repos
- `xlsynth/rules_xlsynth`: [PR
#52](xlsynth/rules_xlsynth#52)
- `xlsynth/xlsynth-crate`: [PR
#851](#851)
- `xlsynth/topstitch`: [PR
#141](xlsynth/topstitch#141)
meheff
approved these changes
Mar 13, 2026
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
2 participants
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.
Problem Solved
Public XLS artifact selection moves out of
.bazelrcartifact-path flags and into a Bzlmod module extension. A workspace now chooses one or more named XLS bundles inMODULE.bazel, registers one default bundle, and consumes a narrower exported bundle contract instead of wiring tool, stdlib, driver, and runtime paths separately.Review Focus
The module extension API, exported bundle labels, and whether
artifact_sourceforms a stable downstream contract.Mental Model
Each
xls.toolchain(...)call materializes a named bundle repo.use_repo(...)exposes that repo,register_toolchains("@<name>//:all")makes one bundle the workspace default, and most DSLX rules use that default through normal toolchain resolution. Rules that must stay on another XLS stack can opt into a different bundle explicitly withxls_bundle = "@<name>//:bundle".The existing TOML-plus-
env_helpersexecution path stays in place. What changed is where artifact selection lives: the workspace root chooses bundles, while rule actions keep consuming declared TOML inputs.Non-goals
dslx_path,type_inference_v2, oruse_system_verilog.Tradeoffs
.bazelrcartifact flags.rustupon the host performing module resolution.Architecture
xlsmodule extension plus repository rule that materializes bundle repos fromauto,eda_tools_only,download_only, orlocal_paths.:bundle,:libxls_link, and thexlsynth_sys_*targets used by Bazel plusxlsynth-sysintegration.env_helpersas the runner boundary for Bazel actions.libxlsruntime identity so native consumers can link and run against bundle-provided runtime files without wrapper staging.xls_bundleescape hatches on the supported leaf rules that truly need a non-default bundle.Observability
artifact_sourcecombinations fail during bundle materialization instead of surfacing later as action-environment drift.libxlsname and whether the selected driver supports enum-case naming.local_pathslocal-development flow.Related PRs in the other repos
xlsynth/rules_xlsynth: PR #52xlsynth/xlsynth-crate: PR #851xlsynth/topstitch: PR #141