From 11fd29bd88e2ead6fd6cf32a2fbf2399bd0ce700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Tue, 23 Apr 2024 16:29:27 -0400 Subject: [PATCH 1/7] 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. --- bzl_library.bzl | 5 +---- tests/bzl_library/BUILD | 32 ++++++++++++++++++++++++++++++++ tests/bzl_library/a.bzl | 1 + tests/bzl_library/b.bzl | 2 ++ 4 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 tests/bzl_library/BUILD create mode 100644 tests/bzl_library/a.bzl create mode 100644 tests/bzl_library/b.bzl diff --git a/bzl_library.bzl b/bzl_library.bzl index 9091894f..a139386e 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/tests/bzl_library/BUILD b/tests/bzl_library/BUILD new file mode 100644 index 00000000..5210d073 --- /dev/null +++ b/tests/bzl_library/BUILD @@ -0,0 +1,32 @@ +load("//:bzl_library.bzl", "bzl_library") +load("//rules:build_test.bzl", "build_test") + +bzl_library( + name = "a_bl", + srcs = ["a.bzl"], +) + +bzl_library( + name = "b_bl", + srcs = ["b.bzl"], + deps = [":a_bl"], +) + +filegroup( + name = "a_fg", + srcs = ["a.bzl"], +) + +bzl_library( + name = "b_fg", + srcs = ["b.bzl"], + deps = [":a_fg"], +) + +build_test( + name = "test_bzl_library", + targets = [ + ":b_bl", + ":b_fg", + ], +) diff --git a/tests/bzl_library/a.bzl b/tests/bzl_library/a.bzl new file mode 100644 index 00000000..7e90de38 --- /dev/null +++ b/tests/bzl_library/a.bzl @@ -0,0 +1 @@ +A = 30 diff --git a/tests/bzl_library/b.bzl b/tests/bzl_library/b.bzl new file mode 100644 index 00000000..a486e208 --- /dev/null +++ b/tests/bzl_library/b.bzl @@ -0,0 +1,2 @@ +load(":a.bzl", "A") +B = A + 70 From 9a6d472ba43e4df89e780155b6c4ae199b33cccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Tue, 23 Apr 2024 16:34:49 -0400 Subject: [PATCH 2/7] buildifier --- tests/bzl_library/a.bzl | 2 ++ tests/bzl_library/b.bzl | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/bzl_library/a.bzl b/tests/bzl_library/a.bzl index 7e90de38..01be5e09 100644 --- a/tests/bzl_library/a.bzl +++ b/tests/bzl_library/a.bzl @@ -1 +1,3 @@ +"""a.bzl, livin' its best life""" + A = 30 diff --git a/tests/bzl_library/b.bzl b/tests/bzl_library/b.bzl index a486e208..f14cf6d1 100644 --- a/tests/bzl_library/b.bzl +++ b/tests/bzl_library/b.bzl @@ -1,2 +1,4 @@ +"""b.bzl, which loads from a.bzl""" + load(":a.bzl", "A") B = A + 70 From d4cd8bfe9b88df10f2da0d72da916ee7729a358f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Tue, 23 Apr 2024 16:37:02 -0400 Subject: [PATCH 3/7] fix tests --- docs/bzl_library.md | 2 +- tests/subpackages_tests.bzl | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/bzl_library.md b/docs/bzl_library.md index f4b0dc7e..b826e5f9 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/subpackages_tests.bzl b/tests/subpackages_tests.bzl index 3c494d68..885d4724 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", From ad07db22ae5992786ca346be60fde28d30209755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Tue, 23 Apr 2024 16:39:02 -0400 Subject: [PATCH 4/7] BUILDIFIER --- tests/bzl_library/b.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bzl_library/b.bzl b/tests/bzl_library/b.bzl index f14cf6d1..9a96ac9b 100644 --- a/tests/bzl_library/b.bzl +++ b/tests/bzl_library/b.bzl @@ -1,4 +1,5 @@ """b.bzl, which loads from a.bzl""" load(":a.bzl", "A") + B = A + 70 From 63ad23162ff06fd467d67229678200d17b9fe74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Wed, 24 Apr 2024 13:59:10 -0400 Subject: [PATCH 5/7] better test setup --- tests/bzl_library/BUILD | 30 +++++++++++----------- tests/bzl_library/b.bzl | 6 ++--- tests/bzl_library/bzl_library_consumer.bzl | 20 +++++++++++++++ tests/bzl_library/c.bzl | 6 +++++ 4 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 tests/bzl_library/bzl_library_consumer.bzl create mode 100644 tests/bzl_library/c.bzl diff --git a/tests/bzl_library/BUILD b/tests/bzl_library/BUILD index 5210d073..bef98773 100644 --- a/tests/bzl_library/BUILD +++ b/tests/bzl_library/BUILD @@ -1,32 +1,32 @@ +load(":bzl_library_consumer.bzl", "bzl_library_consumer") load("//:bzl_library.bzl", "bzl_library") load("//rules:build_test.bzl", "build_test") -bzl_library( - name = "a_bl", +filegroup( + name = "a", srcs = ["a.bzl"], ) bzl_library( - name = "b_bl", + name = "b", srcs = ["b.bzl"], - deps = [":a_bl"], ) -filegroup( - name = "a_fg", - srcs = ["a.bzl"], +bzl_library( + name = "c", + srcs = ["c.bzl"], + deps = [ + ":a", + ":b", + ], ) -bzl_library( - name = "b_fg", - srcs = ["b.bzl"], - deps = [":a_fg"], +bzl_library_consumer( + name = "consumer", + target = ":c", ) build_test( name = "test_bzl_library", - targets = [ - ":b_bl", - ":b_fg", - ], + targets = [":consumer"], ) diff --git a/tests/bzl_library/b.bzl b/tests/bzl_library/b.bzl index 9a96ac9b..254ca450 100644 --- a/tests/bzl_library/b.bzl +++ b/tests/bzl_library/b.bzl @@ -1,5 +1,3 @@ -"""b.bzl, which loads from a.bzl""" +"""b.bzl, havin' a grand time""" -load(":a.bzl", "A") - -B = A + 70 +B = 70 diff --git a/tests/bzl_library/bzl_library_consumer.bzl b/tests/bzl_library/bzl_library_consumer.bzl new file mode 100644 index 00000000..a18ae078 --- /dev/null +++ b/tests/bzl_library/bzl_library_consumer.bzl @@ -0,0 +1,20 @@ +"""A rule that consumes a bzl_library target and asserts its providers are as expected.""" + +load("//:bzl_library.bzl", "StarlarkLibraryInfo") + +def _bzl_library_consumer_impl(ctx): + files = sorted([t.basename for t in ctx.attr.target.files.to_list()]) + if files != ["a.bzl", "b.bzl", "c.bzl"]: + fail("unexpected filenames from DefaultInfo: ", files) + files = sorted([t.basename for t in ctx.attr.target[StarlarkLibraryInfo].transitive_srcs.to_list()]) + if files != ["a.bzl", "b.bzl", "c.bzl"]: + fail("unexpected filenames from StarlarkLibraryInfo: ", files) + +bzl_library_consumer = rule( + implementation = _bzl_library_consumer_impl, + attrs = { + "target": attr.label( + providers = [[StarlarkLibraryInfo]], + ), + }, +) diff --git a/tests/bzl_library/c.bzl b/tests/bzl_library/c.bzl new file mode 100644 index 00000000..1380ece5 --- /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 From 5badc994f23e0264c1725fbd0f5e2f07e2ff2196 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:03:08 -0400 Subject: [PATCH 6/7] fix load order --- tests/bzl_library/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bzl_library/BUILD b/tests/bzl_library/BUILD index bef98773..95d16bb1 100644 --- a/tests/bzl_library/BUILD +++ b/tests/bzl_library/BUILD @@ -1,6 +1,6 @@ -load(":bzl_library_consumer.bzl", "bzl_library_consumer") load("//:bzl_library.bzl", "bzl_library") load("//rules:build_test.bzl", "build_test") +load(":bzl_library_consumer.bzl", "bzl_library_consumer") filegroup( name = "a", From 015248d6a51f514adc1e7f8a60d722b42a2348ac Mon Sep 17 00:00:00 2001 From: Alexandre Rostovtsev Date: Wed, 24 Apr 2024 14:04:38 -0400 Subject: [PATCH 7/7] Implement bzl_library test using standard analysistest machinery --- tests/bzl_library/BUILD | 20 +++++------ tests/bzl_library/bzl_library_consumer.bzl | 20 ----------- tests/bzl_library/bzl_library_test.bzl | 40 ++++++++++++++++++++++ 3 files changed, 50 insertions(+), 30 deletions(-) delete mode 100644 tests/bzl_library/bzl_library_consumer.bzl create mode 100644 tests/bzl_library/bzl_library_test.bzl diff --git a/tests/bzl_library/BUILD b/tests/bzl_library/BUILD index 95d16bb1..6f8ae5cf 100644 --- a/tests/bzl_library/BUILD +++ b/tests/bzl_library/BUILD @@ -1,6 +1,5 @@ load("//:bzl_library.bzl", "bzl_library") -load("//rules:build_test.bzl", "build_test") -load(":bzl_library_consumer.bzl", "bzl_library_consumer") +load(":bzl_library_test.bzl", "bzl_library_test") filegroup( name = "a", @@ -21,12 +20,13 @@ bzl_library( ], ) -bzl_library_consumer( - name = "consumer", - target = ":c", -) - -build_test( - name = "test_bzl_library", - targets = [":consumer"], +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/bzl_library_consumer.bzl b/tests/bzl_library/bzl_library_consumer.bzl deleted file mode 100644 index a18ae078..00000000 --- a/tests/bzl_library/bzl_library_consumer.bzl +++ /dev/null @@ -1,20 +0,0 @@ -"""A rule that consumes a bzl_library target and asserts its providers are as expected.""" - -load("//:bzl_library.bzl", "StarlarkLibraryInfo") - -def _bzl_library_consumer_impl(ctx): - files = sorted([t.basename for t in ctx.attr.target.files.to_list()]) - if files != ["a.bzl", "b.bzl", "c.bzl"]: - fail("unexpected filenames from DefaultInfo: ", files) - files = sorted([t.basename for t in ctx.attr.target[StarlarkLibraryInfo].transitive_srcs.to_list()]) - if files != ["a.bzl", "b.bzl", "c.bzl"]: - fail("unexpected filenames from StarlarkLibraryInfo: ", files) - -bzl_library_consumer = rule( - implementation = _bzl_library_consumer_impl, - attrs = { - "target": attr.label( - providers = [[StarlarkLibraryInfo]], - ), - }, -) diff --git a/tests/bzl_library/bzl_library_test.bzl b/tests/bzl_library/bzl_library_test.bzl new file mode 100644 index 00000000..1e01a296 --- /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", + ), + }, +)