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

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

GMNGeoffrey added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/lld/BUILD.bazel:147
+        "//llvm:TransformUtils",
+        "//llvm:WindowsManifest",
+    ],
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107414