This patch adds a Bazel configuration to build lld. That includes a
BUILD.bazel file to export the libunwind headers for use by lld. Since
the lld target itself requires libxml2 (through WindowsManifest) it's
currently disabled on Buildkite and marked manual, but all the libraries
build.
Details
- Reviewers
chandlerc MaskRay - Commits
- rG693a95a69416: [Bazel] Add support for lld
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
17 | Use single quotes for echo arguments to avoid some escapes. | |
23 | If LLD_REPOSITORY is undefined, LLD_REVISION doesn't need to be defined. | |
71 | Is *.h needed in srcs? | |
148 | In CMake, with LLVM_ENABLE_LIBXML2 this dependency is essentially a no-op. Can Bazel allow optional builds? | |
240 | Just MachO The special name can go to the old port which will go away soon. | |
305 | This comment is repeated multiple times. Is there a Bazel bug or anything we can fix in this file? |
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
71 | hdrs is only for headers that are exported for dependent libraries. Other headers should be declared in srcs: Headers not meant to be included by a client of this library should be listed in the srcs attribute instead, even if they are included by a published header. https://docs.bazel.build/versions/main/be/c-cpp.html#cc_library.hdrs Actually I noticed that the MachO2 headers should move to srcs as well. | |
148 | I don't totally understand the question. What do you mean by optional builds? There's configurability based on select | |
240 | This matches the name in the CMake file (as do all the library names). I'd prefer not to diverge. Having these match makes it much easier to compare the Bazel and CMake rules. I'm happy to update this once the CMake name changes. | |
305 | Filed https://github.com/bazelbuild/bazel/issues/13803 and linked to it instead |
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
240 | https://reviews.llvm.org/D107516 touche :-P |
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
148 | 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. |
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
148 | 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 |
Thanks!
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
148 | This manual/nobuildkite is existing, I think having the tags here is fine and can be fixed along with the other manual/nobuildkite targets. |
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
363 | I think the canonical spelling is macOS |
- Use silly macOS spelling and remove some unnecessary subjunctives
utils/bazel/llvm-project-overlay/lld/BUILD.bazel | ||
---|---|---|
363 | Fine I'll use that as long as we can agree that that's silly |
Use single quotes for echo arguments to avoid some escapes.