[PATCH] D90352: Introduce a Bazel build configuration

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

GMNGeoffrey added inline comments.


================
Comment at: utils/bazel/README.md:29
+   you don't have a checkout yet.
+2. Install Bazel at the version indicated by [.bazelversion](./bazelversion),
+   following the official instructions, if you don't have it installed yet:
----------------
keith wrote:
> should this mention bazelisk?
I don't want to repeat Bazel docs or be too opinionated about how exactly someone gets this. Personally I don't use bazelisk and instead use the wrapper shell script that's been around for a while that delegates to the appropriate bazel version (but requires that it be installed). Bazelisk is mentioned in the Bazel install docs we link to, so I think it's ok to defer to them. WDYT?


================
Comment at: utils/bazel/llvm-project-overlay/llvm/binary_alias.bzl:25
+
+binary_alias = rule(
+    _binary_alias_impl,
----------------
keith wrote:
> skylib has native_binary which appears to do the same thing as this custom rule
Oh nice! That looks relatively new. Let me look into swapping that in. I think currently the main repo doesn't have a dependency on skylib because our only usage in the project was for the config diff tests, which dependent users don't need and it seemed like an unnecessary dependency. This would maybe be worth tipping us over the edge.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90352