Merge lp:~jameinel/bzr-builddeb/changelog-parser into lp:bzr-builddeb

[John A Meinel]

[John A Meinel (jameinel)]

This removes the guts of the Changelog parsing code that I brought in, and replaces it by just importing debian_bundle.changelog and having that do the parsing.

I don't really like depending on cl._blocks, but since import_dsc is doing so, I assumed that was the best we could do.

The tests all still pass, and the parsing is going to be as accurate as python-debian's parsing, which is what import_dsc uses anyway.

[lp:~jameinel/bzr-builddeb/changelog-parser]

402. By John A Meinel on 2010-02-03

Make sure that the blocks are in sorted order before we do anything else.

403. By John A Meinel on 2010-02-03

Fix bug #516060, implement 3-way changelog merge.

This uses a fairly crude 3-way changelog merge algorithm, but doesn't yet
introduce conflicts. That will be next, as it requires an api bump.
If left != right, then we check if the version is in base, and if
one side is identical to base, then we pick the other.
This doesn't:
a) Conflict when left != right != base
b) Allow deleting entries (present in base & this but not other, for instance)
etc.

404. By John A Meinel on 2010-02-03

Conflict when appropriate.

We could try to do a textual merge, but it is just easier to
conflict on the whole region. Mostly because merge3.Merge3 is missing
a decent api for telling whether there was a conflict region :(.

[John A Meinel (jameinel)]

I just updated this branch. I was going to submit a separate proposal, but I did find a bug here, and just fixed it while doing the rest.

Anyway, this now does 3-way merging of the blocks. The basic logic is:

1) Find all blocks in THIS, OTHER and BASE
2) For all blocks in THIS and OTHER, include them in the output in sorted order.
3) If THIS and OTHER both contain a block (matched by Version) with different content, compare against the BASE content
  a) If THIS == BASE, chose OTHER
  b) if OTHER == BASE, chose THIS
  c) If BASE is not present, or BASE is different from both, conflict

What this is missing:

1) If THIS or OTHER deletes a block it will be restored. (I assume that if a version is in THIS or OTHER then you always want it in the output)
2) 3c could use merge3.Merge3() to try to do a textual merge of the conflicted region.

I can open bugs on those two bits if you want.

[lp:~jameinel/bzr-builddeb/changelog-parser]

405. By John A Meinel on 2010-02-03

Simplify the conflict logic slightly.

[Robert Collins (lifeless)]

On Wed, 2010-02-03 at 22:19 +0000, John A Meinel wrote:
>
> What this is missing:
>
> 1) If THIS or OTHER deletes a block it will be restored. (I assume
> that if a version is in THIS or OTHER then you always want it in the
> output)
> 2) 3c could use merge3.Merge3() to try to do a textual merge of the
> conflicted region.
>
> I can open bugs on those two bits if you want.
--

please do

-Rob

[lp:~jameinel/bzr-builddeb/changelog-parser]

406. By John A Meinel on 2010-02-03

