[PATCH] D107414: [Bazel] Add support for lld

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!



================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:147
+        "//llvm:TransformUtils",
+        "//llvm:WindowsManifest",
+    ],
----------------
GMNGeoffrey wrote:
> MaskRay wrote:
> > GMNGeoffrey wrote:
> > > MaskRay wrote:
> > > > In CMake, with `LLVM_ENABLE_LIBXML2` this dependency is essentially a no-op. Can Bazel allow optional builds?
> > > I don't totally understand the question. What do you mean by optional builds? There's configurability based on select
> > You said "Since the lld target itself requires libxml2 (through WindowsManifest) it's currently disabled on Buildkite and marked manual, but all the libraries build."
> > 
> > I think it'd be good to drop `tags = ["manual"]`. In CMake, `WindowsManifest` is built regardless of availability of libxml2.
> > When libxml2 is disabled, `WindowsManifest` is a stub. 
> Hmm @chandlerc added `-lxml2` to WindowsManifest linkopts unconditionally, so we have everything that depends on it marked "manual" and "nobuildkite" (because our RBE runners don't have libxml2). I would be totally fine with removing that, but I think Chandler put it there for a reason (there's a comment about mt.exe not being cross-platform). We do have `LLVM_ENABLE_LIBXML2` hardcoded to false though (for now, we need to enable more configurability), so honestly I'm a bit confused
> 
> https://github.com/llvm/llvm-project/blob/597e407cf23b92/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel#L2385
> https://github.com/llvm/llvm-project/blob/597e407cf23b92/utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h#L325
This manual/nobuildkite is existing, I think having the tags here is fine and can be fixed along with the other manual/nobuildkite targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107414