Allow exported patches to be renumbered and the patch number prefix format to be specified as an option

Now that gbp-pq preserves patch names on import and export the
--[no-]patch-numbers options have been rendered largely useless unless
the patch queue commits are amended with the Gbp[-Pq]: Name tags
removed. The options remain useful if support for renumbering is
added. It's also handy to allow the choice of the format to use
for the numeric prefixes.

Closes: #790890
Signed-off-by: Guido Günther <agx@sigxcpu.org>

6 files changed, 158 insertions, 30 deletions

diff --git a/docs/manpages/gbp-pq.sgml b/docs/manpages/gbp-pq.sgml
index ff39fd18..d4d2774b 100644
--- a/docs/manpages/gbp-pq.sgml
+++ b/docs/manpages/gbp-pq.sgml
@@ -21,6 +21,8 @@
       &gbp-pq;
       &man.common.options.synopsis;
       <arg><option>--[no-]patch-numbers</option></arg>
+      <arg><option>--patch-num-format=</option><replaceable>format</replaceable></arg>
+      <arg><option>--[no-]renumber</option></arg>
       <arg><option>--topic=</option><replaceable>topic</replaceable></arg>
       <arg><option>--time-machine=</option><replaceable>num</replaceable></arg>
       <arg><option>--[no-]drop</option></arg>
@@ -131,7 +133,37 @@
         </term>
         <listitem>
           <para>
-          Whether the patch files should start with a number or not.
+          Whether or not the patch files should be prefixed with a number.
+          The default is to export patches with patch numbers. Note,
+          however, that this normally affects patches whose names are
+          automatically generated, and has no effect on exporting patches
+          which have a <option>Gbp[-Pq]: Name</option> tag, since the
+          name specified is preserved unless the <option>--renumber</option>
+          option is used.
+          </para>
+        </listitem>
+      </varlistentry>
+      <varlistentry>
+        <term><option>--patch-num-format=</option><replaceable>format</replaceable>
+        </term>
+        <listitem>
+          <para>
+          The format specifier for patch number prefixes. The default format is '%04d-'.
+          </para>
+        </listitem>
+      </varlistentry>
+      <varlistentry>
+        <term><option>--[no-]renumber</option>
+        </term>
+        <listitem>
+          <para>
+          Whether or not to renumber patches exported from the patch queue,
+          instead of preserving numbers specified in
+          <option>Gbp-Pq: Name</option> tags. The default is not to renumber
+          patches. Useful when patches need to be renamed for the sake of
+          uniformity. For example, using <option>--renumber</option> with
+          <option>--no-patch-num</option> will strip all numeric prefixes from
+          exported patches.
           </para>
         </listitem>
       </varlistentry>
diff --git a/gbp.conf b/gbp.conf
index b97036df..43fc36dd 100644
--- a/gbp.conf
+++ b/gbp.conf
@@ -106,6 +106,10 @@
 # Options only affecting gbp pq
 [pq]
 #patch-numbers = False
+# The format specifier for patch number prefixes
+#patch-num-format = '%04d-'
+# Whether to renumber patches when exporting patch queues
+#renumber = False
 # Whether to drop patch queue after export
 #drop = False
 
diff --git a/gbp/config.py b/gbp/config.py
index 8116ec9a..413cb6a8 100644
--- a/gbp/config.py
+++ b/gbp/config.py
@@ -143,6 +143,8 @@ class GbpOptionParser(OptionParser):
                  'customizations'  : '',
                  'spawn-editor'    : 'release',
                  'patch-numbers'   : 'True',
+                 'patch-num-format': '%04d-',
+                 'renumber'        : 'False',
                  'notify'          : 'auto',
                  'merge'           : 'True',
                  'track'           : 'True',
@@ -251,6 +253,13 @@ class GbpOptionParser(OptionParser):
              'patch-numbers':
                   ("Whether to number patch files, "
                    "default is %(patch-numbers)s"),
+             'patch-num-format':
+                  ("The format specifier for patch number prefixes, "
+                   "default is %(patch-num-format)s"),
+             'renumber':
+                  ("Whether to renumber patches exported from patch queues, "
+                   "instead of preserving the number specified in "
+                   "'Gbp-Pq: Name' tags, default is %(renumber)s"),
              'notify':
                   ("Whether to send a desktop notification after the build, "
                    "default is '%(notify)s'"),
