This is an archive of the discontinued LLVM Phabricator instance.

Simplify setting up LLVM as bazel external repo
ClosedPublic

Authored by csigg on Aug 8 2021, 6:19 AM.

Details

Summary

Only require one intermediate repository instead of two.
Fewer parameters in llvm_config.
Remove bazel_skylib dependency.

Diff Detail

Event Timeline

csigg created this revision.Aug 8 2021, 6:19 AM
csigg requested review of this revision.Aug 8 2021, 6:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2021, 6:19 AM
csigg updated this revision to Diff 365018.Aug 8 2021, 6:21 AM

Remove no longer needed attr.

csigg updated this revision to Diff 365023.Aug 8 2021, 7:25 AM

Update WORKSPACE file.

csigg updated this revision to Diff 365028.Aug 8 2021, 8:55 AM

Add back bazel_skylib, it's needed by the tests.

goncharov accepted this revision.Aug 10 2021, 7:47 AM
goncharov added a subscriber: goncharov.

Hi @csigg ! LGTM, and tests also pass. I don't have much experience with bazel though :)

This revision is now accepted and ready to land.Aug 10 2021, 7:47 AM

Nice! This is much better. I think some of the workspace munging came from when this rule was more general (overlay any directory over any other). I still think such functionality is useful, but not at the cost of making this API really hard to use.

A few nits though. Sorry I was out and couldn't review :-)

utils/bazel/configure.bzl
36

I think this could just be repository_ctx.path(".")

utils/bazel/examples/http_archive/WORKSPACE
12

We should update the commit here so it's one that includes this change. For future reference, I think it would've been better to leave this example as-is, since it is still a working example for the given LLVM commit and then follow up with an update for your commit (since it's impossible to know the commit hash before it's committed)

13

bazel_skylib is still a required dependency for tblgen, although we could go back to manual path manipulation to avoid the dependency

csigg updated this revision to Diff 366831.Aug 17 2021, 1:42 AM

Undo example changes, they should be submitted separately.

csigg updated this revision to Diff 366890.EditedAug 17 2021, 7:11 AM

Revert back to repository_ctx.path(Label("//:WORKSPACE")).dirname.

repository_ctx.path(".") simply resolves to ".".
Adding .realpath doesn't help either, because that then resolves to the root of the repository we are creating (e.g. llvm-project), instead of the source repo (e.g. llvm-archive).

This revision was automatically updated to reflect the committed changes.

I actually just tested this further and it breaks resolution of deps_impl. This is yet another case where Bazel makes it really hard to understand what labels are being resolved relative to...

~/src/llvm-project/utils/bazel/examples/http_archive$ bazel build @llvm-project//llvm:llvm-symbolizer
INFO: Invocation ID: b5fb0254-073a-4ef5-b9f3-6dcaad9ac98f
INFO: Repository llvm_zlib instantiated at:
  /usr/local/google/home/gcmn/src/llvm-project/utils/bazel/examples/http_archive/WORKSPACE:45:35: in <toplevel>
  /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-archive/utils/bazel/configure.bzl:93:10: in llvm_disable_optional_support_deps
  /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/bazel_tools/tools/build_defs/repo/utils.bzl:201:18: in maybe
Repository rule llvm_zlib_disable defined at:
  /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-archive/utils/bazel/zlib.bzl:72:36: in <toplevel>
INFO: Repository llvm_terminfo instantiated at:
  /usr/local/google/home/gcmn/src/llvm-project/utils/bazel/examples/http_archive/WORKSPACE:45:35: in <toplevel>
  /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-archive/utils/bazel/configure.bzl:98:10: in llvm_disable_optional_support_deps
  /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/bazel_tools/tools/build_defs/repo/utils.bzl:201:18: in maybe
Repository rule llvm_terminfo_disable defined at:
  /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-archive/utils/bazel/terminfo.bzl:35:40: in <toplevel>
ERROR: An error occurred during the fetch of repository 'llvm_zlib':
   Traceback (most recent call last):
        File "/usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-archive/utils/bazel/zlib.bzl", line 66, column 28, in _llvm_zlib_disable_impl
                repository_ctx.template(
Error in template: Unable to load package for @llvm-archive//deps_impl:zlib_disable.BUILD: BUILD file not found in directory 'deps_impl' of external repository @llvm-archive. Add a BUILD file to a directory to mark it as a package.
ERROR: Error fetching repository: Traceback (most recent call last):
        File "/usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-archive/utils/bazel/zlib.bzl", line 66, column 28, in _llvm_zlib_disable_impl
                repository_ctx.template(
Error in template: Unable to load package for @llvm-archive//deps_impl:zlib_disable.BUILD: BUILD file not found in directory 'deps_impl' of external repository @llvm-archive. Add a BUILD file to a directory to mark it as a package.
ERROR: /usr/local/google/home/gcmn/.cache/bazel/_bazel_gcmn/862d4f35edba4a3b7a8ffb6f5ccce305/external/llvm-project/llvm/BUILD.bazel:170:11: @llvm-project//llvm:Support depends on @llvm_zlib//:zlib in repository @llvm_zlib which failed to fetch. no such package '@llvm_zlib//': Unable to load package for @llvm-archive//deps_impl:zlib_disable.BUILD: BUILD file not found in directory 'deps_impl' of external repository @llvm-archive. Add a BUILD file to a directory to mark it as a package.
ERROR: Analysis of target '@llvm-project//llvm:llvm-symbolizer' failed; build aborted: Analysis failed
INFO: Elapsed time: 3.309s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (15 packages loaded, 1015 targets configured)