topstitch: pin xlsynth-crate to published rollout rev#141
Merged
dank-openai merged 6 commits intomainfrom Mar 13, 2026
Merged
Conversation
dank-openai
commented
Mar 11, 2026
Cargo.toml
Outdated
| num-bigint = "0.4.3" | ||
| indexmap = "2.5.0" | ||
| xlsynth = "0.19.0" | ||
| xlsynth = { git = "https://github.com/xlsynth/xlsynth-crate.git", rev = "fae5850379c6868b8e228d703a636209db56f2b4", version = "0.37.0" } |
Contributor
Author
This was referenced Mar 11, 2026
meheff
approved these changes
Mar 11, 2026
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)
Update the xlsynth git dependency to bc173bd70d5526095058a6c5076252f2192f75ea after xlsynth-crate PR #851 merged and move the semver guard to 0.39.0 so Cargo can resolve the merged source. Validation: env CARGO_NET_GIT_FETCH_WITH_CLI=true cargo check --locked
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
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.
Summary
topstitchto the publishedxlsynth-craterollout revb1b862d95338e345a9784b9b84e9b0f6ad9276e5Cargo.lockto resolvexlsynthandxlsynth-sysfrom that published git revision instead of local-only pinning0.95.1used for the XLS toolchain rollout lineValidation
cargo metadata --locked --format-version 1 --no-depscargo test --locked --no-run