[PATCH] D150641: [bazel] Refactor entries in WORKSPACE to bzl files

[Aaron Siddhartha Mondal via Phabricator via llvm-commits]

aaronmondal added inline comments.


================
Comment at: utils/bazel/WORKSPACE:16
+# won't need to call bcr_deps, since we can just use the BCR directly.
+load("//repos:bcr_deps.bzl", "bcr_deps")
 
----------------
This (and the other `//repos`) occurrances being relative might be risky since we move buildfiles around a lot when applying the configure logic.

This keeps confusing me, so I might be wrong here, but I think we want this to be `@llvm-raw//utils/bazel/repos:bcr_deps.bzl` etc. to ensure that these files are found after applying the overlay.


================
Comment at: utils/bazel/repos/bcr_deps.bzl:12
+
+SKYLIB_VERSION = "1.3.0"
+
----------------
We should probably update this dep (in a later patch). This is ancient.


================
Comment at: utils/bazel/repos/bcr_deps.bzl:24-34
+    maybe(
+        http_archive,
+        name = "zlib",
+        build_file = "@llvm-raw//utils/bazel/third_party_build:zlib.BUILD",
+        sha256 = "91844808532e5ce316b3c010929493c0244f3d37593afd6de04f71821d5136d9",
+        strip_prefix = "zlib-1.2.12",
+        urls = [
----------------
This is superceded by the llvm_zlib (zlib-ng) dependency in non_bcr_deps.bzl.


================
Comment at: utils/bazel/repos/llvm_repos.bzl:13
+    llvm_configure(name = "llvm-project")
+
+    maybe(
----------------
Ahh such simplicity :D

It makes my heart hurt a bit that we still need the vulkan_sdk_setup, but that's on me. We can fix it later.


================
Comment at: utils/bazel/repos/non_bcr_deps.bzl:12-22
+def non_bcr_deps():
+    maybe(
+        http_archive,
+        name = "vulkan_headers",
+        build_file = "@llvm-raw//utils/bazel/third_party_build:vulkan_headers.BUILD",
+        sha256 = "19f491784ef0bc73caff877d11c96a48b946b5a1c805079d9006e3fbaa5c1895",
+        strip_prefix = "Vulkan-Headers-9bd3f561bcee3f01d22912de10bb07ce4e23d378",
----------------
Oh I overlooked this dependency. The current implementation doesn't match the config_setting style that we now use for all other dependencies, but the current implementation (in vulkan_sdk.bzl) doesn't seem to interfere with bzlmod. I think this should be fine to fix in a later patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150641