Merge branch 'benbrown/rpm-improvements' into 'master'

Extract manifests into the metadir before packaging and separate cache-key logic for rpms

See merge request baserock/ybd!404

3 files changed, 43 insertions, 24 deletions

diff --git a/ybd/cache.py b/ybd/cache.py
index b4f5e27..2109f2b 100644
--- a/ybd/cache.py
+++ b/ybd/cache.py
@@ -30,18 +30,22 @@ import yaml
 import re
 
 
-def cache_key(dn):
+def cache_key(dn, mode='normal'):
     if dn is None:
         app.log(dn, 'No definition found for', dn, exit=True)
 
     if type(dn) is not dict:
         dn = app.defs.get(dn)
 
-    if dn.get('cache') == 'calculating':
+    cache_name = 'cache'
+    if mode != 'normal':
+        cache_name = '{}-{}'.format(mode, cache_name)
+
+    if dn.get(cache_name) == 'calculating':
         app.log(dn, 'Recursion loop for', dn, exit=True)
 
-    if dn.get('cache'):
-        return dn['cache']
+    if dn.get(cache_name):
+        return dn[cache_name]
 
     if dn.get('arch', app.config['arch']) != app.config['arch']:
         if 'tried' not in dn:
@@ -50,7 +54,7 @@ def cache_key(dn):
                     app.config['arch'])
         return False
 
-    dn['cache'] = 'calculating'
+    dn[cache_name] = 'calculating'
 
     key = 'no-build'
     if app.config.get('mode', 'normal') in ['keys-only', 'normal']:
@@ -59,11 +63,11 @@ def cache_key(dn):
                 dn['tree'], dn['sha'] = get_tree(dn)
             if not dn.get('repourl'):
                 dn['repourl'] = get_repo_url(dn.get('repo'))
-        factors = hash_factors(dn)
+        factors = hash_factors(dn, mode)
         factors = json.dumps(factors, sort_keys=True).encode('utf-8')
         key = hashlib.sha256(factors).hexdigest()
 
-    dn['cache'] = dn['name'] + "." + key
+    dn[cache_name] = dn['name'] + "." + key
 
     app.config['total'] += 1
     x = 'x'
@@ -78,16 +82,16 @@ def cache_key(dn):
     if dn.get('kind', 'chunk') == 'system':
         app.config['systems'] += 1
 
-    app.log('CACHE-KEYS', '[%s]' % x, dn['cache'])
+    app.log('CACHE-KEYS', '[%s]' % x, dn[cache_name])
     if app.config.get('manifest', False):
         update_manifest(dn, app.config['manifest'])
 
     if 'keys' in app.config:
-        app.config['keys'] += [dn['cache']]
-    return dn['cache']
+        app.config['keys'] += [dn[cache_name]]
+    return dn[cache_name]
 
 
-def hash_factors(dn):
+def hash_factors(dn, mode='normal'):
     hash_factors = {'arch': app.config['arch']}
 
     for factor in dn.get('contents', []):
@@ -139,10 +143,6 @@ def hash_factors(dn):
                 else:
                     hash_factors['max-jobs'] = 'parallel'
 
-        if artifact_version not in range(0, 10):
-            if dn.get('rpm-metadata'):
-                hash_factors['rpm-metadata'] = dn['rpm-metadata']
-
         if artifact_version in range(0, 12):
             for factor in dn.get('build-depends', []):
                 hash_factors[factor] = cache_key(factor)
@@ -155,6 +155,11 @@ def hash_factors(dn):
             if dn.get('prefix'):
                 hash_factors['prefix'] = dn['prefix']
 
+        if artifact_version not in range(0, 10):
+            if artifact_version < 13 or mode == 'rpm':
+                if dn.get('rpm-metadata'):
+                    hash_factors['rpm-metadata'] = dn['rpm-metadata']
+
     return hash_factors
 
 
diff --git a/ybd/config/ybd.conf b/ybd/config/ybd.conf
index 9b13936..129e1bd 100644
--- a/ybd/config/ybd.conf
+++ b/ybd/config/ybd.conf
@@ -67,7 +67,9 @@ aliases:
 # 11: (after bd721e8) Fix potentially incorrect checkout of ref instead of sha
 # 12: (after b1eefba and 33a3553) build-depends file renames don't cause
 #     cache-key changes and cache_key now includes prefix
