Skip to content

Commit

Permalink
Allow bzl_library to depend on non-bzl_library targets (#495)
Browse files Browse the repository at this point in the history
Allow `bzl_library` to depend on non-`bzl_library` targets

Notably, `filegroup`. `bzl_library` doesn't actually read anything from the `StarlarkLibraryInfo` provider, and requiring all deps to be other `bzl_library` targets is really painful for anyone loading .bzls from `@bazel_tools` or `@platforms` because those core modules/repos don't want a dependency on Skylib just for access to `bzl_library`.

The medium-term plan will be to move `bzl_library` into `@bazel_tools`; but before then, this can serve as a stop-gap.

Co-authored-by: Alexandre Rostovtsev <arostovtsev@google.com>
  • Loading branch information
Wyverald and tetromino committed Apr 24, 2024
1 parent 054ebf5 commit 80b51b3
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 5 deletions.
5 changes: 1 addition & 4 deletions bzl_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ bzl_library = rule(
),
"deps": attr.label_list(
allow_files = [".bzl", ".scl"],
providers = [
[StarlarkLibraryInfo],
],
doc = """List of other `bzl_library` targets that are required by the
doc = """List of other `bzl_library` or `filegroup` targets that are required by the
Starlark files listed in `srcs`.""",
),
},
Expand Down
2 changes: 1 addition & 1 deletion docs/bzl_library.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Example:
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="bzl_library-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="bzl_library-deps"></a>deps | List of other <code>bzl_library</code> targets that are required by the Starlark files listed in <code>srcs</code>. | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | <code>[]</code> |
| <a id="bzl_library-deps"></a>deps | List of other <code>bzl_library</code> or <code>filegroup</code> targets that are required by the Starlark files listed in <code>srcs</code>. | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | <code>[]</code> |
| <a id="bzl_library-srcs"></a>srcs | List of <code>.bzl</code> and <code>.scl</code> files that are processed to create this target. | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | <code>[]</code> |


Expand Down
32 changes: 32 additions & 0 deletions tests/bzl_library/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
load("//:bzl_library.bzl", "bzl_library")
load(":bzl_library_test.bzl", "bzl_library_test")

filegroup(
name = "a",
srcs = ["a.bzl"],
)

bzl_library(
name = "b",
srcs = ["b.bzl"],
)

bzl_library(
name = "c",
srcs = ["c.bzl"],
deps = [
":a",
":b",
],
)

bzl_library_test(
name = "bzl_library_test",
expected_srcs = ["c.bzl"],
expected_transitive_srcs = [
"a.bzl",
"b.bzl",
"c.bzl",
],
target_under_test = ":c",
)
3 changes: 3 additions & 0 deletions tests/bzl_library/a.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"""a.bzl, livin' its best life"""

A = 30
3 changes: 3 additions & 0 deletions tests/bzl_library/b.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"""b.bzl, havin' a grand time"""

B = 70
40 changes: 40 additions & 0 deletions tests/bzl_library/bzl_library_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
"""Unit tests for bzl_library"""

load("//:bzl_library.bzl", "StarlarkLibraryInfo")
load("//lib:sets.bzl", "sets")
load("//lib:unittest.bzl", "analysistest", "asserts")

def _assert_same_files(env, expected_file_targets, actual_files):
"""Assertion that a list of expected file targets and an actual list or depset of files contain the same files"""
expected_files = []
for target in expected_file_targets:
target_files = target[DefaultInfo].files.to_list()
asserts.true(env, len(target_files) == 1, "expected_file_targets must contain only file targets")
expected_files.append(target_files[0])
if type(actual_files) == "depset":
actual_files = actual_files.to_list()
asserts.set_equals(env = env, expected = sets.make(expected_files), actual = sets.make(actual_files))

def _bzl_library_test_impl(ctx):
env = analysistest.begin(ctx)
target_under_test = analysistest.target_under_test(env)
_assert_same_files(env, ctx.attr.expected_srcs, target_under_test[StarlarkLibraryInfo].srcs)
_assert_same_files(env, ctx.attr.expected_transitive_srcs, target_under_test[StarlarkLibraryInfo].transitive_srcs)
_assert_same_files(env, ctx.attr.expected_transitive_srcs, target_under_test[DefaultInfo].files)
return analysistest.end(env)

bzl_library_test = analysistest.make(
impl = _bzl_library_test_impl,
attrs = {
"expected_srcs": attr.label_list(
mandatory = True,
allow_files = True,
doc = "Expected direct srcs in target_under_test's providers",
),
"expected_transitive_srcs": attr.label_list(
mandatory = True,
allow_files = True,
doc = "Expected transitive srcs in target_under_test's providers",
),
},
)
6 changes: 6 additions & 0 deletions tests/bzl_library/c.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""c.bzl, standin' on the shoulder of giants"""

load(":a.bzl", "A")
load(":b.bzl", "B")

C = A + B
2 changes: 2 additions & 0 deletions tests/subpackages_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def _all_test(env):
"""Unit tests for subpackages.all."""

all_pkgs = [
"bzl_library",
"common_settings",
"copy_directory",
"copy_file",
Expand All @@ -39,6 +40,7 @@ def _all_test(env):

# These exist in all cases
filtered_pkgs = [
"bzl_library",
"common_settings",
"copy_directory",
"copy_file",
Expand Down

0 comments on commit 80b51b3

Please sign in to comment.