feat: build host-only doc tools in host config via aspect_bazel_lib platform_transition_binary#635
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //src:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
|
@a-zw could you have a look on it, please? |
The runnable documentation targets (docs, docs_combo, docs_check, docs_link_check, live_preview, live_preview_combo_experimental, ide_support) are host tools that run Python/Sphinx locally. Under a cross `--platforms` (e.g. QNX) they were configured for the target platform and failed Python toolchain resolution, breaking `bazel build //...` in cross-compiling consumers. Wrap each with `platform_transition_binary` from aspect_bazel_lib, which transitions the wrapped binary to a fixed `target_platform` (`@local_config_platform//:host`) and forwards its executable, runfiles and RunEnvironmentInfo, so `bazel run //:docs` keeps working. `sphinx_build` is only consumed as a tool (already built in the exec/host config) and is tagged `manual` so wildcard builds under a cross platform do not try to configure it for the target platform. aspect_bazel_lib is already in the module graph via aspect_rules_py; it is declared as a direct dep so the rule can be loaded.
5cbc574 to
8f6f97e
Compare
a-zw
left a comment
There was a problem hiding this comment.
Minor nitpicks. The essential change looks good to me.
| # `sphinx_build` is only consumed as a tool (see `needs_json`'s `sphinx` attr below), so Bazel | ||
| # already builds it in the exec (host) config -- no host transition needed. It is tagged | ||
| # `manual` so a cross-platform `bazel build //...` doesn't configure it for the target platform | ||
| # (which would fail Python toolchain resolution). |
There was a problem hiding this comment.
This comment is overly specific. My understanding is that tagging as "manual" means bazel wildcards (e.g. //...) do not match it.
manual keyword will exclude the target from expansion of target pattern wildcards (..., :*, :all, etc.) and test_suite rules which do not list the test explicitly when computing the set of top-level targets to build/run for the build, test, and coverage commands. It does not affect target wildcard or test suite expansion in other contexts, including the query command. Note that manual does not imply that a target should not be built/run automatically by continuous build/test systems. For example, it may be desirable to exclude a target from bazel test ... because it requires specific Bazel flags, but still have it included in properly-configured presubmit or continuous test runs.
| name = "docs", | ||
| binary = ":docs_impl", | ||
| target_platform = "@local_config_platform//:host", | ||
| tags = ["cli_help=Build documentation:\nbazel run //:docs"], |
There was a problem hiding this comment.
| tags = ["cli_help=Build documentation:\nbazel run //:docs"], |
This tag never made sense, so this would be a good opportunity to remove them.
| # `platform_transition_binary` (used to build host-only doc tools in the host | ||
| # config under a cross `--platforms`). Already in the graph via aspect_rules_py. |
There was a problem hiding this comment.
| # `platform_transition_binary` (used to build host-only doc tools in the host | |
| # config under a cross `--platforms`). Already in the graph via aspect_rules_py. |
No need for verbosity.
| # -- Host-platform transition for host-only documentation tooling -------------- | ||
| # | ||
| # The runnable doc targets (docs, ide_support, live_preview, ...) are pure host tools that run | ||
| # Python/Sphinx locally. Under a cross `--platforms` (e.g. QNX) they would be configured for the | ||
| # target platform and fail Python toolchain resolution. To avoid that, each is wrapped with | ||
| # `platform_transition_binary` from aspect_bazel_lib, which transitions the wrapped binary to a fixed | ||
| # `target_platform` (the autodetected host, `@local_config_platform//:host`) while forwarding its | ||
| # executable, runfiles and RunEnvironmentInfo -- so `bazel build //...` under a cross platform and | ||
| # `bazel run //:docs` both keep working. | ||
|
|
There was a problem hiding this comment.
| # -- Host-platform transition for host-only documentation tooling -------------- | |
| # | |
| # The runnable doc targets (docs, ide_support, live_preview, ...) are pure host tools that run | |
| # Python/Sphinx locally. Under a cross `--platforms` (e.g. QNX) they would be configured for the | |
| # target platform and fail Python toolchain resolution. To avoid that, each is wrapped with | |
| # `platform_transition_binary` from aspect_bazel_lib, which transitions the wrapped binary to a fixed | |
| # `target_platform` (the autodetected host, `@local_config_platform//:host`) while forwarding its | |
| # executable, runfiles and RunEnvironmentInfo -- so `bazel build //...` under a cross platform and | |
| # `bazel run //:docs` both keep working. |
This makes sense for the PR description but is too verbose for this bzl file.
Why
The runnable documentation targets created by
docs()—docs,docs_combo,docs_check,docs_link_check,live_preview,live_preview_combo_experimental,ide_support— are host tools that run Python/Sphinx on the build machine. When a consumer builds with a cross--platforms(e.g.--config=per-x86_64-qnx -- //...), these top-level targets get configured for the target platform and fail Python toolchain resolution (No matching toolchains ... tools/python), breaking the whole build in cross-compiling consumers (persistency, reference_integration).What
Each host-only run target is wrapped with
platform_transition_binaryfromaspect_bazel_lib, transitioning the wrapped binary to a fixedtarget_platform(@local_config_platform//:host) and forwarding its executable, runfiles andRunEnvironmentInfo, sobazel run //:docskeeps working unchanged.aspect_bazel_libis already in the module graph viaaspect_rules_py; it is declared as a directbazel_dep(2.16.0) so the rule can be loaded.sphinx_buildis only ever consumed as a tool (already built in the exec/host config), so it needs no transition; it is taggedmanualso a wildcardbazel build //...under a cross platform skips it.Comparison with #633
_host_binary)platform_transition_binary)--host_platformdynamically@local_config_platform//:host(autodetected host)bazel_deponaspect_bazel_lib(already transitive)The dynamic
--host_platformbehaviour of #633 is more precise when a consumer sets a specific--host_platformthat differs from the autodetected host; this PR trades that for less code and a battle-tested rule.Validation
bazel build //:docs //:ide_support //:needs_json //:metrics_json— no regression;cqueryconfirmsRunEnvironmentInfois forwarded and the binary transitions to@local_config_platform//:host.bazel build --config=per-x86_64-qnx -- //...no longer fails Python toolchain resolution (0 toolchain errors; only the unrelatedvalid_fromskew fixed by Set valid_from attribute for all features score#3064).buildifierclean.