Trying to trigger the invalid code, only to find bugs in python-debian :(

[Robert Collins (lifeless)]

 review: approve

On Wed, 2010-02-03 at 20:19 +0000, John A Meinel wrote:
>
> + elif right_block is None:
> + next_block = left_block
> + left_block = step(left_blocks)
> ...

> + elif left_block.version > right_block.version:
> + # left comes first
> + next_block = left_block
> + left_block = step(left_blocks)

The None and > cases are the same - I think it would be shorter to group
them.

elif right_block_is None or left_block.version > right_block.version:

-Rob

review: Approve

[John A Meinel (jameinel)]

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

Robert Collins wrote:
> Review: Approve
> review: approve
>
> On Wed, 2010-02-03 at 20:19 +0000, John A Meinel wrote:
>> + elif right_block is None:
>> + next_block = left_block
>> + left_block = step(left_blocks)
>> ...
>
>> + elif left_block.version > right_block.version:
>> + # left comes first
>> + next_block = left_block
>> + left_block = step(left_blocks)
>
>
> The None and > cases are the same - I think it would be shorter to group
> them.
>
> elif right_block_is None or left_block.version > right_block.version:
>
> -Rob
>
>

I believe I've updated to that style in more recent patches. But I'm
also trying to address some of James's concerns about always being
active. I'm trying to get Strict handling and fallback to regular
processing working (only to find that Strict isn't properly triggering
in some edge cases... :(

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAktq63wACgkQJdeBCYSNAAOBmwCg1EmjxckXoDq5QlGLAln6P9Rz
50gAn02X+OR5lx/dWL/QmrAfSCtr4f7r
=271q
-----END PGP SIGNATURE-----

[lp:~jameinel/bzr-builddeb/changelog-parser]

407. By John A Meinel on 2010-02-04

Sort out some more details for the 3-way merge code.
The main problem I was running into was that the constructor always suppressed parse failures.

[James Westby (james-w)]

I think what I would like to do is use strict=True and then let
merge3 do the merge if we can't parse it. That way we won't drop
text if we can't parse it.

Thanks,

James

[James Westby (james-w)]

Oh, it already has code for that, but I don't understand the comment

 # BASE lines don't end up in the output, so we allow strict=False

Would it also be good to note to the user when we tried to merge but
it failed due to parsing?

Thanks,

James

[John A Meinel (jameinel)]

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

James Westby wrote:
> I think what I would like to do is use strict=True and then let
> merge3 do the merge if we can't parse it. That way we won't drop
> text if we can't parse it.
>
> Thanks,
>
> James
>

That is what I ended up with. I don't know if the current diff shows that.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt1na4ACgkQJdeBCYSNAAMTQgCfYfcKCebNDyQPtpkwkCrInf0S
Bh0Anit1wIn+m7J24Gr7U1p/sBsfQsQS
=h8Ig
-----END PGP SIGNATURE-----

=== modified file 'merge_changelog.py'
--- merge_changelog.py	2010-01-29 10:51:45 +0000
+++ merge_changelog.py	2010-02-04 16:34:14 +0000
@@ -20,243 +20,125 @@
 
 from bzrlib import (
     merge,
+    merge3,
     )
 
+from debian_bundle import changelog
+
 class ChangeLogFileMerge(merge.ConfigurableFileMerger):
 
     name_prefix = 'deb_changelog'
     default_files = ['debian/changelog']
 
     def merge_text(self, params):
-        return 'success', merge_changelog(params.this_lines, params.other_lines)
-
-
-########################################################################
-# Changelog Management
-########################################################################
+        return merge_changelog(params.this_lines, params.other_lines,
+                               params.base_lines)
+
 
 # Regular expression for top of debian/changelog
 CL_RE = re.compile(r'^(\w[-+0-9a-z.]*) \(([^\(\) \t]+)\)((\s+[-0-9a-z]+)+)\;',
                    re.IGNORECASE)
 
-def merge_changelog(left_changelog_lines, right_changelog_lines):
+def merge_changelog(this_lines, other_lines, base_lines=[]):
     """Merge a changelog file."""
 
-    left_cl = read_changelog(left_changelog_lines)
-    right_cl = read_changelog(right_changelog_lines)
+    try:
+        left_cl = read_changelog(this_lines)
+        right_cl = read_changelog(other_lines)
+        # BASE lines don't end up in the output, so we allow strict=False
+        base_cl = read_changelog(base_lines, strict=False)
+    except changelog.ChangelogParseError:
+        return ('not_applicable', None)
 
     content = []
-    # TODO: This is not a 3-way merge, but a 2-way merge
-    #       The resolution is currently 'if left and right have texts that have
-    #       the same "version" string, use left', aka "prefer-mine".
-    #       We could introduce BASE, and cause conflicts, or appropriately
-    #       resolve, etc.
-    #       Note also that this code is only invoked when there is a
-    #       left-and-right change, so merging a pure-right change will take all
-    #       changes.
-    for right_ver, right_text in right_cl:
-        while len(left_cl) and left_cl[0][0] > right_ver:
-            (left_ver, left_text) = left_cl.pop(0)
-            content.append(left_text)
-            content.append('\n')
-
-        while len(left_cl) and left_cl[0][0] == right_ver:
-            (left_ver, left_text) = left_cl.pop(0)
-
-        content.append(right_text)
-        content.append('\n')
-
-    for left_ver, left_text in left_cl:
-        content.append(left_text)
-        content.append('\n')
-	    
-    return content
-
-
-def read_changelog(lines):
+    def step(iterator):
+        try:
+            return iterator.next()
+        except StopIteration:
+            return None
+    left_blocks = dict((b.version, b) for b in left_cl._blocks)
+    right_blocks = dict((b.version, b) for b in right_cl._blocks)
+    # Unfortunately, while version objects implement __eq__ they *don't*
+    # implement __hash__, which means we can't do dict lookups properly, so
+    # instead, we fall back on the version string instead of the object.
+    # Make sure never to try to use right_version in left_blocks because of
+    # this.
+    # We lazily parse the base data, in case we never need it
+    base_blocks = dict((b.version.full_version, b) for b in base_cl._blocks)
+    left_order = iter(sorted(left_blocks.keys(), reverse=True))
+    right_order = iter(sorted(right_blocks.keys(), reverse=True))
+    left_version = step(left_order)
+    right_version = step(right_order)
+
+    # TODO: Do we want to support the ability to delete a section? We could do
+    #       a first-pass algorithm that checks the versions in base versus the
+    #       versions in this and other, to determine what versions should be in
+    #       the output. For now, we just assume that if a version is present in
+    #       any of this or other, then we want it in the output.
+    conflict_status = 'success'
+
+    while left_version is not None or right_version is not None:
+        if (left_version is None or
+            (right_version is not None and right_version > left_version)):
+            next_content = str(right_blocks[right_version])
+            right_version = step(right_order)
+        elif (right_version is None or
+            (left_version is not None and left_version > right_version)):
+            next_content = str(left_blocks[left_version])
+            left_version = step(left_order)
+        else:
+            assert left_version == right_version
+            # Same version, step both
+            # TODO: Conflict if left_version != right
+            # Note: See above comment why we can't use
+            #       right_blocks[left_version] even though they *should* be
+            #       equivalent
+            left_content = str(left_blocks[left_version])
+            right_content = str(right_blocks[right_version])
+            if left_content == right_content:
+                # Identical content
+                next_content = left_content
+            else:
+                # Sides disagree, compare with base
+                base_content = str(base_blocks.get(left_version.full_version,
+                                                   ''))
+                if left_content == base_content:
+                    next_content = right_content
+                elif right_content == base_content:
+                    next_content = left_content
+                else:
+                    # TODO: We could use merge3.Merge3 to try a line-based
+                    #       textual merge on the content. However, for now I'm
+                    #       just going to conflict on the whole region
+                    # Conflict names taken from merge.py
+                    next_content = ('<<<<<<< TREE\n'
+                                    + left_content
+                                    + '=======\n'
+                                    + right_content
+                                    + '>>>>>>> MERGE-SOURCE\n'
+                                   )
+                    conflict_status = 'conflicted'
+            next_block = left_blocks[left_version]
+            left_version = step(left_order)
+            right_version = step(right_order)
+        content.append(next_content)
+
+    return conflict_status, content
+
+
+def read_changelog(lines, strict=True):
     """Return a parsed changelog file."""
-    entries = []
-
-    (ver, text) = (None, "")
-    for line in lines:
-        match = CL_RE.search(line)
-        if match:
-            try:
-                ver = Version(match.group(2))
-            except ValueError:
-                ver = None
-
-            text += line
-        elif line.startswith(" -- "):
-            if ver is None:
-                ver = Version("0")
-
-            text += line
-            entries.append((ver, text))
-            (ver, text) = (None, "")
-        elif len(line.strip()) or ver is not None:
-            text += line
-
-    if len(text):
-        entries.append((ver, text))
-
-    return entries
-
-########################################################################
-# Version parsing code
-########################################################################
-# Regular expressions make validating things easy
-valid_epoch = re.compile(r'^[0-9]+$')
-valid_upstream = re.compile(r'^[A-Za-z0-9+:.~-]*$')
-valid_revision = re.compile(r'^[A-Za-z0-9+.~]+$')
-
-# Character comparison table for upstream and revision components
-cmp_table = "~ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz+-.:"
-
-
-class Version(object):
-    """Debian version number.
-
-    This class is designed to be reasonably transparent and allow you
-    to write code like:
-
-    |   s.version >= '1.100-1'
-
-    The comparison will be done according to Debian rules, so '1.2' will
-    compare lower.
-
-    Properties:
-      epoch       Epoch
-      upstream    Upstream version
-      revision    Debian/local revision
-    """
-
-    def __init__(self, ver):
-        """Parse a string or number into the three components."""
-        self.epoch = 0
-        self.upstream = None
-        self.revision = None
-
-        ver = str(ver)
-        if not len(ver):
-            raise ValueError
-
-        # Epoch is component before first colon
-        idx = ver.find(":")
-        if idx != -1:
-            self.epoch = ver[:idx]
-            if not len(self.epoch):
-                raise ValueError
-            if not valid_epoch.search(self.epoch):
-                raise ValueError
-            ver = ver[idx+1:]
-
-        # Revision is component after last hyphen
-        idx = ver.rfind("-")
-        if idx != -1:
-            self.revision = ver[idx+1:]
-            if not len(self.revision):
-                raise ValueError
-            if not valid_revision.search(self.revision):
-                raise ValueError
-            ver = ver[:idx]
-
-        # Remaining component is upstream
-        self.upstream = ver
-        if not len(self.upstream):
-            raise ValueError
-        if not valid_upstream.search(self.upstream):
-            raise ValueError
-
-        self.epoch = int(self.epoch)
-
-    def getWithoutEpoch(self):
-        """Return the version without the epoch."""
-        str = self.upstream
-        if self.revision is not None:
-            str += "-%s" % (self.revision,)
-        return str
-
-    without_epoch = property(getWithoutEpoch)
-
-    def __str__(self):
-        """Return the class as a string for printing."""
-        str = ""
-        if self.epoch > 0:
-            str += "%d:" % (self.epoch,)
-        str += self.upstream
-        if self.revision is not None:
-            str += "-%s" % (self.revision,)
-        return str
-
-    def __repr__(self):
-        """Return a debugging representation of the object."""
-        return "<%s epoch: %d, upstream: %r, revision: %r>" \
-               % (self.__class__.__name__, self.epoch,
-                  self.upstream, self.revision)
-
-    def __cmp__(self, other):
-        """Compare two Version classes."""
-        other = Version(other)
-
-        result = cmp(self.epoch, other.epoch)
-        if result != 0: return result
-
-        result = deb_cmp(self.upstream, other.upstream)
-        if result != 0: return result
-
-        result = deb_cmp(self.revision or "", other.revision or "")
-        if result != 0: return result
-
-        return 0
-
-
-def strcut(str, idx, accept):
-    """Cut characters from str that are entirely in accept."""
-    ret = ""
-    while idx < len(str) and str[idx] in accept:
-        ret += str[idx]
-        idx += 1
-
-    return (ret, idx)
-
-def deb_order(str, idx):
-    """Return the comparison order of two characters."""
-    if idx >= len(str):
-        return 0
-    elif str[idx] == "~":
-        return -1
-    else:
-        return cmp_table.index(str[idx])
-
-def deb_cmp_str(x, y):
-    """Compare two strings in a deb version."""
-    idx = 0
-    while (idx < len(x)) or (idx < len(y)):
-        result = deb_order(x, idx) - deb_order(y, idx)
-        if result < 0:
-            return -1
-        elif result > 0:
-            return 1
-
-        idx += 1
-
-    return 0
-
-def deb_cmp(x, y):
-    """Implement the string comparison outlined by Debian policy."""
-    x_idx = y_idx = 0
-    while x_idx < len(x) or y_idx < len(y):
-        # Compare strings
-        (x_str, x_idx) = strcut(x, x_idx, cmp_table)
-        (y_str, y_idx) = strcut(y, y_idx, cmp_table)
-        result = deb_cmp_str(x_str, y_str)
-        if result != 0: return result
-
-        # Compare numbers
-        (x_str, x_idx) = strcut(x, x_idx, "0123456789")
-        (y_str, y_idx) = strcut(y, y_idx, "0123456789")
-        result = cmp(int(x_str or "0"), int(y_str or "0"))
-        if result != 0: return result
-
-    return 0
+    # Note: There appears to be a bug in Changelog if you pass it an iterable
+    #       of lines (like a file obj, or a list of lines). Specifically, it
+    #       does not strip trailing newlines, and it adds ones back in, so you
+    #       get doubled blank lines... :(
+    #       So we just ''.join() the lines and don't worry about it
+    # Note: There is also a bug that the Changelog constructor suppresses parse
+    #       errors, so we want to always call parse_changelog separately
+    content = ''.join(lines)
+    cl = changelog.Changelog()
+    if content:
+        # We get a warning if we try to parse an empty changelog file, which in
+        # strict mode is an error, so only parse when we have content
+        cl.parse_changelog(content, strict=strict)
+    return cl

=== modified file 'tests/test_merge_changelog.py'
--- tests/test_merge_changelog.py	2010-01-29 10:51:45 +0000
+++ tests/test_merge_changelog.py	2010-02-04 16:34:14 +0000
@@ -1,5 +1,5 @@
 #    Copyright (C) 2010 Canonical Ltd
-#    
+#
 #    This file is part of bzr-builddeb.
 #
 #    bzr-builddeb is free software; you can redistribute it and/or modify
@@ -19,6 +19,10 @@
 
 """Tests for the merge_changelog code."""
 
+import warnings
+
+from debian_bundle import changelog
+
 from bzrlib import (
     memorytree,
     merge,
@@ -28,66 +32,181 @@
 from bzrlib.plugins.builddeb import merge_changelog
 
 
+v_111_2 = """\
+psuedo-prog (1.1.1-2) unstable; urgency=low
+
+  * New upstream release.
+  * Awesome bug fixes.
+
+ -- Joe Foo <[e-mail]>  Thu, 28 Jan 2010 10:45:44 +0000
+
+""".splitlines(True)
+
+
+v_111_2b = """\
+psuedo-prog (1.1.1-2) unstable; urgency=low
+
+  * New upstream release.
+  * Awesome bug fixes.
+  * But more is better
+
+ -- Joe Foo <[e-mail]>  Thu, 28 Jan 2010 10:45:44 +0000
+
+""".splitlines(True)
+
+
+v_111_2c = """\
+psuedo-prog (1.1.1-2) unstable; urgency=low
+
+  * New upstream release.
+  * Yet another content for 1.1.1-2
+
+ -- Joe Foo <[e-mail]>  Thu, 28 Jan 2010 10:45:44 +0000
+
+""".splitlines(True)
+
+
+v_112_1 = """\
+psuedo-prog (1.1.2-1) unstable; urgency=low
+
+  * New upstream release.
+  * No bug fixes :(
+
+ -- Barry Foo <[e-mail]>  Thu, 27 Jan 2010 10:45:44 +0000
+
+""".splitlines(True)
+
+
+v_001_1 = """\
+psuedo-prog (0.0.1-1) unstable; urgency=low
+
+  * New project released!!!!
+  * No bugs evar
+
+ -- Barry Foo <[e-mail]>  Thu, 27 Jan 2010 10:00:44 +0000
+
+""".splitlines(True)
+
+
 class TestReadChangelog(tests.TestCase):
 
     def test_read_changelog(self):
-        lines = """\
-psuedo-prog (1.1.1-2) unstable; urgency=low
-
-  * New upstream release.
-  * Awesome bug fixes.
-
- -- Joe Foo <[e-mail]> Thu, 28 Jan 2010 10:45:44 +0000
-""".splitlines(True)
-
-                
-        entries = merge_changelog.read_changelog(lines)
-        self.assertEqual(1, len(entries))
-
-    
+        cl = merge_changelog.read_changelog(v_112_1)
+        self.assertEqual(1, len(cl._blocks))
+
+
 class TestMergeChangelog(tests.TestCase):
 
-    def assertMergeChangelog(self, expected_lines, this_lines, other_lines):
-        merged_lines = merge_changelog.merge_changelog(this_lines, other_lines)
+    def assertMergeChangelog(self, expected_lines, this_lines, other_lines,
+                             base_lines=[], conflicted=False):
+        status, merged_lines = merge_changelog.merge_changelog(
+                                    this_lines, other_lines, base_lines)
+        if conflicted:
+            self.assertEqual('conflicted', status)
+        else:
+            self.assertEqual('success', status)
         self.assertEqualDiff(''.join(expected_lines), ''.join(merged_lines))
 
     def test_merge_by_version(self):
-        v_111_2 = """\
-psuedo-prog (1.1.1-2) unstable; urgency=low
-
-  * New upstream release.
-  * Awesome bug fixes.
-
- -- Joe Foo <[e-mail]> Thu, 28 Jan 2010 10:45:44 +0000
-
-""".splitlines(True)
-
-        v_112_1 = """\
-psuedo-prog (1.1.2-1) unstable; urgency=low
-
-  * New upstream release.
-  * No bug fixes :(
-
- -- Barry Foo <[e-mail]> Thu, 27 Jan 2010 10:45:44 +0000
-
-""".splitlines(True)
-
-        v_001_1 = """\
-psuedo-prog (0.0.1-1) unstable; urgency=low
-
-  * New project released!!!!
-  * No bugs evar
-
- -- Barry Foo <[e-mail]> Thu, 27 Jan 2010 10:00:44 +0000
-
-""".splitlines(True)
-
         this_lines = v_111_2 + v_001_1
         other_lines = v_112_1 + v_001_1
         expected_lines = v_112_1 + v_111_2 + v_001_1
         self.assertMergeChangelog(expected_lines, this_lines, other_lines)
         self.assertMergeChangelog(expected_lines, other_lines, this_lines)
 
+    def test_this_shorter(self):
+        self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1,
+            this_lines=v_111_2,
+            other_lines=v_112_1 + v_001_1,
+            base_lines=[])
+        self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1,
+            this_lines=v_001_1,
+            other_lines=v_112_1 + v_111_2,
+            base_lines=[])
+
+    def test_other_shorter(self):
+        self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1,
+            this_lines=v_112_1 + v_001_1,
+            other_lines=v_111_2,
+            base_lines=[])
+        self.assertMergeChangelog(v_112_1 + v_111_2 + v_001_1,
+            this_lines=v_112_1 + v_111_2,
+            other_lines=v_001_1,
+            base_lines=[])
+
+    def test_unsorted(self):
+        # Passing in an improperly sorted text should result in a properly
+        # sorted one
+        self.assertMergeChangelog(v_111_2 + v_001_1,
+                                  this_lines = v_001_1 + v_111_2,
+                                  other_lines = [],
+                                  base_lines = [])
+
+    def test_3way_merge(self):
+        # Check that if one of THIS or OTHER matches BASE, then we select the
+        # other content
+        self.assertMergeChangelog(expected_lines=v_111_2,
+                                  this_lines=v_111_2, other_lines=v_111_2b,
+                                  base_lines=v_111_2b)
+        self.assertMergeChangelog(expected_lines=v_111_2b,
+                                  this_lines=v_111_2, other_lines=v_111_2b,
+                                  base_lines=v_111_2)
+
+    def test_3way_conflicted(self):
+        self.assertMergeChangelog(
+            expected_lines=['<<<<<<< TREE\n']
+                           + v_111_2b
+                           + ['=======\n']
+                           + v_111_2c
+                           + ['>>>>>>> MERGE-SOURCE\n'],
+            this_lines=v_111_2b, other_lines=v_111_2c,
+            base_lines=v_111_2,
+            conflicted=True)
+        self.assertMergeChangelog(
+            expected_lines=['<<<<<<< TREE\n']
+                           + v_111_2b
+                           + ['=======\n']
+                           + v_111_2c
+                           + ['>>>>>>> MERGE-SOURCE\n'],
+            this_lines=v_111_2b, other_lines=v_111_2c,
+            base_lines=[],
+            conflicted=True)
+
+    def test_not_valid_changelog(self):
+        invalid_changelog = """\
+psuedo-prog (1.1.1-2) unstable; urgency=low
+
+  * New upstream release.
+  * Awesome bug fixes.
+
+ -- Thu, 28 Jan 2010 10:45:44 +0000
+
+""".splitlines(True)
+        # Missing the author and we don't have allow_missing_author set
+        cl = changelog.Changelog()
+        self.assertRaises(changelog.ChangelogParseError,
+                          cl.parse_changelog, ''.join(invalid_changelog), strict=True)
+        # If strict parsing fails, don't try to do special merging
+        self.assertEqual(('not_applicable', None),
+            merge_changelog.merge_changelog(invalid_changelog, v_111_2,
+                                            v_111_2))
+        self.assertEqual(('not_applicable', None),
+            merge_changelog.merge_changelog(v_111_2, invalid_changelog,
+                                            v_111_2))
+        # We are non-strict about parsing BASE, because its contents are not
+        # included in the output.
+        # This triggers a warning, but we don't want to clutter the test run
+        cur_filters = warnings.filters[:]
+        warnings.simplefilter('ignore', UserWarning)
+        try:
+            self.assertMergeChangelog(v_112_1 + v_111_2,
+                                      this_lines=v_111_2,
+                                      other_lines=v_112_1,
+                                      base_lines=invalid_changelog,
+                                      )
+        finally:
+            warnings.filters = cur_filters[:]
+
 
 class TestChangelogHook(tests.TestCaseWithMemoryTransport):
 
@@ -122,6 +241,7 @@
     def test_changelog_merge_hook_successful(self):
         params, merger = self.make_params()
         params.other_lines = ['']
+        params.base_lines = ['']
         file_merger = builddeb.changelog_merge_hook_factory(merger)
         result, new_content = file_merger.merge_text(params)
         self.assertEqual('success', result)