-artifact-version: 12
+# 13: (after 0c26334 and 30709ae) rpm: place manifests into metadir before
+#     packaging and separate rpm-metadata from normal cache-key calculation
+artifact-version: 13
 
 # path to be used in default chroots for builds
 base-path: ['/usr/bin', '/bin', '/usr/sbin', '/sbin']
diff --git a/ybd/rpm.py b/ybd/rpm.py
index f7e29b6..00c628f 100644
--- a/ybd/rpm.py
+++ b/ybd/rpm.py
@@ -18,17 +18,16 @@ import utils
 import subprocess
 
 
-# Because rpm is otherwise totally broken
+# Common parameters required for rpm.
 # NOTE: _build_name_fmt would ordinary be
-# %%{NAME}-%%{VERSION}-%%{RELEASE}.%%{ARCH}.rpm
+# %{NAME}-%{VERSION}-%{RELEASE}.%{ARCH}.rpm
 # but we are pulling them out of a cache with a different naming scheme now.
-#
 common_rpm_args = (
   '--dbpath=/var/lib/rpm '
   '--define "_rpmconfigdir /usr/lib/rpm" '
   '--define "_rpmlock_path /var/lib/rpm/.rpm.lock" '
   '--define "_fileattrsdir /usr/lib/rpm/fileattrs" '
-  '--define "_build_name_fmt %%{NAME}.rpm" '
+  '--define "_build_name_fmt %{NAME}.rpm" '
   '--define "_rpmfilename %{_build_name_fmt}" '
   '--define "_tmppath /tmp" '
   '--define "_unpackaged_files_terminate_build 0" '
@@ -52,7 +51,7 @@ def foreach_def(dn, callback, user_data, traversed=None, whitelist=None):
         dn = app.defs.get(dn)
 
     # if we can't calculate cache key, we can't create this component
-    if cache_key(dn) is False:
+    if cache_key(dn, mode='rpm') is False:
         if 'tried' not in dn:
             log(dn, 'No cache_key, so skipping compose')
             dn['tried'] = True
@@ -171,7 +170,7 @@ def generate_spec(dn, stage_dir, metafile, output, name, system):
                             output_f.write('%s: %s\n' % (field, item))
         output_f.write('\n')
 
-        output_f.write('%%description\n')
+        output_f.write('%description\n')
         output_f.write('%s\n' % description)
         output_f.write('\n')
 
@@ -230,7 +229,8 @@ def generate_spec(dn, stage_dir, metafile, output, name, system):
                 output_f.write('%%files -n %s%s\n' %
                                (package['name'], metafile))
             for filename in package['files']:
-                output_f.write(filename + "\n")
+                output_f.write(
+                    filename.replace('%manifest /', '%manifest ') + "\n")
             output_f.write('\n')
 
     return True
@@ -264,6 +264,17 @@ def extract_metafiles(system, dn, instdir, metadir):
             shutil.move(os.path.join(instdir, metafile), metadir)
 
 
+def extract_manifests(system, dn, instdir, metadir):
+    packages = dn['rpm-metadata'].get('packages', [])
+    manifests = {line.split()[1].lstrip(os.sep)
+                 for package in packages
+                 for line in package.get('files', [])
+                 if line.startswith('%manifest')}
+    for manifest in manifests:
+        manifest = expand_macro(system, dn, manifest)
+        shutil.move(os.path.join(instdir, manifest), metadir)
+
+
 def get_package_names_with_fields(system, dn, fields):
     # Ignores subpackages without files sections as those aren't generated.
     if 'rpm-metadata' not in dn:
@@ -324,7 +335,7 @@ def get_remote_rpm(dn, pkgfilename):
 
 
 def get_cache_pkgfilename(pkgname, dn):
-    return "{}.{}.rpm".format(pkgname, cache_key(dn))
+    return "{}.{}.rpm".format(pkgname, cache_key(dn, mode='rpm'))
 
 
 def all_rpms_cached(system, dn):
@@ -463,6 +474,7 @@ def package_one_rpm(dn, userdata):
                              name, system):
                 defines = extract_defines(dn)
                 extract_metafiles(system, dn, fulldir, metadir)
+                extract_manifests(system, dn, fulldir, metadir)
 
                 # XXX Now we gonna run rpmbuild in the sandbox !!!
                 command = ('rpmbuild ' + common_rpm_args +