xlsynth-sys: support XLSYNTH_ARTIFACT_CONFIG#851
Merged
dank-openai merged 5 commits intomainfrom Mar 13, 2026
Merged
Conversation
dank-openai
force-pushed
the
dank/xls-toolchain-toml
branch
5 times, most recently
from
824d867 to
4aee06f
Compare
Resolve relative artifact-config paths against the config file directory, cover that contract in the nested cargo smoke test, and document that XLS_DSO_PATH may be either a file path or a directory depending on how the DSO was materialized.
dank-openai
force-pushed
the
dank/xls-toolchain-toml
branch
from
b1b862d to
fae5850
Compare
This was referenced Mar 11, 2026
Contributor
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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/rules_xlsynth
that referenced
this pull request
Mar 13, 2026
## Problem Solved
Public XLS artifact selection moves out of `.bazelrc` artifact-path
flags and into a Bzlmod module extension. A workspace now chooses one or
more named XLS bundles in `MODULE.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_source` forms 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 with `xls_bundle = "@<name>//:bundle"`.
The existing TOML-plus-`env_helpers` execution 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
- This does not remove the existing behavior settings such as warning
knobs, `dslx_path`, `type_inference_v2`, or `use_system_verilog`.
- This does not require every rule callsite to spell its bundle
explicitly.
- This does not make downstream consumers reach into generic bundle
internals by hand.
## Tradeoffs
- More logic now lives in the module extension and bundle
materialization path.
- The public API is stricter: artifact selection is bundle-only, not a
mix of bundle selection and `.bazelrc` artifact flags.
- Download-backed bundle resolution requires `rustup` on the host
performing module resolution.
## Architecture
- Add the `xls` module extension plus repository rule that materializes
bundle repos from `auto`, `eda_tools_only`, `download_only`, or
`local_paths`.
- Export a stable repo surface for downstream consumers, including
`:bundle`, `:libxls_link`, and the `xlsynth_sys_*` targets used by Bazel
plus `xlsynth-sys` integration.
- Keep per-action TOML generation and `env_helpers` as the runner
boundary for Bazel actions.
- Normalize the staged `libxls` runtime identity so native consumers can
link and run against bundle-provided runtime files without wrapper
staging.
- Add explicit `xls_bundle` escape hatches on the supported leaf rules
that truly need a non-default bundle.
## Observability
- Invalid `artifact_source` combinations fail during bundle
materialization instead of surfacing later as action-environment drift.
- Bundle metadata records the normalized `libxls` name and whether the
selected driver supports enum-case naming.
- The checked-in examples show both the normal workspace-default flow
and the `local_paths` local-development flow.
-
# Related PRs in the other repos
- `xlsynth/rules_xlsynth`: [PR
#52](#52)
- `xlsynth/xlsynth-crate`: [PR
#851](xlsynth/xlsynth-crate#851)
- `xlsynth/topstitch`: [PR
#141](xlsynth/topstitch#141)
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
xlsynth-sysneeded 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 wherebuild.rsrecords 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_CONFIGprecedence, declared-link mode, and compatibility with the legacyXLS_DSO_PATH+DSLX_STDLIB_PATHoverride.Mental Model
XLSYNTH_ARTIFACT_CONFIGis now the preferred declared-input contract: a TOML file withdso_pathanddslx_stdlib_path.build.rsresolves 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 onXLSYNTH_SYS_LINK_MODE.If that config is absent,
build.rsdeliberately falls back to the older pairedXLS_DSO_PATHplusDSLX_STDLIB_PATHoverride, the local workspace override, and the download fallback so existing users who cannot or will not migrate immediately still keep working.Non-goals
xlsynthRust API.Tradeoffs
build.rsnow owns more validation and precedence logic.declaredlink mode is stricter: it preserves artifact discovery but leaves final native link ownership to the orchestrator.Architecture
XLSYNTH_ARTIFACT_CONFIGparsing, precedence, and relative-path resolution from an absolute config path.artifact_paths.rsso Rust code still sees one resolved artifact choice.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.cargo:DSO_PATHso dependent build scripts can consume the resolved runtime location through Cargo's normalDEP_XLSYNTH_DSO_PATHmechanism.Observability
build.rsnow reports which artifact source won: config file, paired env override, local workspace, or download.XLSYNTH_SYS_LINK_MODE=declaredis explicit in the build output when native link directives are intentionally skipped.Related PRs in the other repos
xlsynth/rules_xlsynth: PR #52xlsynth/xlsynth-crate: PR #851xlsynth/topstitch: PR #141