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

Proposed by Colin Watson
Status: Merged
Approved by: Vincent Ladeuil
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
William Grant Approve
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.
Revision history for this message
William Grant (wgrant) :
review: Approve
Revision history for this message
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
6605. By Colin Watson on 2015-07-02

Use assertLength.

Revision history for this message
Vincent Ladeuil (vila) :
review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Revision history for this message
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.)

Revision history for this message
Richard Wilbur (richard-wilbur) wrote :

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

Revision history for this message
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
1=== modified file 'bzrlib/patches.py'
2--- bzrlib/patches.py 2014-12-12 03:59:25 +0000
3+++ bzrlib/patches.py 2015-07-02 11:31:08 +0000
4@@ -358,6 +358,14 @@
5
6 for line in iter_lines:
7 if line.startswith('=== '):
8+ if len(saved_lines) > 0:
9+ if keep_dirty and len(dirty_head) > 0:
10+ yield {'saved_lines': saved_lines,
11+ 'dirty_head': dirty_head}
12+ dirty_head = []
13+ else:
14+ yield saved_lines
15+ saved_lines = []
16 dirty_head.append(line)
17 continue
18 if line.startswith('*** '):
19
20=== modified file 'bzrlib/tests/test_patches.py'
21--- bzrlib/tests/test_patches.py 2014-12-12 03:59:25 +0000
22+++ bzrlib/tests/test_patches.py 2015-07-02 11:31:08 +0000
23@@ -67,14 +67,22 @@
24 lines = ["=== added directory 'foo/bar'\n",
25 "=== modified file 'orig/commands.py'\n",
26 "--- orig/commands.py\n",
27- "+++ mod/dommands.py\n"]
28+ "+++ mod/dommands.py\n",
29+ "=== modified file 'orig/another.py'\n",
30+ "--- orig/another.py\n",
31+ "+++ mod/another.py\n"]
32 patches = parse_patches(
33 lines.__iter__(), allow_dirty=True, keep_dirty=True)
34+ self.assertLength(2, patches)
35 self.assertEqual(patches[0]['dirty_head'],
36 ["=== added directory 'foo/bar'\n",
37 "=== modified file 'orig/commands.py'\n"])
38 self.assertEqual(patches[0]['patch'].get_header().splitlines(True),
39 ["--- orig/commands.py\n", "+++ mod/dommands.py\n"])
40+ self.assertEqual(patches[1]['dirty_head'],
41+ ["=== modified file 'orig/another.py'\n"])
42+ self.assertEqual(patches[1]['patch'].get_header().splitlines(True),
43+ ["--- orig/another.py\n", "+++ mod/another.py\n"])
44
45 def testValidPatchHeader(self):
46 """Parse a valid patch header"""