tweag / rules_sh

Shell rules for Bazel
Apache License 2.0
42 stars 3 forks source link

Allow disabling the toolchain `sh_configure` creates under bzlmod #50

Closed rrbutani closed 11 months ago

rrbutani commented 11 months ago

Motivation

I have a use case where I want to register my own posix toolchain and don't want Bazel to fall back to using the local_posix toolchain (the toolchain I'm making has certain constraints — when they aren't met I want Bazel to error).

sh_posix_configure provides an easy way to skip registering the local toolchain at @local_posix_config//:local_posix_toolchain but using rules_sh via bzlmod does not expose a way to configure this:

https://github.com/tweag/rules_sh/blob/0e3d91d962ebbbc2425a6414d0052ff43d853149/MODULE.bazel#L14

A Solution

While we can condition the register_toolchains invocation [^1] in MODULE.bazel (i.e. register_toolchains(...) if <condition> else None is permitted), unfortunately, as far as I know, there isn't a way to read data from module extension tag classes within MODULE.bazel.

This PR instead has the sh_configure module extension conditionally generate the @local_posix_config//:local_posix_toolchain toolchain and alters MODULE.bazel to register @local_posix_config//... (empty when the toolchain isn't created).

[^1]: additionally, only register_toolchains invocations from within MODULE.bazel files are honored


This PR adds a local_posix_config tag class to sh_configure. This allows users to add snippets like this to their MODULE.bazel file to configure whether the local posix toolchain is generated and registered:

sh_configure = use_extension("@rules_sh//bzlmod:extensions.bzl", "sh_configure")
sh_configure.local_posix_config(enable = False)

If no tags are specified, the toolchain is generated (making this is a backwards compatible change).

If multiple tags are specified with different values for enable, the extension emits a warning and takes the first value in module_ctx.modules (i.e. first tag of the first module reached via BFS from the root module):

DEBUG: /build/.cache/bazel/_bazel/fd74e05c5349be765d3f152fca12c623/external/rules_sh~override/bzlmod/extensions.bzl:41:14: Mismatched values for `sh_configure.local_posix_config.enable`:
  - module `bzlmod_test` (version: 0.0.1) specifies:
    * True
    * False
  - module `rules_sh` (version: 0.3.0) specifies:
    * False

Using `True` from `bzlmod_test` (version: 0.0.1) — this is the first module specifying this tag that's reached via BFS from the root module.

Open Questions/Notes

avdv commented 11 months ago

Hi @rrbutani, thank you for your contribution. I think your approach is reasonable, and skipping the creation of the toolchain instead of its registration is OK for me.

avdv commented 11 months ago

@mergifyio rebase

mergify[bot] commented 11 months ago

rebase

✅ Branch has been successfully rebased

rrbutani commented 11 months ago

My bad, had an extra closing quote on one of the doc strings; CI should (hopefully) pass now? :crossed_fingers: