Steve Kowalik has proposed merging lp:~stevenk/launchpad/allow-dirty-patches
into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1007131 in Launchpad itself: "MalformedHunkHeader: Malformed hunk
header. Does not match format. '<!--\r"
https://bugs.launchpad.net/launchpad/+bug/1007131
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/allow-dirty-patches/+merge/137117
Allow dirty patches while we generate diffstat output by parsing the patches.
This means that things like MalformedHunkHeader and MalformedLine will no
longer be raised (and cause OOPSes), but the patch will instead not be counted.
I considered this an acceptable tradeoff, and it means that merge proposals
that used to get no diffstat counts do now get counts, which could be off a
little.
Things like MalformedPatchHeader and more serious exceptions are still raised
and will still cause OOPSes, but they could indicate a problem with the code
that generates the diff. There is an existing test that tosses a "not a real
diff" string into the machinery, and I've verified that this change does not
break it.
--
https://code.launchpad.net/~stevenk/launchpad/allow-dirty-patches/+merge/137117
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~stevenk/launchpad/allow-dirty-patches into lp:launchpad.
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2012-09-19 01:19:35 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2012-11-30 05:34:22 +0000
@@ -1,8 +1,6 @@
# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-# pylint: disable-msg=E0611,W0212,F0401
-
"""Database class for branch merge prosals."""
__metaclass__ = type
=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py 2012-09-28 06:25:44 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py 2012-11-30 05:34:22 +0000
@@ -1,17 +1,14 @@
# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-
"""Job classes related to BranchMergeProposals are in here.
This includes both jobs for the proposals themselves, or jobs that are
creating proposals, or diffs relating to the proposals.
"""
-
__metaclass__ = type
-
__all__ = [
'BranchMergeProposalJob',
'BranchMergeProposalJobSource',
@@ -376,8 +373,7 @@
with server(get_ro_server(), no_replace=True):
preview = PreviewDiff.fromBranchMergeProposal(
self.branch_merge_proposal)
- with BranchMergeProposalDelta.monitor(
- self.branch_merge_proposal):
+ with BranchMergeProposalDelta.monitor(self.branch_merge_proposal):
self.branch_merge_proposal.preview_diff = preview
def getOperationDescription(self):
=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py 2012-10-02 01:25:48 +0000
+++ lib/lp/code/model/diff.py 2012-11-30 05:34:22 +0000
@@ -231,11 +231,6 @@
diff_content.seek(0)
diff_content_bytes = diff_content.read(size)
diff_lines_count = len(diff_content_bytes.strip().split('\n'))
- # Generation of diffstat is currently failing in some circumstances.
- # See bug 436325. Since diffstats are incidental to the whole
- # process, we don't want failure here to kill the generation of the
- # diff itself, but we do want to hear about it. So log an error using
- # the error reporting utility.
try:
diffstat = cls.generateDiffstat(diff_content_bytes)
except Exception:
@@ -262,7 +257,9 @@
:return: A map of {filename: (added_line_count, removed_line_count)}
"""
file_stats = {}
- for patch in parse_patches(diff_bytes.splitlines(True)):
+ # Set allow_dirty, so we don't raise exceptions for dirty patches.
+ patches = parse_patches(diff_bytes.splitlines(True), allow_dirty=True)
+ for patch in patches:
if not isinstance(patch, Patch):
continue
path = patch.newname.split('\t')[0]
=== modified file 'lib/lp/code/model/tests/test_diff.py'
--- lib/lp/code/model/tests/test_diff.py 2012-01-01 02:58:52 +0000
+++ lib/lp/code/model/tests/test_diff.py 2012-11-30 05:34:22 +0000
@@ -259,10 +259,18 @@
{'foo': (2, 1), 'bar': (0, 3), 'baz': (2, 0)},
Diff.generateDiffstat(self.diff_bytes))
+ def test_generateDiffstat_with_CR(self):
+ diff_bytes = (
+ "--- foo\t2009-08-26 15:53:23.000000000 -0400\n"
+ "+++ foo\t2009-08-26 15:56:43.000000000 -0400\n"
+ "@@ -1,1 +1,1 @@\n"
+ " a\r-b\r c\r+d\r+e\r+f\r")
+ self.assertEqual({'foo': (0, 0)}, Diff.generateDiffstat(diff_bytes))
+
def test_fromFileSetsDiffstat(self):
diff = Diff.fromFile(StringIO(self.diff_bytes), len(self.diff_bytes))
- self.assertEqual({'bar': (0, 3), 'baz': (2, 0), 'foo': (2, 1)},
- diff.diffstat)
+ self.assertEqual(
+ {'bar': (0, 3), 'baz': (2, 0), 'foo': (2, 1)}, diff.diffstat)
def test_fromFileAcceptsBinary(self):
diff_bytes = "Binary files a\t and b\t differ\n"
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
