Fix RPM package release and version files expansion (#816)

3 files changed, 72 insertions, 22 deletions

diff --git a/pkg/rpm_pfg.bzl b/pkg/rpm_pfg.bzl
index 0f2dd75..0ccdbaf 100644
--- a/pkg/rpm_pfg.bzl
+++ b/pkg/rpm_pfg.bzl
@@ -274,7 +274,7 @@ def _pkg_rpm_impl(ctx):
         if ctx.attr.version:
             fail("Both version and version_file attributes were specified")
 
-        preamble_pieces.append("Version: ${VERSION_FROM_FILE}")
+        preamble_pieces.append("Version: ${{VERSION_FROM_FILE}}")
         args.append("--version=@" + ctx.file.version_file.path)
         files.append(ctx.file.version_file)
     elif ctx.attr.version:
@@ -287,7 +287,7 @@ def _pkg_rpm_impl(ctx):
         if ctx.attr.release:
             fail("Both release and release_file attributes were specified")
 
-        preamble_pieces.append("Release: ${RELEASE_FROM_FILE}")
+        preamble_pieces.append("Release: ${{RELEASE_FROM_FILE}}")
         args.append("--release=@" + ctx.file.release_file.path)
         files.append(ctx.file.release_file)
     elif ctx.attr.release:
@@ -684,8 +684,8 @@ def _pkg_rpm_impl(ctx):
     )
 
     changes = []
-    if ctx.attr.changelog:
-        changes = [ctx.attr.changelog]
+    if ctx.file.changelog:
+        changes = [ctx.file.changelog]
 
     output_groups = {
         "out": [default_file],
diff --git a/tests/rpm/BUILD b/tests/rpm/BUILD
index efae3ff..f51d75b 100644
--- a/tests/rpm/BUILD
+++ b/tests/rpm/BUILD
@@ -149,6 +149,25 @@ _POSTTRANS_SCRIPTLET = "echo posttrans"
 ]
 
 ############################################################################
+# versionfile for testing
+############################################################################
+
+_VERSION = "1.1.1"
+_RELEASE = "2222"
+
+genrule(
+    name = "version_file",
+    outs = ["version"],
+    cmd = "echo '{}' > $@".format(_VERSION),
+)
+
+genrule(
+    name = "release_file",
+    outs = ["release"],
+    cmd = "echo '{}' > $@".format(_RELEASE),
+)
+
+############################################################################
 # Test RPMs
 ############################################################################
 
@@ -167,12 +186,12 @@ pkg_rpm(
     preun_scriptlet = _PREUN_SCRIPTLET,
     posttrans_scriptlet = _POSTTRANS_SCRIPTLET,
     provides = ["test"],
-    release = "2222",
+    release = _RELEASE,
     requires = ["test-lib > 1.0"],
     requires_contextual = {"preun": ["bash"]},
     spec_template = "template-test.spec.tpl",
     summary = "pkg_rpm test rpm summary",
-    version = "1.1.1",
+    version = _VERSION,
 )
 
 # Just like the above one, except the compression is changed.
@@ -192,12 +211,12 @@ pkg_rpm(
     preun_scriptlet = _PREUN_SCRIPTLET,
     posttrans_scriptlet = _POSTTRANS_SCRIPTLET,
     provides = ["test"],
-    release = "2222",
+    release = _RELEASE,
     requires = ["test-lib > 1.0"],
     requires_contextual = {"preun": ["bash"]},
     spec_template = "template-test.spec.tpl",
     summary = "pkg_rpm test rpm summary",
-    version = "1.1.1",
+    version = _VERSION,
 )
 
 # Like the first one, except `srcs` is now passed in without using a
@@ -220,12 +239,12 @@ pkg_rpm(
     preun_scriptlet = _PREUN_SCRIPTLET,
     posttrans_scriptlet = _POSTTRANS_SCRIPTLET,
     provides = ["test"],
-    release = "2222",
+    release = _RELEASE,
     requires = ["test-lib > 1.0"],
     requires_contextual = {"preun": ["bash"]},
     spec_template = "template-test.spec.tpl",
     summary = "pkg_rpm test rpm summary",
-    version = "1.1.1",
+    version = _VERSION,
 )
 
 # Like the first one, except we use files for scriptlets
@@ -249,7 +268,31 @@ pkg_rpm(
     requires_contextual = {"preun": ["bash"]},
     spec_template = "template-test.spec.tpl",
     summary = "pkg_rpm test rpm summary",
-    version = "1.1.1",
+    version = _VERSION,
+)
+
+# Like the first one, except we use release and version files
+pkg_rpm(
+    name = "test_rpm_release_version_files",
+    srcs = [
+        ":test_pfg",
+    ],
+    architecture = "noarch",
+    conflicts = ["not-a-test"],
+    description = """pkg_rpm test rpm description""",
+    license = "Apache 2.0",
+    post_scriptlet_file = ":post",
+    postun_scriptlet_file = ":postun",
+    pre_scriptlet_file = ":pre",
+    preun_scriptlet_file = ":preun",
+    posttrans_scriptlet_file = ":posttrans",
+    provides = ["test"],
+    release_file = ":release_file",
+    requires = ["test-lib > 1.0"],
+    requires_contextual = {"preun": ["bash"]},
+    spec_template = "template-test.spec.tpl",
+    summary = "pkg_rpm test rpm summary",
+    version_file = ":version_file",
 )
 
 ############################################################################
@@ -363,6 +406,7 @@ sh_library(
         ":test_rpm_manifest",
         ":test_rpm_metadata",
         ":test_rpm_scriptlets_files",
+        ":test_rpm_release_version_files",
     ],
 )
 
diff --git a/tests/rpm/pkg_rpm_basic_test.py b/tests/rpm/pkg_rpm_basic_test.py
index 83bb84b..8b29934 100644
--- a/tests/rpm/pkg_rpm_basic_test.py
+++ b/tests/rpm/pkg_rpm_basic_test.py
@@ -50,6 +50,8 @@ class PkgRpmBasicTest(unittest.TestCase):
             "rules_pkg/tests/rpm/test_rpm_bzip2-1.1.1-2222.noarch.rpm")
         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")
         self.maxDiff = None
 
     def test_scriptlet_content(self):
@@ -71,23 +73,27 @@ echo posttrans
             self.assertEqual(output, expected)
 
     def test_basic_headers(self):
-        fields = {
-            "NAME": b"test_rpm",
+        common_fields = {
             "VERSION": b"1.1.1",
             "RELEASE": b"2222",
             "ARCH": b"noarch",
             "GROUP": b"Unspecified",
             "SUMMARY": b"pkg_rpm test rpm summary",
         }
-        for fieldname, expected in fields.items():
-            output = subprocess.check_output([
-                "rpm", "-qp", "--queryformat", "%{" + fieldname + "}",
-                self.test_rpm_path
-            ])
-
-            self.assertEqual(
-                output, expected,
-                "RPM Tag {} does not match expected value".format(fieldname))
+        for rpm, fields in [
+            (self.test_rpm_path, {"NAME": b"test_rpm"}),
+            (self.test_rpm_release_version_files, {"NAME": b"test_rpm_release_version_files"}),
+        ]:
+            fields.update(common_fields)
+            for fieldname, expected in fields.items():
+                output = subprocess.check_output([
+                    "rpm", "-qp", "--queryformat", "%{" + fieldname + "}",
+                    rpm,
+                ])
+
+                self.assertEqual(
+                    output, expected,
+                    "RPM Tag {} does not match expected value".format(fieldname))
 
     def test_contents(self):
         manifest_file = self.runfiles.Rlocation(