diff --git a/gbp/scripts/common/pq.py b/gbp/scripts/common/pq.py
index 29ad693a..5580426b 100644
--- a/gbp/scripts/common/pq.py
+++ b/gbp/scripts/common/pq.py
@@ -174,28 +174,50 @@ def write_patch_file(filename, commit_info, diff):
     return filename
 
 
+DEFAULT_PATCH_NUM_PREFIX_FORMAT = "%04d-"
+
 def format_patch(outdir, repo, commit_info, series, numbered=True,
-                 path_exclude_regex=None, topic='', name=None):
+                 path_exclude_regex=None, topic='', name=None, renumber=False,
+                 patch_num_prefix_format=DEFAULT_PATCH_NUM_PREFIX_FORMAT):
     """Create patch of a single commit"""
 
     # Determine filename and path
     outdir = os.path.join(outdir, topic)
     if not os.path.exists(outdir):
         os.makedirs(outdir)
-    num_prefix = '%04d-' % (len(series) + 1)
-    suffix = '.patch'
-    base_maxlen = 63 - len(num_prefix) - len(suffix)
-    base = commit_info['patchname'][:base_maxlen]
+
+    try:
+        num_prefix = str(patch_num_prefix_format) % (len(series) + 1) \
+                     if numbered else ''
+    except Exception:
+        gbp.log.warn("Bad format format string '%s', " \
+                     "falling back to default '%s'" % \
+                     (str(patch_num_prefix_format),
+                      DEFAULT_PATCH_NUM_PREFIX_FORMAT))
+        num_prefix = DEFAULT_PATCH_NUM_PREFIX_FORMAT % (len(series) + 1)
+
     if name is not None:
-        filename = name
+        if renumber:
+            # Remove any existing numeric prefix if the patch
+            # should be renumbered
+            name =  re.sub('^\d+[-_]*', '', name)
+        else:
+            # Otherwise, clear proposed prefix
+            num_prefix = ''
+        (base, suffix) = os.path.splitext(name)
     else:
-        filename = (num_prefix if numbered else '') + base + suffix
+        suffix = '.patch'
+        base_maxlen = 63 - len(num_prefix) - len(suffix)
+        base = commit_info['patchname'][:base_maxlen]
+
+    filename = num_prefix + base + suffix
     filepath = os.path.join(outdir, filename)
     # Make sure that we don't overwrite existing patches in the series
     if filepath in series:
-        presuffix = '-%d' % len(series)
-        base = base[:base_maxlen - len(presuffix)] + presuffix
-        filename = (num_prefix if numbered else '') + base + suffix
+        presuffix = '-%d' % \
+                    len([p for p in series \
+                         if p.startswith(os.path.splitext(filepath)[0])])
+        filename = num_prefix + base + presuffix + suffix
         filepath = os.path.join(outdir, filename)
 
     # Determine files to include
diff --git a/gbp/scripts/pq.py b/gbp/scripts/pq.py
index 93f0960f..cea5b68d 100755
--- a/gbp/scripts/pq.py
+++ b/gbp/scripts/pq.py
@@ -97,7 +97,9 @@ def generate_patches(repo, start, end, outdir, options):
                 topic = cmds['topic']
             name = cmds.get('name', None)
             format_patch(outdir, repo, info, patches, options.patch_numbers,
-                         topic=topic, name=name)
+                         topic=topic, name=name,
+                         renumber=options.renumber,
+                         patch_num_prefix_format=options.patch_num_format)
         else:
             gbp.log.info('Ignoring commit %s' % info['id'])
 
@@ -335,6 +337,8 @@ def build_parser(name):
         return None
 
     parser.add_boolean_config_file_option(option_name="patch-numbers", dest="patch_numbers")
+    parser.add_config_file_option(option_name="patch-num-format", dest="patch_num_format")
+    parser.add_boolean_config_file_option(option_name="renumber", dest="renumber")
     parser.add_option("-v", "--verbose", action="store_true", dest="verbose", default=False,
                       help="verbose command execution")
     parser.add_option("--topic", dest="topic", help="in case of 'apply' topic (subdir) to put patch into")
diff --git a/tests/13_test_gbp_pq.py b/tests/13_test_gbp_pq.py
index fc7394e1..e78cdd0b 100644
--- a/tests/13_test_gbp_pq.py
+++ b/tests/13_test_gbp_pq.py
@@ -113,33 +113,25 @@ class TestWritePatch(testutils.DebianGitTestRepo):
     def tearDown(self):
         context.teardown()
 
