Package: git-buildpackage
Version: 0.6.31
Severity: wishlist
Tags: patch

Hello,

Now that gbp-pq preserves patch names on import and export (which makes
a whole lot of sense), 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 could remain useful if
support for renumbering were added. It would also be handy to allow the
choice of the format to use for the numeric prefixes.

Patch included.

Carlos

-- System Information:
Debian Release: stretch/sid
  APT prefers unstable
  APT policy: (990, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.0.0-2-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages git-buildpackage depends on:
ii  devscripts            2.15.5
ii  git                   1:2.1.4-2.1
ii  man-db                2.7.0.2-5
ii  python                2.7.9-1
ii  python-dateutil       2.2-2
ii  python-pkg-resources  17.0-1
ii  python-six            1.9.0-3

Versions of packages git-buildpackage recommends:
ii  cowbuilder       0.73
ii  pristine-tar     1.33
ii  python-requests  2.7.0-3

Versions of packages git-buildpackage suggests:
ii  python-notify  0.1.1-4
ii  unzip          6.0-17

-- no debconf information
>From c8168a67ffc74346de4ea17c7cea37a0cba68024 Mon Sep 17 00:00:00 2001
From: Carlos Maddela <madd...@labyrinth.net.au>
Date: Sat, 27 Jun 2015 01:51:19 +1000
Subject: [PATCH] Allow exported patches to be renumbered and the patch number
 prefix format to be specified as an option.

---
 docs/manpages/gbp-pq.sgml | 34 ++++++++++++++++-
 gbp.conf                  |  4 ++
 gbp/config.py             |  9 +++++
 gbp/scripts/common/pq.py  | 42 ++++++++++++++++-----
 gbp/scripts/pq.py         |  6 ++-
 tests/13_test_gbp_pq.py   | 93 ++++++++++++++++++++++++++++++++++++++---------
 6 files changed, 158 insertions(+), 30 deletions(-)

diff --git a/docs/manpages/gbp-pq.sgml b/docs/manpages/gbp-pq.sgml
index ff39fd1..d4d2774 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 b97036d..43fc36d 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 8116ec9..413cb6a 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 29ad693..5580426 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 93f0960..cea5b68 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 fc7394e..e78cdd0 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):
-- 
2.1.4

Reply via email to