[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