-    def test_generate_patches(self):
-        """Test generation of patches"""
-
-        class opts:
-            patch_numbers = False
+    def _test_generate_patches(self, changes, expected_patches, opts):
+        self.assertEqual(len(changes), len(expected_patches))
 
         d = context.new_tmpdir(__name__)
-        expected_paths = [os.path.join(str(d), 'gbptest', n) for n in
-                          ['added-foo.patch', 'patchname.diff']]
-        # Add test file with topic:
-        msg = ("added foo\n\n"
-               "Gbp-Pq: Topic gbptest")
-        self.add_file('foo', 'foo', msg)
-        msg = ("added bar\n\n"
-               "Gbp-Pq: Topic gbptest\n"
-               "Gbp-Pq: Name patchname.diff")
-        self.add_file('baz', 'baz', msg)
+        expected_paths = [os.path.join(str(d), n) for n in expected_patches ]
+
+        # Commit changes
+        for c in changes:
+            self.add_file(c[0], c[1], c[2])
 
         # Write it out as patch and check it's existence
-        patchfiles = generate_patches(self.repo, 'HEAD^^', 'HEAD', str(d),
-                                      opts)
+        origin = 'HEAD~%d' % len(changes)
+        patchfiles = generate_patches(self.repo, origin, 'HEAD', str(d), opts)
         for expected in expected_paths:
             self.assertIn(expected, patchfiles)
             self.assertTrue(os.path.exists(expected))
 
         # Reapply the patch to a new branch
-        self.repo.create_branch('testapply', 'HEAD^^')
+        self.repo.create_branch('testapply', origin)
         self.repo.set_branch('testapply')
         for expected in expected_paths:
             self.repo.apply_patch(expected)
@@ -148,6 +140,71 @@ class TestWritePatch(testutils.DebianGitTestRepo):
         # Branches must be identical afterwards
         self.assertEqual('', diff)
 
+    def test_generate_patches(self):
+        """Test generation of patches"""
+
+        class opts:
+            patch_numbers = False
+            renumber = False
+            patch_num_format = '%04d-'
+
+        expected_patches = [ 'gbptest/added-foo.patch',
+                             'gbptest/patchname.diff' ]
+
+        changes = [ ('foo', 'foo', ("added foo\n\n"
+                                    "Gbp-Pq: Topic gbptest")),
+                    ('baz', 'baz', ("added bar\n\n"
+                                    "Gbp-Pq: Topic gbptest\n"
+                                    "Gbp-Pq: Name patchname.diff")) ]
+
+        self._test_generate_patches(changes, expected_patches, opts)
+
+    def test_generate_renumbered_patches(self):
+        """Test generation of renumbered patches"""
+
+        class opts:
+            patch_numbers = True
+            renumber = True
+            patch_num_format = '%02d_'
+
+        expected_patches = [ 'gbptest/01_added-foo.patch',
+                             'gbptest/02_patchname.diff' ]
+
+        changes = [ ('foo', 'foo', ("added foo\n\n"
+                                    "Gbp-Pq: Topic gbptest")),
+                    ('baz', 'baz', ("added bar\n\n"
+                                    "Gbp-Pq: Topic gbptest\n"
+                                    "Gbp-Pq: Name 099-patchname.diff")) ]
+
+        self._test_generate_patches(changes, expected_patches, opts)
+
+    def test_generate_patches_with_name_clashes(self):
+        """Test generation of patches which have name clashes"""
+
+        class opts:
+            patch_numbers = False
+            renumber = True
+            patch_num_format = '%02d_'
+
+        expected_patches = [ 'gbptest/added-foo.patch',
+                             'gbptest/patchname.diff',
+                             'gbptest/patchname-1.diff',
+                             'gbptest/patchname-2.diff' ]
+
+        changes = [ ('foo', 'foo', ("added foo\n\n"
+                                    "Gbp-Pq: Topic gbptest")),
+                    ('baz', 'baz', ("added bar\n\n"
+                                    "Gbp-Pq: Topic gbptest\n"
+                                    "Gbp-Pq: Name 099-patchname.diff")),
+                    ('qux', 'qux', ("added qux\n\n"
+                                    "Gbp-Pq: Topic gbptest\n"
+                                    "Gbp-Pq: Name 100-patchname.diff")),
+                    ('norf', 'norf', ("added norf\n\n"
+                                      "Gbp-Pq: Topic gbptest\n"
+                                      "Gbp-Pq: Name 101-patchname.diff")) ]
+
+        self._test_generate_patches(changes, expected_patches, opts)
+
 
 class TestExport(testutils.DebianGitTestRepo):
     class Options(object):