Exclude release from filename when using release_file (#863)

* Exclude release from filename when using release_file

When we're using `release_file` in lieu of `release` we're just
pointing rpmbuild at the file containing the `Release` string and we
don't have it available to inject into the filename resulting in a
strange looking filename of the form `Foo-version-.arch.rpm`.

This change extracts the RPM name generation to a single helper,
`_make_rpm_filename` and tweaks it s.t. if we're missing the value for
`release` we'll just exclude it from the filename format instead.

* Fix test broken test

The test had was using the odd RPM name structure and this change
tweaks it so that the test passes.

2 files changed, 28 insertions, 12 deletions

diff --git a/pkg/rpm_pfg.bzl b/pkg/rpm_pfg.bzl
index 99b84bc..cf6cb6b 100644
--- a/pkg/rpm_pfg.bzl
+++ b/pkg/rpm_pfg.bzl
@@ -175,6 +175,22 @@ def _make_absolute_if_not_already_or_is_macro(path):
     # this can be inlined easily.
     return path if path.startswith(("/", "%")) else "/" + path
 
+def _make_rpm_filename(rpm_name, version, architecture, package_name=None, release=None):
+    prefix = "%s-%s"
+    items = [rpm_name, version]
+
+    if package_name:
+        prefix += "-%s"
+        items = [rpm_name, package_name, version]
+
+    if release:
+        prefix += "-%s"
+        items += [release]
+
+    fmt = prefix + ".%s.rpm"
+
+    return fmt % tuple(items + [architecture])
+
 #### Input processing helper functions.
 
 # TODO(nacl, #459): These are redundant with functions and structures in
@@ -386,12 +402,12 @@ def _process_subrpm(ctx, rpm_name, rpm_info, rpm_ctx, debuginfo_type):
     rpm_ctx.install_script_pieces.extend(sub_rpm_ctx.install_script_pieces)
     rpm_ctx.packaged_directories.extend(sub_rpm_ctx.packaged_directories)
 
-    package_file_name = "%s-%s-%s-%s.%s.rpm" % (
-        rpm_name,
-        rpm_info.package_name,
-        rpm_info.version or ctx.attr.version,
-        ctx.attr.release,
-        rpm_info.architecture or ctx.attr.architecture,
+    package_file_name = _make_rpm_filename(
+        rpm_name = rpm_name,
+        version = rpm_info.version or ctx.attr.version,
+        architecture = rpm_info.architecture or ctx.attr.architecture,
+        package_name = rpm_info.package_name,
+        release = ctx.attr.release,
     )
 
     default_file = ctx.actions.declare_file("{}-{}.rpm".format(rpm_name, rpm_info.package_name))
@@ -482,11 +498,11 @@ def _pkg_rpm_impl(ctx):
 
     package_file_name = ctx.attr.package_file_name
     if not package_file_name:
-        package_file_name = "%s-%s-%s.%s.rpm" % (
+        package_file_name = _make_rpm_filename(
             rpm_name,
             ctx.attr.version,
-            ctx.attr.release,
             ctx.attr.architecture,
+            release = ctx.attr.release,
         )
 
     #### rpm spec "preamble"
@@ -725,12 +741,12 @@ def _pkg_rpm_impl(ctx):
     if debuginfo_type != "none":
         debuginfo_default_file = ctx.actions.declare_file(
             "{}-debuginfo.rpm".format(rpm_name))
-        debuginfo_package_file_name = "%s-%s-%s-%s.%s.rpm" % (
+        debuginfo_package_file_name = _make_rpm_filename(
             rpm_name,
-            "debuginfo",
             ctx.attr.version,
-            ctx.attr.release,
             ctx.attr.architecture,
+            package_name = "debuginfo",
+            release = ctx.attr.release,
         )
 
         _, debuginfo_output_file, _ = setup_output_files(
diff --git a/tests/rpm/pkg_rpm_basic_test.py b/tests/rpm/pkg_rpm_basic_test.py
index ab5c49e..8e4dff2 100644
--- a/tests/rpm/pkg_rpm_basic_test.py
+++ b/tests/rpm/pkg_rpm_basic_test.py
@@ -51,7 +51,7 @@ class PkgRpmBasicTest(unittest.TestCase):
         self.test_rpm_scriptlets_files_path = self.runfiles.Rlocation(
             "rules_pkg/tests/rpm/test_rpm_scriptlets_files-1.1.1-2222.noarch.rpm")
         self.test_rpm_release_version_files = self.runfiles.Rlocation(
-            "rules_pkg/tests/rpm/test_rpm_release_version_files--.noarch.rpm")
+            "rules_pkg/tests/rpm/test_rpm_release_version_files-.noarch.rpm")
         self.test_rpm_epoch = self.runfiles.Rlocation(
             "rules_pkg/tests/rpm/test_rpm_epoch-1.1.1-2222.noarch.rpm")
         self.maxDiff = None