[PATCH] D112883: [bazel] Re-introduce `copts` hacks for lib/AST includes.

Mon Nov 1 18:02:10 PDT 2021

GMNGeoffrey added a comment.

In D112883#3101665 <https://reviews.llvm.org/D112883#3101665>, @chandlerc wrote:

> In D112883#3101645 <https://reviews.llvm.org/D112883#3101645>, @GMNGeoffrey wrote:
>
>> :-((((( Did you try the separate library with `strip_include_prefix`?
>
> I can add a separate library to do that.
>
> It would still expose all consumers to `Opcodes.inc` instead of only exposing *this* library to it. If we can afford to have the dependency on the repository name, I feel like the current solution is actually a cleaner workaround. But if we genuinely cannot, then we could revisit this.

Yeah there's really no great way. Bazel has helpfully classified https://github.com/bazelbuild/bazel/issues/13803 as P4 <https://reviews.llvm.org/P4>.

> However, I thought with Bazel having a consistent repo name was important to allow cross-repo dependencies to reliably resolve, and so maybe this isn't that bad of a thing to rely on?

I think cross-repo dependencies just tend to only work by chance. I haven't seen any specific guidance for how to name repository, so I don't think it can really be relied upon. Bazel has functionality in `local_repository` specifically to remap repository names (https://docs.bazel.build/versions/main/be/workspace.html#local_repository.repo_mapping), but of course it's only available in local_repository and there's no way to compose it.

> Is this breaking actual users? I wouldn't expect it to break unless they depend on this target.

I don't know :-) I just know that people have sent me patches to remove the repository name dependence. I think https://github.com/llvm/llvm-project/blob/d1fdd745d510f40d8741d44ce39f5ae24ee7f91a/utils/bazel/llvm-project-overlay/clang/BUILD.bazel#L1460 is the only previous instance in clang. That gets used only in "frontend". I'm not sure if there are people using "ast" but not "frontend"

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112883