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

Sun May 28 17:51:31 PDT 2023

matts1 added a comment.

In D150641#4369062 <https://reviews.llvm.org/D150641#4369062>, @GMNGeoffrey wrote:

> I would like to see refactoring like I described in https://reviews.llvm.org/D143320 that makes it easy for users to configure the dependencies they need.

I don't disagree here, but I'll leave that to a future patch, since this is supposed to be a no-op refactor.

> The distinction between BCR and non-BCR is not really relevant for anyone using WORKSPACE files, so I don't think it helps. This actually makes it harder for people using WORKSPACE files because it adds an additional non-meaningful layer of indirection. As mentioned there, the examples in https://github.com/llvm/llvm-project/tree/main/utils/bazel/examples should be updated to demonstrate how to actually use this. It's a bit tricky, since a commit has to be checked in before it can be referenced, but we should check that it will work before checking this in (using the pending commit) and then actually update the examples to point at the commit once checked in. It would be helpful to add a bzlmod example there as well (and we should add a `git_repository` one as well).

I hadn't considered this up until now. I was just assuming that users would still use the same method described in examples (which this change doesn't break - it still works). However, you raise a good point, so I've changed it to work the same way as most other repos (load the @<repo>, then call <repo>_deps to load the dependencies, then call <repo>_repos to execute our repo rules. This should make it much easier to load, but I'll leave that to a future CL.

> Generally, I'm not really sold on bzlmod actually fixing the problems with WORKSPACE files without creating new ones, so I don't want this to make using WORKSPACE files any harder (it already has with the zstd and zlib changes, which we should fix).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150641