[PATCH] D90352: Introduce a Bazel build configuration

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey added inline comments.


================
Comment at: utils/bazel/WORKSPACE:15
+
+load(":terminfo.bzl", "llvm_terminfo_from_env")
+
----------------
csigg wrote:
> Downstream projects need to replicate everything in this WORKSPACE file, and it would therefore be good to keep it small.
> 
> My recommendation would be to move loading of external dependencies to llvm_configure, with the exception of
> - load/call bazel_skylib_workspace(): this initializes a unittest toolchain and downstream project do not need it to depend on LLVM (I'm not even sure this repo uses it)
> - load/call rbe_autoconfig(): downstream projects would need to replicate that plus copy and adjust the .bazelrc if they want to use a similar RBE setup.
I actually mostly disagree for this case. All of the things that are configurable here are deliberately configurable. In fact we would not recommend that a user use the *_from_env versions of the rules that provide these repositories, as they likely just want to use a single version. Some of these dependencies are also optional. We also want to make sure that users are conscious of their dependencies (Chandler, in particular, pointed out that some of this has implications for licensing). 

LLVM does not pull in new dependencies often, so I don't think this will create much churn for downstream users. As an example of what this looks like, this is what IREE needs to do to configure it's dependency: https://github.com/google/iree/blob/main/WORKSPACE#L42-L69


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90352