rpm.SpecFile: drop the internal 'patches' structure

To get rid of duplicate data tracking. Also, add test for testing the
macro expansion of patch and source names.

Also add tests for SpecFile.patchseries.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>

5 files changed, 74 insertions, 54 deletions

diff --git a/gbp/rpm/__init__.py b/gbp/rpm/__init__.py
index d5d306fa..0aac3c53 100644
--- a/gbp/rpm/__init__.py
+++ b/gbp/rpm/__init__.py
@@ -149,7 +149,6 @@ class SpecFile(object):
         self.packager = source_header[rpm.RPMTAG_PACKAGER]
         self.specfile = os.path.basename(specfile)
         self.specdir = os.path.dirname(os.path.abspath(specfile))
-        self.patches = {}
         self.sources = {}
         self._tags = {}
         self._special_directives = defaultdict(list)
@@ -211,6 +210,12 @@ class SpecFile(object):
             return sorted([int(num) for num in data])
         return []
 
+    def _patches(self):
+        """Get all patch tags as a dict"""
+        if 'patch' not in self._tags:
+            return {}
+        return {patch['num']: patch for patch in self._tags['patch']['lines']}
+
     def _macro_replace(self, matchobj):
         macro_dict = {'name': self.name,
                       'version': self.upstreamversion,
@@ -268,14 +273,6 @@ class SpecFile(object):
         # 'Patch:' tags
         elif tagname == 'patch':
             tagnum = -1 if tagnum is None else tagnum
-            new_patch = {'name': matchobj.group('name').strip(),
-                         'filename': matchobj.group('name'),
-                         'apply': False,
-                         'strip': '0',
-                         'macro_line': None,
-                         'autoupdate': True,
-                         'tag_line': lineobj}
-            self.patches[tagnum] = new_patch
 
         # Record all tag locations
         try:
@@ -306,16 +303,21 @@ class SpecFile(object):
 
         return tagname
 
-    def _parse_directive(self, lineobj):
-        """Parse special directive/scriptlet/macro lines"""
+    @staticmethod
+    def _patch_macro_opts(args):
+        """Parse arguments of the '%patch' macro"""
 
-        # Parser for '%patch' macros
         patchparser = OptionParser()
         patchparser.add_option("-p", dest="strip")
         patchparser.add_option("-s", dest="silence")
         patchparser.add_option("-P", dest="patchnum")
         patchparser.add_option("-b", dest="backup")
         patchparser.add_option("-E", dest="removeempty")
+        arglist = args.split()
+        return patchparser.parse_args(arglist)[0]
+
+    def _parse_directive(self, lineobj):
+        """Parse special directive/scriptlet/macro lines"""
 
         # Parser for '%setup' macros
         setupparser = OptionParser()
@@ -337,19 +339,13 @@ class SpecFile(object):
         # '%patch' macros
         directiveid = None
         if directivename == 'patch':
-            arglist = matchobj.group('args').split()
-            (opts, args) = patchparser.parse_args(arglist)
+            opts = self._patch_macro_opts(matchobj.group('args'))
             if matchobj.group('num'):
                 directiveid = int(matchobj.group('num'))
             elif opts.patchnum:
                 directiveid = int(opts.patchnum)
             else:
                 directiveid = -1
-
-            if opts.strip:
-                self.patches[directiveid]['strip'] = opts.strip
-            self.patches[directiveid]['macro_line'] = lineobj
-            self.patches[directiveid]['apply'] = True
         # '%setup' macros
         elif directivename == 'setup':
             arglist = matchobj.group('args').split()
@@ -416,6 +412,7 @@ class SpecFile(object):
 
         # Update sources info (basically possible macros expanded by rpm)
         # And, double-check that we parsed spec content correctly
+        patches = self._patches()
         for (name, num, typ) in self._specinfo.sources:
             # workaround rpm parsing bug
             if typ == 1 or typ == 9:
@@ -439,16 +436,11 @@ class SpecFile(object):
                 # having number (2^31-1), we use number -1
                 if num >= pow(2,30):
                     num = -1
-                if num in self.patches:
-                    self.patches[num]['filename'] = name
+                if num in patches:
+                    patches[num]['linevalue'] = name
                 else:
                     gbp.log.err("BUG: we didn't correctly parse all 'Patch' tags!")
 
-        # Mark ignored patches
-        for patchnum in self.patches:
-            if patchnum in self.ignorepatches:
-                self.patches[patchnum]['autoupdate'] = False
-
     def _delete_tag(self, tag, num):
         """Delete a tag"""
         key = tag.lower()
@@ -577,7 +569,7 @@ class SpecFile(object):
         macro_prev = None
         ignored = self.ignorepatches
         # Remove 'Patch:̈́' tags
-        for tag in self._tags['patch']['lines']:
+        for tag in self._patches().values():
             if not tag['num'] in ignored:
                 tag_prev = self._delete_tag('patch', tag['num'])
                 # Remove a preceding comment if it seems to originate from GBP
@@ -651,20 +643,26 @@ class SpecFile(object):
             macro_line = self._set_special_macro('patch', patchnum, '-p1',
                                                  macro_line)
 
-    def patchseries(self):
-        """
-        Return patches of the RPM as a gbp patchseries
-        """
+    def patchseries(self, unapplied=False, ignored=False):
+        """Return non-ignored patches of the RPM as a gbp patchseries"""
         series = PatchSeries()
-        patchdir = self.specdir
-        for n, p in sorted(self.patches.iteritems()):
-            if p['autoupdate'] and p['apply']:
-                fname = os.path.basename(p['filename'])
-                series.append(Patch(os.path.join(patchdir, fname),
-                                    strip = int(p['strip'])))
+        if 'patch' in self._tags:
+            tags = self._patches()
+            macros = {macro['id']: macro['args']
+                      for macro in self._special_directives['patch']}
+            ignored = [] if ignored else self.ignorepatches
+
+            for num, tag in sorted(tags.iteritems()):
+                strip = 0
+                if num in macros:
+                    opts = self._patch_macro_opts(macros[num])
+                    strip = int(opts.strip) if opts.strip else 0
+                if (unapplied or (num in macros)) and num not in ignored:
+                    filename = os.path.basename(tag['linevalue'])
+                    series.append(Patch(os.path.join(self.specdir, filename),
+                                        strip=strip))
         return series
 
-
     def guess_orig_file(self):
         """
         Try to guess the name of the primary upstream/source archive
diff --git a/gbp/scripts/import_srpm.py b/gbp/scripts/import_srpm.py
index 0db870e0..28802eb5 100755
--- a/gbp/scripts/import_srpm.py
+++ b/gbp/scripts/import_srpm.py
@@ -318,8 +318,8 @@ def main(argv):
                 raise
 
         # Need to copy files to the packaging directory given by caller
-        files = [os.path.basename(patch['filename']) \
-                for patch in spec.patches.itervalues()]
+        files = [os.path.basename(patch.path) \
+                for patch in spec.patchseries(unapplied=True, ignored=True)]
         for num, src in spec.sources.iteritems():
             if num != spec.orig_src_num:
                 files.append(src['filename'])
diff --git a/gbp/scripts/pq_rpm.py b/gbp/scripts/pq_rpm.py
index 64ba4236..9b1f1662 100755
--- a/gbp/scripts/pq_rpm.py
+++ b/gbp/scripts/pq_rpm.py
@@ -230,17 +230,15 @@ def rm_patch_files(spec):
     marked as not maintained by gbp.
     """
     # Remove all old patches from the spec dir
-    for n, p in spec.patches.iteritems():
-        if p['autoupdate']:
-            f = os.path.join(spec.specdir, p['filename'])
-            gbp.log.debug("Removing '%s'" % f)
-            try:
-                os.unlink(f)
-            except OSError, (e, msg):
-                if e != errno.ENOENT:
-                    raise GbpError, "Failed to remove patch: %s" % msg
-                else:
-                    gbp.log.debug("%s does not exist." % f)
+    for patch in spec.patchseries(unapplied=True):
+        gbp.log.debug("Removing '%s'" % patch.path)
+        try:
+            os.unlink(patch.path)
+        except OSError as err:
+            if err.errno != errno.ENOENT:
+                raise GbpError("Failed to remove patch: %s" % err)
+            else:
+                gbp.log.debug("Patch %s does not exist." % patch.path)
 
 
 def update_patch_series(repo, spec, start, end, options):
diff --git a/tests/test_rpm.py b/tests/test_rpm.py
index 01d8b277..830e4506 100644
--- a/tests/test_rpm.py
+++ b/tests/test_rpm.py
@@ -199,6 +199,7 @@ class TestSpecFile(object):
         spec.protected('_delete_tag')('source', 0)
         spec.protected('_delete_tag')('patch', 0)
         spec.protected('_delete_tag')('patch', -1)
+        assert spec.protected('_patches')() == {}
         prev = spec.protected('_delete_tag')('invalidtag', None)
 
         with assert_raises(GbpError):
@@ -248,8 +249,28 @@ class TestSpecFile(object):
                 assert val['value'] == rval, ("'%s:' is '%s', expecting '%s'" %
                                               (name, val['value'], rval))
             assert spec.ignorepatches == []
+            # Check patch numbers and patch filenames
+            patches = {patch['num']: patch['linevalue'] for patch in
+                       spec.protected('_tags')['patch']['lines']}
+            assert patches == {0: 'my_patch0', -1: 'my_patch'}
 
-            assert spec.patches.keys() == [0, -1]
+    def test_patch_series(self):
+        """Test the getting the patches as a patchseries"""
+        spec_filepath = os.path.join(SPEC_DIR, 'gbp-test-native.spec')
+        spec = SpecFileTester(spec_filepath)
+
+        assert len(spec.patchseries()) == 0
+        spec.update_patches(['1.patch', '2.patch', '3.patch'])
+        assert len(spec.patchseries()) == 3
+        spec.protected('_gbp_tags')['ignore-patches'].append({'args': "0"})
+        spec.update_patches(['4.patch'])
+        assert len(spec.patchseries()) == 1
+        assert len(spec.patchseries(ignored=True)) == 2
+        spec.protected('_delete_special_macro')('patch', 0)
+        assert len(spec.patchseries(ignored=True)) == 1
+        series = spec.patchseries(unapplied=True, ignored=True)
+        assert len(series) == 2
+        assert os.path.basename(series[-1].path) == '4.patch'
 
 
 class TestUtilityFunctions(object):
diff --git a/tests/test_rpm_data/specs/gbp-test-tags.spec b/tests/test_rpm_data/specs/gbp-test-tags.spec
index acbb9213..c96f035c 100644
--- a/tests/test_rpm_data/specs/gbp-test-tags.spec
+++ b/tests/test_rpm_data/specs/gbp-test-tags.spec
@@ -9,6 +9,9 @@
 
 %define test_arch_os_tags %(test -n "$GBP_SKIP_ARCH_OS_TAGS" && echo 0 || echo 1)
 
+%define source_fn_base source
+%define patch_fn_base patch
+
 # Gbp-Undefined-Tag: foobar
 
 # Test that we accept different cases
@@ -27,8 +30,8 @@ Packager:       my_packager
 Url:            my_url
 Vcs:            my_vcs
 Source:         my_source
-Patch:          my_patch
-Patch0:         my_patch0
+Patch:          my_%patch_fn_base
+Patch0:         my_%{patch_fn_base}0
 Nosource:       0
 Nopatch:        0
 #Icon:           my_icon