[PATCH] D143295: [bazel] Move bazel configuration to a Python script

Thomas Preud'homme via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 20 08:39:07 PST 2023


thopre planned changes to this revision.
thopre added a comment.

In D143295#4121029 <https://reviews.llvm.org/D143295#4121029>, @chapuni wrote:

> I supposed `llvm_configure` is the only public entry for users.

It is, but its implementation function is not executed when using a --override_repository on the name associated with its repository_rule, as documented in tensorflow's MLIR readme. I failed to draw the right conclusion though: I now think the problem lies in that documentation. Tensorflow has a separate external repository for LLVM before the overlay and that's what should be overriden. I've created a PR against tensorflow to that effect. I'll keep this patch in "plan changes" state for the time being in case there is a reason for tensorflow to recommend overriding the repository defined by llvm_configure.

Thanks for all the feedback!

> I won't oppose if `overlay_directories.py` may be assumed as 2nd public entry.
>
> One concern. It seems this change tends to miss update of `llvm/CMakeLists.txt`.
> (for example, `bazel build` doesn't detect changes if `llvm/CMakeLists.txt` is changed.)
> That said, `utils/bazel` has been flaky for change of directory structure to be mirrored.
> We could run `bazel sync` to satisfy changes, I think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143295



More information about the llvm-commits mailing list