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

Proposed by Colin Watson on 2015-07-02
Status: Merged
Approved by: Vincent Ladeuil on 2015-07-02
Approved revision: 6605
Merged at revision: 6605
Proposed branch: lp:~cjwatson/bzr/fix-keep-dirty
Merge into: lp:bzr
Diff against target: 46 lines (+17/-1)
2 files modified
bzrlib/patches.py (+8/-0)
bzrlib/tests/test_patches.py (+9/-1)
To merge this branch: bzr merge lp:~cjwatson/bzr/fix-keep-dirty
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve on 2015-07-02
William Grant 2015-07-02 Approve on 2015-07-02
Review via email: mp+263632@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve
Vincent Ladeuil (vila) wrote :

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 updated on 2015-07-02
6605. By Colin Watson on 2015-07-02

Use assertLength.

Vincent Ladeuil (vila) :
review: Approve
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Vincent Ladeuil (vila) wrote :

sent to pqm by email

Vincent Ladeuil (vila) wrote :

sent to pqm by email

Richard Wilbur (richard-wilbur) wrote :

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) wrote :

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

Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
358358
359 for line in iter_lines:359 for line in iter_lines:
360 if line.startswith('=== '):360 if line.startswith('=== '):
361 if len(saved_lines) > 0:
362 if keep_dirty and len(dirty_head) > 0:
363 yield {'saved_lines': saved_lines,
364 'dirty_head': dirty_head}
365 dirty_head = []
366 else:
367 yield saved_lines
368 saved_lines = []
361 dirty_head.append(line)369 dirty_head.append(line)
362 continue370 continue
363 if line.startswith('*** '):371 if line.startswith('*** '):
364372
=== 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 @@
67 lines = ["=== added directory 'foo/bar'\n",67 lines = ["=== added directory 'foo/bar'\n",
68 "=== modified file 'orig/commands.py'\n",68 "=== modified file 'orig/commands.py'\n",
69 "--- orig/commands.py\n",69 "--- orig/commands.py\n",
70 "+++ mod/dommands.py\n"]70 "+++ mod/dommands.py\n",
71 "=== modified file 'orig/another.py'\n",
72 "--- orig/another.py\n",
73 "+++ mod/another.py\n"]
71 patches = parse_patches(74 patches = parse_patches(
72 lines.__iter__(), allow_dirty=True, keep_dirty=True)75 lines.__iter__(), allow_dirty=True, keep_dirty=True)
76 self.assertLength(2, patches)
73 self.assertEqual(patches[0]['dirty_head'],77 self.assertEqual(patches[0]['dirty_head'],
74 ["=== added directory 'foo/bar'\n",78 ["=== added directory 'foo/bar'\n",
75 "=== modified file 'orig/commands.py'\n"])79 "=== modified file 'orig/commands.py'\n"])
76 self.assertEqual(patches[0]['patch'].get_header().splitlines(True),80 self.assertEqual(patches[0]['patch'].get_header().splitlines(True),
77 ["--- orig/commands.py\n", "+++ mod/dommands.py\n"])81 ["--- orig/commands.py\n", "+++ mod/dommands.py\n"])
82 self.assertEqual(patches[1]['dirty_head'],
83 ["=== modified file 'orig/another.py'\n"])
84 self.assertEqual(patches[1]['patch'].get_header().splitlines(True),
85 ["--- orig/another.py\n", "+++ mod/another.py\n"])
7886
79 def testValidPatchHeader(self):87 def testValidPatchHeader(self):
80 """Parse a valid patch header"""88 """Parse a valid patch header"""