Merge lp:~abentley/launchpad/ignore-md-diff into lp:launchpad

[Aaron Bentley]

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

= Summary =
This patch stops launchpad from storing the diffs in merge directives,
fixing bug #453010.

== Proposed fix ==
bzr has a bug in its patch parsing, but we are only exposed to it when
we process diffs that we don't get directly from bzrlib. When we
receive diffs via email, there's a chance that the mail client will
munge the newlines, which exposes us to this bug.

We are no longer using this type of diff at all, so there's no need to
be retrieving it from the merge directive.

So this patch simply stops attempting to store the diff from the merge
directive. For the time being, this means that it will be generated by
the MergeProposalCreationJob, but eventually, we can shut that off too.

== Pre-implementation notes ==
This patch has no pre-implementation call.

== Implementation details ==
Nothing to add

== Tests ==
bin/test -v test_codehandler

== Demo and Q/A ==
Generate a merge directive. Convert its line endings to CRLF. Send it
as an attachment to <email address hidden>. Launchpad should not
report an OOPS (via email).

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/mail/codehandler.py
  lib/lp/code/mail/tests/test_codehandler.py
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrYqkkACgkQ0F+nu1YWqI360gCdGSh7GtIj/OjKgm1f76PkOZkk
piYAn1pfPDnOPUgeA7HJ4izjFyD+9pJ/
=xtLB
-----END PGP SIGNATURE-----

[Abel Deuring (adeuring)]

review: Approve (code)

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/lp/code/mail/codehandler.py'
--- lib/lp/code/mail/codehandler.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/mail/codehandler.py	2009-10-16 17:20:28 +0000
@@ -21,7 +21,6 @@
 
 from lp.codehosting.bzrutils import is_branch_stackable
 from lp.codehosting.vfs import get_lp_server
-from lp.code.interfaces.diff import IStaticDiffSource
 from canonical.launchpad.interfaces.mail import (
     IMailHandler, EmailProcessingError)
 from canonical.launchpad.interfaces.message import IMessageSet
@@ -582,25 +581,10 @@
                 'Error Creating Merge Proposal', body)
             return
 
-        if md.patch is not None:
-            diff_source = getUtility(IStaticDiffSource)
-            # XXX: Tim Penhey, 2009-02-12, bug 328271
-            # If the branch is private we should probably use the restricted
-            # librarian.
-            # Using the .txt suffix to allow users to view the file in
-            # firefox without firefox trying to get them to download it.
-            filename = '%s.diff.txt' % source.name
-            review_diff = diff_source.acquireFromText(
-                md.base_revision_id, md.revision_id, md.patch,
-                filename=filename)
-            transaction.commit()
-        else:
-            review_diff = None
 
         try:
             bmp = source.addLandingTarget(submitter, target,
-                                          needs_review=True,
-                                          review_diff=review_diff)
+                                          needs_review=True)
 
             context = CodeReviewEmailCommandExecutionContext(
                 bmp, submitter, notify_event_listeners=False)

=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py	2009-09-24 05:41:53 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py	2009-10-16 17:20:28 +0000
@@ -468,7 +468,7 @@
         bmp, comment = code_handler.processMergeProposal(message)
         self.assertEqual(source, bmp.source_branch)
         self.assertEqual(target, bmp.target_branch)
-        self.assertEqual('', bmp.review_diff.diff.text)
+        self.assertIs(None, bmp.review_diff)
         self.assertEqual('Hi!\n', comment.message.text_contents)
         self.assertEqual('My subject', comment.message.subject)
         # No emails are sent.