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 <[email protected]>

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`.""",
         ),
     },

docs/bzl_library.md

@@ -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> |
 
 

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",
+)

tests/bzl_library/a.bzl

@@ -0,0 +1,3 @@
+"""a.bzl, livin' its best life"""
+
+A = 30

tests/bzl_library/b.bzl

@@ -0,0 +1,3 @@
+"""b.bzl, havin' a grand time"""
+
+B = 70

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",
+        ),
+    },
+)

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

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",