From 80b51b36d6f32ace823804541b23e5427e9ded86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Wed, 24 Apr 2024 14:16:57 -0400 Subject: [PATCH] Allow `bzl_library` to depend on non-`bzl_library` targets (#495) 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 --- bzl_library.bzl | 5 +--- docs/bzl_library.md | 2 +- tests/bzl_library/BUILD | 32 +++++++++++++++++++++ tests/bzl_library/a.bzl | 3 ++ tests/bzl_library/b.bzl | 3 ++ tests/bzl_library/bzl_library_test.bzl | 40 ++++++++++++++++++++++++++ tests/bzl_library/c.bzl | 6 ++++ tests/subpackages_tests.bzl | 2 ++ 8 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 tests/bzl_library/BUILD create mode 100644 tests/bzl_library/a.bzl create mode 100644 tests/bzl_library/b.bzl create mode 100644 tests/bzl_library/bzl_library_test.bzl create mode 100644 tests/bzl_library/c.bzl diff --git a/bzl_library.bzl b/bzl_library.bzl index 9091894..a139386 100644 --- a/bzl_library.bzl +++ b/bzl_library.bzl @@ -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`.""", ), }, diff --git a/docs/bzl_library.md b/docs/bzl_library.md index f4b0dc7..b826e5f 100755 --- a/docs/bzl_library.md +++ b/docs/bzl_library.md @@ -63,7 +63,7 @@ Example: | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | List of other bzl_library targets that are required by the Starlark files listed in srcs. | List of labels | optional | [] | +| deps | List of other bzl_library or filegroup targets that are required by the Starlark files listed in srcs. | List of labels | optional | [] | | srcs | List of .bzl and .scl files that are processed to create this target. | List of labels | optional | [] | diff --git a/tests/bzl_library/BUILD b/tests/bzl_library/BUILD new file mode 100644 index 0000000..6f8ae5c --- /dev/null +++ b/tests/bzl_library/BUILD @@ -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", +) diff --git a/tests/bzl_library/a.bzl b/tests/bzl_library/a.bzl new file mode 100644 index 0000000..01be5e0 --- /dev/null +++ b/tests/bzl_library/a.bzl @@ -0,0 +1,3 @@ +"""a.bzl, livin' its best life""" + +A = 30 diff --git a/tests/bzl_library/b.bzl b/tests/bzl_library/b.bzl new file mode 100644 index 0000000..254ca45 --- /dev/null +++ b/tests/bzl_library/b.bzl @@ -0,0 +1,3 @@ +"""b.bzl, havin' a grand time""" + +B = 70 diff --git a/tests/bzl_library/bzl_library_test.bzl b/tests/bzl_library/bzl_library_test.bzl new file mode 100644 index 0000000..1e01a29 --- /dev/null +++ b/tests/bzl_library/bzl_library_test.bzl @@ -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", + ), + }, +) diff --git a/tests/bzl_library/c.bzl b/tests/bzl_library/c.bzl new file mode 100644 index 0000000..1380ece --- /dev/null +++ b/tests/bzl_library/c.bzl @@ -0,0 +1,6 @@ +"""c.bzl, standin' on the shoulder of giants""" + +load(":a.bzl", "A") +load(":b.bzl", "B") + +C = A + B diff --git a/tests/subpackages_tests.bzl b/tests/subpackages_tests.bzl index 3c494d6..885d472 100644 --- a/tests/subpackages_tests.bzl +++ b/tests/subpackages_tests.bzl @@ -21,6 +21,7 @@ def _all_test(env): """Unit tests for subpackages.all.""" all_pkgs = [ + "bzl_library", "common_settings", "copy_directory", "copy_file", @@ -39,6 +40,7 @@ def _all_test(env): # These exist in all cases filtered_pkgs = [ + "bzl_library", "common_settings", "copy_directory", "copy_file",