Remove PackageArtifactsInfo. (#752)

* Remove PackageArtifactsInfo. Fixes #396 RELNOTES: No rules return a PackageArtifactsInfo provider. Rules which return multiple files (such as a .rpm and a .changes) now exclusively distinguish them through OutputGroupInfo. --------- Co-authored-by: Alex Eagle <[email protected]>
bazelbuild · Sep 12, 2023 · 6dd6841 · 6dd6841
1 parent 19ef5a6
commit 6dd6841
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 54 deletions.
diff --git a/pkg/private/deb/deb.bzl b/pkg/private/deb/deb.bzl
@@ -13,7 +13,7 @@
 # limitations under the License.
 """Rule for creating Debian packages."""
 
-load("//pkg:providers.bzl", "PackageArtifactInfo", "PackageVariablesInfo")
+load("//pkg:providers.bzl", "PackageVariablesInfo")
 load("//pkg/private:util.bzl", "setup_output_files")
 
 _tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.bz2", "tar.xz"]
@@ -172,16 +172,23 @@ def _pkg_deb_impl(ctx):
             files = depset([output_file]),
             runfiles = ctx.runfiles(files = outputs),
         ),
-        PackageArtifactInfo(
-            label = ctx.label.name,
-            file = output_file,
-            file_name = output_name,
-        ),
     ]
 
 # A rule for creating a deb file, see README.md
 pkg_deb_impl = rule(
     implementation = _pkg_deb_impl,
+    doc = """
+    Create a Debian package.
+
+    This rule produces 2 artifacts: a .deb and a .changes file. The DefaultInfo will
+    include both. If you need downstream rule to specificially depend on only the .deb or
+    .changes file then you can use `filegroup` to select distinct output groups.
+
+    **OutputGroupInfo**
+    - `out` the Debian package or a symlink to the actual package.
+    - `deb` the package with any precise file name created with `package_file_name`.
+    - `changes` the .changes file.
+    """,
     attrs = {
         # @unsorted-dict-items
         "data": attr.label(
@@ -361,7 +368,6 @@ See https://www.debian.org/doc/debian-policy/ch-files.html#s-config-files.""",
             allow_files = True,
         ),
     },
-    provides = [PackageArtifactInfo],
 )
 
 def pkg_deb(name, out = None, **kwargs):

diff --git a/pkg/private/pkg_files.bzl b/pkg/private/pkg_files.bzl
@@ -34,7 +34,6 @@ Concepts and terms:
 load("//pkg:path.bzl", "compute_data_path", "dest_path")
 load(
     "//pkg:providers.bzl",
-    "PackageArtifactInfo",
     "PackageDirsInfo",
     "PackageFilegroupInfo",
     "PackageFilesInfo",

diff --git a/pkg/private/tar/tar.bzl b/pkg/private/tar/tar.bzl
@@ -14,7 +14,7 @@
 """Rules for making .tar files."""
 
 load("//pkg:path.bzl", "compute_data_path", "dest_path")
-load("//pkg:providers.bzl", "PackageArtifactInfo", "PackageVariablesInfo")
+load("//pkg:providers.bzl", "PackageVariablesInfo")
 load(
     "//pkg/private:pkg_files.bzl",
     "add_directory",
@@ -238,11 +238,6 @@ def _pkg_tar_impl(ctx):
         OutputGroupInfo(
             manifest = [manifest_file], 
         ),
-        PackageArtifactInfo(
-            label = ctx.label.name,
-            file = output_file,
-            file_name = output_name,
-        ),
     ]
 
 # A rule for creating a tar file, see README.md
@@ -305,7 +300,6 @@ The name may contain variables, same as [package_file_name](#package_file_name)"
             allow_files = True,
         ),
     },
-    provides = [PackageArtifactInfo],
 )
 
 def pkg_tar(name, **kwargs):

diff --git a/pkg/private/util.bzl b/pkg/private/util.bzl
@@ -29,10 +29,7 @@ def setup_output_files(ctx, package_file_name = None, default_output_file = None
     Callers should:
        - write to `output_file`
        - add `outputs` to their returned `DefaultInfo(files)` provider
-       - return a `PackageArtifactInfo` provider of the form:
-            label: `ctx.label.name`
-            file: `output_file`
-            file_name: `output_name`
+       - Possibly add a distinguishing element to OutputGroups
 
     Args:
       ctx: rule context

diff --git a/pkg/private/zip/zip.bzl b/pkg/private/zip/zip.bzl
@@ -16,7 +16,6 @@
 load("//pkg:path.bzl", "compute_data_path", "dest_path")
 load(
     "//pkg:providers.bzl",
-    "PackageArtifactInfo",
     "PackageVariablesInfo",
 )
 load(
@@ -83,11 +82,6 @@ def _pkg_zip_impl(ctx):
             files = depset([output_file]),
             runfiles = ctx.runfiles(files = outputs),
         ),
-        PackageArtifactInfo(
-            label = ctx.label.name,
-            file = output_file,
-            file_name = output_name,
-        ),
     ]
 
 pkg_zip_impl = rule(
@@ -160,7 +154,6 @@ The list of compressions is the same as Python's ZipFile: https://docs.python.or
             allow_files = True,
         ),
     },
