Merge lp:~cjwatson/bzr/fix-keep-dirty into lp:bzr

[Colin Watson]

Avoid associating dirty patch headers with the previous file in the patch.

The keep_dirty feature added in https://code.launchpad.net/~blr/bzr/bug-1400567/+merge/244531 turns out to be slightly buggy. You can see the effect by inspecting the quoted "===" lines in http://paste.ubuntu.com/11809068/ - all dirty headers except the first are wrongly associated with the previous file in the patch.

To fix this, when we encounter a "===" line, check whether there are any saved lines and if so emit them at that point before continuing.

I extended the existing tests slightly to cover this (./bzr selftest -v -s bt.test_patches), and haven't bothered adding a release note since this is a bug-fix to a change that as yet hasn't been part of a bzr release.

[William Grant (wgrant)]

review: Approve

[Vincent Ladeuil (vila)]

Thanks for fixing that.

Just a minor nit if you don't mind ;) see inline comments.

Good to land otherwise.

review: Needs Fixing

[lp:~cjwatson/bzr/fix-keep-dirty]

6605. By Colin Watson on 2015-07-02

Use assertLength.

[Vincent Ladeuil (vila)]

review: Approve

[Vincent Ladeuil (vila)]

sent to pqm by email

[Vincent Ladeuil (vila)]

sent to pqm by email

[Vincent Ladeuil (vila)]

sent to pqm by email

[Richard Wilbur (richard-wilbur)]

I have an open rt ticket with ubuntu.com regarding pqm. I am bugging them again about it because they said they found an issue 10 days ago and would get back to me when they had it resolved.

(By the way, thanks Colin for the fix. And thanks Vincent and William for participating in the review process.)

[Richard Wilbur (richard-wilbur)]

The open rt ticket regarding pqm is #26672 at ubuntu.com.

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/patches.py'
--- bzrlib/patches.py	2014-12-12 03:59:25 +0000
+++ bzrlib/patches.py	2015-07-02 11:31:08 +0000
@@ -358,6 +358,14 @@
 
     for line in iter_lines:
         if line.startswith('=== '):
+            if len(saved_lines) > 0:
+                if keep_dirty and len(dirty_head) > 0:
+                    yield {'saved_lines': saved_lines,
+                           'dirty_head': dirty_head}
+                    dirty_head = []
+                else:
+                    yield saved_lines
+                saved_lines = []
             dirty_head.append(line)
             continue
         if line.startswith('*** '):

=== modified file 'bzrlib/tests/test_patches.py'
--- bzrlib/tests/test_patches.py	2014-12-12 03:59:25 +0000
+++ bzrlib/tests/test_patches.py	2015-07-02 11:31:08 +0000
@@ -67,14 +67,22 @@
         lines = ["=== added directory 'foo/bar'\n",
                  "=== modified file 'orig/commands.py'\n",
                  "--- orig/commands.py\n",
-                 "+++ mod/dommands.py\n"]
+                 "+++ mod/dommands.py\n",
+                 "=== modified file 'orig/another.py'\n",
+                 "--- orig/another.py\n",
+                 "+++ mod/another.py\n"]
         patches = parse_patches(
             lines.__iter__(), allow_dirty=True, keep_dirty=True)
+        self.assertLength(2, patches)
         self.assertEqual(patches[0]['dirty_head'],
                          ["=== added directory 'foo/bar'\n",
                           "=== modified file 'orig/commands.py'\n"])
         self.assertEqual(patches[0]['patch'].get_header().splitlines(True),
                          ["--- orig/commands.py\n", "+++ mod/dommands.py\n"])
+        self.assertEqual(patches[1]['dirty_head'],
+                         ["=== modified file 'orig/another.py'\n"])
+        self.assertEqual(patches[1]['patch'].get_header().splitlines(True),
+                         ["--- orig/another.py\n", "+++ mod/another.py\n"])
 
     def testValidPatchHeader(self):
         """Parse a valid patch header"""