[PATCH] D137008: [Bazel] Add bzlmod support

[Aaron Siddhartha Mondal via Phabricator via llvm-commits]

aaronmondal planned changes to this revision.
aaronmondal added inline comments.


================
Comment at: utils/bazel/extensions.bzl:34
+
+def _llvm_configure_extension_impl(ctx):
+    targets = []
----------------
matts1 wrote:
> FYI, since I wasn't aware of your implementation when I was working on this yesterday, I wrote my own implementation. I'll add the code below, so feel free to steal whatever seems good from it.
> 
> ```
> load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file")
> load("@bazel_tools//tools/build_defs/repo:git.bzl", "new_git_repository")
> load("//repos:non_bcr_deps.bzl", "non_bcr_deps")
> load("//:configure.bzl", "llvm_disable_optional_support_deps", "llvm_use_system_support_deps", _llvm_configure_repo = "llvm_configure")
> load("//:terminfo.bzl", "llvm_terminfo_disable", "llvm_terminfo_system")
> load("//:vulkan_sdk.bzl", "vulkan_sdk_setup")
> load("//:zlib.bzl", "llvm_zlib_disable", "llvm_zlib_external", "llvm_zlib_system")
> 
> _ZLIB_NAME = "llvm_zlib"
> _TERMINFO_NAME = "llvm_terminfo"
> 
> def _llvm_configure_impl(module_ctx):
>     root = module_ctx.modules[0]
>     tag = root.tags.configure[0]
> 
>     non_bcr_deps()
> 
>     if tag.zlib_strategy == "external":
>         llvm_zlib_external(name = _ZLIB_NAME, external_zlib = "@zlib")
>     elif tag.zlib_strategy == "system":
>         llvm_zlib_system(name = _ZLIB_NAME)
>     elif tag.zlib_strategy == "disabled":
>         llvm_zlib_disable(name = _ZLIB_NAME)
>     else:
>         fail("zlib_strategy must be one of 'external', 'system', and 'disabled'")
> 
>     if tag.terminfo_strategy == "system":
>         llvm_terminfo_system(name = _TERMINFO_NAME)
>     elif tag.terminfo_strategy == "disabled":
>         llvm_terminfo_disable(name = _TERMINFO_NAME)
>     else:
>         fail("terminfo_strategy must be one of 'disabled' and 'system'")
> 
>     vulkan_sdk_setup(name = "vulkan_sdk")
> 
>     _llvm_configure_repo(name = tag.name)
> 
> _llvm_configure_tag = tag_class(attrs = dict(
>     name = attr.string(mandatory = True),
>     zlib_strategy = attr.string(mandatory = True, default = "disabled"),
>     terminfo_strategy = attr.string(default = "disabled"),
> ))
> 
> llvm_configure = module_extension(
>     implementation = _llvm_configure_impl,
>     tag_classes = dict(configure = _llvm_configure_tag),
> )
> ```
Thanks a lot for this! After the changes to zlib we con't need the custom zlib logic anymore, and if we remove terminfo from the external deps entirely we can also get rid of that. I feel like your patch at https://reviews.llvm.org/D150641 was the missing piece to make all of this fall into place quite nicely :) I propose that we first remove terminfo, then get you patch in and then move forward with the patch here to finally add bzlmod support natively.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137008/new/

https://reviews.llvm.org/D137008