-    provides = [PackageArtifactInfo],
 )
 
 def pkg_zip(name, out = None, **kwargs):

diff --git a/pkg/rpm_pfg.bzl b/pkg/rpm_pfg.bzl
@@ -27,7 +27,6 @@ find_system_rpmbuild(name="rules_pkg_rpmbuild")
 
 load(
     "//pkg:providers.bzl",
-    "PackageArtifactInfo",
     "PackageDirsInfo",
     "PackageFilegroupInfo",
     "PackageFilesInfo",
@@ -678,11 +677,6 @@ def _pkg_rpm_impl(ctx):
         DefaultInfo(
             files = depset(outputs),
         ),
-        PackageArtifactInfo(
-            file = output_file,
-            file_name = output_name,
-            label = ctx.label.name,
-        ),
     ]
 
 # Define the rule.
@@ -715,6 +709,14 @@ pkg_rpm = rule(
 
     Is the equivalent to `%config(missingok, noreplace)` in the `%files` list.
 
+    This rule produces 2 artifacts: an .rpm and a .changes file. The DefaultInfo will
+    include both. If you need downstream rule to specificially depend on only the .rpm or
+    .changes file then you can use `filegroup` to select distinct output groups.
+
+    **OutputGroupInfo**
+    - `out` the RPM or a symlink to the actual package.
+    - `rpm` the package with any precise file name created with `package_file_name`.
+    - `changes` the .changes file.
     """,
     # @unsorted-dict-items
     attrs = {
@@ -1022,6 +1024,5 @@ pkg_rpm = rule(
     },
     executable = False,
     implementation = _pkg_rpm_impl,
-    provides = [PackageArtifactInfo],
     toolchains = ["@rules_pkg//toolchains/rpm:rpmbuild_toolchain_type"],
 )
diff --git a/tests/rpm/analysis_tests.bzl b/tests/rpm/analysis_tests.bzl
@@ -22,7 +22,7 @@ load(
     "pkg_mklink",
 )
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
-load("//pkg:providers.bzl", "PackageArtifactInfo", "PackageVariablesInfo")
+load("//pkg:providers.bzl", "PackageVariablesInfo")
 load("//pkg:rpm.bzl", "pkg_rpm")
 load("//tests/util:defs.bzl", "directory", "generic_base_case_test", "generic_negative_test")
 
@@ -167,42 +167,48 @@ def _package_naming_test_impl(ctx):
     env = analysistest.begin(ctx)
     target_under_test = analysistest.target_under_test(env)
 
-    pai = target_under_test[PackageArtifactInfo]
+    ogi = target_under_test[OutputGroupInfo]
 
-    pai_file = pai.file
-    pai_name = pai.file_name
+    out_file = ogi.out.to_list()[0]
+    rpm_file = ogi.rpm.to_list()[0]
+    changes_file = ogi.changes.to_list()[0] if ogi.changes else None
     asserts.equals(
         env,
-        pai_name,
+        rpm_file.basename,
         ctx.attr.expected_name,
-        "PackageArtifactInfo file name does not match expected value.",
+        "OutputGroupInfo rpm name does not match expected value.",
     )
 
-    # Try to find the expected files in the DefaultInfo.  We have to look for
-    # them; PackageArtifactInfo only gives a file name, not a File structure.
-    packaged_file = None
-    packaged_file_found = False
+    # Try to find the expected files in the DefaultInfo.
+    out_file_found = False
+    rpm_file_found = False
+    changes_file_found = False if changes_file else True
     default_name_found = False
     for f in target_under_test[DefaultInfo].files.to_list():
-        if f == pai.file:
-            packaged_file_found = True
-        if f.basename == pai_name:
-            packaged_file = f
+        if f == out_file:
+            out_file_found = True
+        if f == rpm_file:
+            rpm_file_found = True
+        if f == changes_file:
+            changes_file_found = True
         elif f.basename == ctx.attr.expected_default_name and not default_name_found:
             default_name_found = True
 
     asserts.true(
         env,
-        packaged_file != None,
-        "File name mentioned in PackageArtifactInfo '{}' is not in DefaultInfo".format(pai_name),
+        out_file_found,
+        "out component of OutputGroupInfo '{}' is not in DefaultInfo".format(out_file),
     )
-
     asserts.true(
         env,
-        packaged_file_found,
-        "File object mentioned in PackageArtifactInfo '{}' missing from DefaultInfo".format(pai_name),
+        rpm_file_found,
+        "rpm component of OutputGroupInfo '{}' is not in DefaultInfo".format(rpm_file),
+    )
+    asserts.true(
+        env,
+        changes_file_found,
+        "changes component of OutputGroupInfo '{}' is not in DefaultInfo".format(changes_file),
     )
-
     asserts.true(
         env,
         default_name_found,
@@ -239,7 +245,7 @@ dummy_pkg_variables = rule(
 
 def _test_naming(name):
     # Test whether name templating via PackageVariablesInfo functions as expected, and ensure that
-    # outputs are passed through to PackageArtifactsInfo.
+    # outputs are passed through to OutputGroupInfo.
     pkg_files(
         name = "{}_file_base".format(name),
         srcs = ["foo"],