Merge lp:~jelmer/bzr/merge-dir-prober into lp:bzr

Proposed by Jelmer Vernooij
Status: Rejected
Rejected by: Jelmer Vernooij
Proposed branch: lp:~jelmer/bzr/merge-dir-prober
Merge into: lp:bzr
Diff against target: 509 lines (+107/-58)
12 files modified
NEWS (+12/-0)
bzrlib/builtins.py (+1/-1)
bzrlib/bundle/commands.py (+1/-1)
bzrlib/hooks.py (+1/-1)
bzrlib/merge_directive.py (+64/-28)
bzrlib/send.py (+2/-2)
bzrlib/tests/blackbox/test_bundle_info.py (+1/-1)
bzrlib/tests/blackbox/test_merge.py (+1/-1)
bzrlib/tests/blackbox/test_send.py (+4/-4)
bzrlib/tests/per_repository/test_merge_directive.py (+2/-2)
bzrlib/tests/test_merge_core.py (+2/-2)
bzrlib/tests/test_merge_directive.py (+16/-15)
To merge this branch: bzr merge lp:~jelmer/bzr/merge-dir-prober
Reviewer Review Type Date Requested Status
Aaron Bentley mid-implementation Pending
bzr-core mid-implementation Pending
Review via email: mp+31795@code.launchpad.net

Description of the change

This branch makes it possible to register custom "Prober" classes that can detect merge directives in files. This is necessary so that we can start registering custom merge directives, such as Mercurial bundles, Git-am-style bundles, etc.

There is a single Prober implementation that reads the existing bzr bundle formats.

I've also renamed a few classes to make the distinction between the base class and the bzr implementations a bit clearer:

 BaseMergeDirective -> MergeDirective
 MergeDirective -> BzrMergeDirective1
 MergeDirective2 -> BzrMergeDirective2

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

(I'm not looking for an in-depth review but would be interested to hear whether this is the right overall approach).

Revision history for this message
Aaron Bentley (abentley) wrote :

This seems fine in general, but I believe you're changing the signature of from_lines somewhat. Previously, it accepted an iterable, which could be a collection (i.e. list) or an iterator. Now, it seems a collection is mandatory because the iteration is repeated for every prober, but an iterator would be exhausted by the first prober.

Mandating lists may increase memory use, though of course, a given line could be twelve gigabytes if we were really unlucky. At the very least, you should change the docstring if you change the signature.

Because each prober is handled in sequence, you'll also be consuming the whole list of lines for every probe type, even if there's a header at the top of the file, indicating its kind.

Perhaps it would be better to us a coroutine approach instead. For example:

def get_merge_directive_type(lines):
    for line in lines:
        for prober in probers:
            prober.consume(line)
            if prober.merge_directive_type is not None:
                return prober.merge_directive_type

Revision history for this message
Martin Packman (gz) wrote :

> This seems fine in general, but I believe you're changing the signature of
> from_lines somewhat. Previously, it accepted an iterable, which could be a
> collection (i.e. list) or an iterator. Now, it seems a collection is
> mandatory because the iteration is repeated for every prober, but an iterator
> would be exhausted by the first prober.

Just to note, until I rewrote it in r4792.7.3 from_lines did in fact require a list despite the docstring claiming it took an iterable, so changing the contract to actually require a list is unlikely to cause any kind of regression.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

On Fri, 2010-08-06 at 19:58 +0000, Martin [gz] wrote:
> > This seems fine in general, but I believe you're changing the signature of
> > from_lines somewhat. Previously, it accepted an iterable, which could be a
> > collection (i.e. list) or an iterator. Now, it seems a collection is
> > mandatory because the iteration is repeated for every prober, but an iterator
> > would be exhausted by the first prober.
>
> Just to note, until I rewrote it in r4792.7.3 from_lines did in fact
> require a list despite the docstring claiming it took an iterable, so
> changing the contract to actually require a list is unlikely to cause
> any kind of regression.
Ahh, thanks for letting me know. I thought that was the case so was
surprised to find it was using an iterator now.

Cheers,

Jelmer

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Hi Aaron,

On Thu, 2010-08-05 at 02:42 +0000, Aaron Bentley wrote:
> This seems fine in general, but I believe you're changing the signature
> of from_lines somewhat. Previously, it accepted an iterable, which
> could be a collection (i.e. list) or an iterator. Now, it seems a
> collection is mandatory because the iteration is repeated for every
> prober, but an iterator would be exhausted by the first prober.
>
> Mandating lists may increase memory use, though of course, a given line
> could be twelve gigabytes if we were really unlucky. At the very
> least, you should change the docstring if you change the signature.
>
> Because each prober is handled in sequence, you'll also be consuming
> the whole list of lines for every probe type, even if there's a header
> at the top of the file, indicating its kind.
>
> Perhaps it would be better to us a coroutine approach instead. For example:
That's a good point, for some reason I thought that it scaled with the
size of the file anyway. Using coroutines does indeed seem like a good
approach here.

Thanks for the review.

Cheers,

Jelmer

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-07-29 11:17:57 +0000
+++ NEWS 2010-08-04 22:41:44 +0000
@@ -14,6 +14,14 @@
14Compatibility Breaks14Compatibility Breaks
15********************15********************
1616
17* `BaseMergeDirective` has been renamed to `MergeDirective`.
18 `MergeDirective` has been renamed to `BzrMergeDirective1`,
19 `MergeDirective2` has been renamed to `BzrMergeDirective2`.
20
21 `MergeDirective.from_lines` should still be used for opening
22 merge directives from lines.
23 (Jelmer Vernooij)
24
17* BzrError subclasses no longer support the name "message" to be used25* BzrError subclasses no longer support the name "message" to be used
18 as an argument for __init__ or in _fmt format specification as this26 as an argument for __init__ or in _fmt format specification as this
19 breaks in some Python versions. errors.LockError.__init__ argument27 breaks in some Python versions. errors.LockError.__init__ argument
@@ -33,6 +41,7 @@
33 macrobenchmark suite.41 macrobenchmark suite.
34 (Martin Pool)42 (Martin Pool)
3543
44
36New Features45New Features
37************46************
3847
@@ -84,6 +93,9 @@
84Internals93Internals
85*********94*********
8695
96* `MergeDirective.register_prober` can now be used to register custom
97 probers for non-bzr merge directives. (Jelmer Vernooij)
98
87Testing99Testing
88*******100*******
89101
90102
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2010-07-29 11:17:57 +0000
+++ bzrlib/builtins.py 2010-08-04 22:41:44 +0000
@@ -5061,7 +5061,7 @@
5061 revision_id = ensure_null(revision_id)5061 revision_id = ensure_null(revision_id)
5062 if revision_id == NULL_REVISION:5062 if revision_id == NULL_REVISION:
5063 raise errors.BzrCommandError('No revisions to bundle.')5063 raise errors.BzrCommandError('No revisions to bundle.')
5064 directive = merge_directive.MergeDirective2.from_objects(5064 directive = merge_directive.BzrMergeDirective2.from_objects(
5065 branch.repository, revision_id, time.time(),5065 branch.repository, revision_id, time.time(),
5066 osutils.local_time_offset(), submit_branch,5066 osutils.local_time_offset(), submit_branch,
5067 public_branch=public_branch, include_patch=include_patch,5067 public_branch=public_branch, include_patch=include_patch,
50685068
=== modified file 'bzrlib/bundle/commands.py'
--- bzrlib/bundle/commands.py 2010-04-02 19:12:58 +0000
+++ bzrlib/bundle/commands.py 2010-08-04 22:41:44 +0000
@@ -54,7 +54,7 @@
54 from bzrlib import osutils54 from bzrlib import osutils
55 term_encoding = osutils.get_terminal_encoding()55 term_encoding = osutils.get_terminal_encoding()
56 bundle_info = read_mergeable_from_url(location)56 bundle_info = read_mergeable_from_url(location)
57 if isinstance(bundle_info, merge_directive.BaseMergeDirective):57 if isinstance(bundle_info, merge_directive.MergeDirective):
58 bundle_file = StringIO(bundle_info.get_raw_bundle())58 bundle_file = StringIO(bundle_info.get_raw_bundle())
59 bundle_info = read_bundle(bundle_file)59 bundle_info = read_bundle(bundle_file)
60 else:60 else:
6161
=== modified file 'bzrlib/hooks.py'
--- bzrlib/hooks.py 2010-05-26 04:26:59 +0000
+++ bzrlib/hooks.py 2010-08-04 22:41:44 +0000
@@ -58,7 +58,7 @@
58 ('bzrlib.version_info_formats.format_rio', 'RioVersionInfoBuilder.hooks'),58 ('bzrlib.version_info_formats.format_rio', 'RioVersionInfoBuilder.hooks'),
59 'bzrlib.version_info_formats.format_rio', 'RioVersionInfoBuilderHooks')59 'bzrlib.version_info_formats.format_rio', 'RioVersionInfoBuilderHooks')
60known_hooks.register_lazy(60known_hooks.register_lazy(
61 ('bzrlib.merge_directive', 'BaseMergeDirective.hooks'),61 ('bzrlib.merge_directive', 'MergeDirective.hooks'),
62 'bzrlib.merge_directive', 'MergeDirectiveHooks')62 'bzrlib.merge_directive', 'MergeDirectiveHooks')
6363
6464
6565
=== modified file 'bzrlib/merge_directive.py'
--- bzrlib/merge_directive.py 2010-03-13 02:49:14 +0000
+++ bzrlib/merge_directive.py 2010-08-04 22:41:44 +0000
@@ -64,7 +64,7 @@
64 " provided to the next.", (1, 15, 0), False))64 " provided to the next.", (1, 15, 0), False))
6565
6666
67class BaseMergeDirective(object):67class MergeDirective(object):
68 """A request to perform a merge into a branch.68 """A request to perform a merge into a branch.
6969
70 This is the base class that all merge directive implementations 70 This is the base class that all merge directive implementations
@@ -78,6 +78,8 @@
7878
79 multiple_output_files = False79 multiple_output_files = False
8080
81 _probers = []
82
81 def __init__(self, revision_id, testament_sha1, time, timezone,83 def __init__(self, revision_id, testament_sha1, time, timezone,
82 target_branch, patch=None, source_branch=None, message=None,84 target_branch, patch=None, source_branch=None, message=None,
83 bundle=None):85 bundle=None):
@@ -102,6 +104,28 @@
102 self.source_branch = source_branch104 self.source_branch = source_branch
103 self.message = message105 self.message = message
104106
107 @classmethod
108 def register_prober(klass, prober):
109 """Register a merge directive prober.
110
111 """
112 klass._probers.append(prober)
113
114 @classmethod
115 def from_lines(klass, lines):
116 """Deserialize a MergeRequest from an iterable of lines
117
118 :param lines: An iterable of lines
119 :return: a MergeRequest
120 """
121 for prober_kls in klass._probers:
122 prober = prober_kls()
123 try:
124 return prober.probe_lines(lines)
125 except errors.NotAMergeDirective, e:
126 pass
127 raise e
128
105 def to_lines(self):129 def to_lines(self):
106 """Serialize as a list of lines130 """Serialize as a list of lines
107131
@@ -341,8 +365,7 @@
341 basename, body)365 basename, body)
342366
343367
344class MergeDirective(BaseMergeDirective):368class BzrMergeDirective1(MergeDirective):
345
346 """A request to perform a merge into a branch.369 """A request to perform a merge into a branch.
347370
348 Designed to be serialized and mailed. It provides all the information371 Designed to be serialized and mailed. It provides all the information
@@ -376,7 +399,7 @@
376 :param source_branch: A public location to merge the revision from399 :param source_branch: A public location to merge the revision from
377 :param message: The message to use when committing this merge400 :param message: The message to use when committing this merge
378 """401 """
379 BaseMergeDirective.__init__(self, revision_id, testament_sha1, time,402 MergeDirective.__init__(self, revision_id, testament_sha1, time,
380 timezone, target_branch, patch, source_branch, message)403 timezone, target_branch, patch, source_branch, message)
381 if patch_type not in (None, 'diff', 'bundle'):404 if patch_type not in (None, 'diff', 'bundle'):
382 raise ValueError(patch_type)405 raise ValueError(patch_type)
@@ -402,22 +425,6 @@
402 bundle = property(_bundle)425 bundle = property(_bundle)
403426
404 @classmethod427 @classmethod
405 def from_lines(klass, lines):
406 """Deserialize a MergeRequest from an iterable of lines
407
408 :param lines: An iterable of lines
409 :return: a MergeRequest
410 """
411 line_iter = iter(lines)
412 firstline = ""
413 for line in line_iter:
414 if line.startswith('# Bazaar merge directive format '):
415 return _format_registry.get(line[2:].rstrip())._from_lines(
416 line_iter)
417 firstline = firstline or line.strip()
418 raise errors.NotAMergeDirective(firstline)
419
420 @classmethod
421 def _from_lines(klass, line_iter):428 def _from_lines(klass, line_iter):
422 stanza = rio.read_patch_stanza(line_iter)429 stanza = rio.read_patch_stanza(line_iter)
423 patch_lines = list(line_iter)430 patch_lines = list(line_iter)
@@ -442,7 +449,7 @@
442 except KeyError:449 except KeyError:
443 pass450 pass
444 kwargs['revision_id'] = kwargs['revision_id'].encode('utf-8')451 kwargs['revision_id'] = kwargs['revision_id'].encode('utf-8')
445 return MergeDirective(time=time, timezone=timezone,452 return BzrMergeDirective1(time=time, timezone=timezone,
446 patch_type=patch_type, patch=patch, **kwargs)453 patch_type=patch_type, patch=patch, **kwargs)
447454
448 def to_lines(self):455 def to_lines(self):
@@ -466,7 +473,7 @@
466 return None, self.revision_id, 'inapplicable'473 return None, self.revision_id, 'inapplicable'
467474
468475
469class MergeDirective2(BaseMergeDirective):476class BzrMergeDirective2(MergeDirective):
470477
471 _format_string = 'Bazaar merge directive format 2 (Bazaar 0.90)'478 _format_string = 'Bazaar merge directive format 2 (Bazaar 0.90)'
472479
@@ -475,7 +482,7 @@
475 bundle=None, base_revision_id=None):482 bundle=None, base_revision_id=None):
476 if source_branch is None and bundle is None:483 if source_branch is None and bundle is None:
477 raise errors.NoMergeSource()484 raise errors.NoMergeSource()
478 BaseMergeDirective.__init__(self, revision_id, testament_sha1, time,485 MergeDirective.__init__(self, revision_id, testament_sha1, time,
479 timezone, target_branch, patch, source_branch, message)486 timezone, target_branch, patch, source_branch, message)
480 self.bundle = bundle487 self.bundle = bundle
481 self.base_revision_id = base_revision_id488 self.base_revision_id = base_revision_id
@@ -655,7 +662,19 @@
655 return 'inapplicable'662 return 'inapplicable'
656663
657664
658class MergeDirectiveFormatRegistry(registry.Registry):665class MergeDirectiveProber(object):
666
667 def probe_lines(self, lines):
668 """Find a merge directive using the contents of a file (as lines).
669
670 :param lines: An iterable over lines
671 :raise NotAMergeDirective: Raised when no merge directive could be opened.
672 :return: A MergeDirective instance
673 """
674 raise NotImplementedError(self.probe_lines)
675
676
677class BzrMergeDirectiveFormatRegistry(registry.Registry):
659678
660 def register(self, directive, format_string=None):679 def register(self, directive, format_string=None):
661 if format_string is None:680 if format_string is None:
@@ -663,11 +682,28 @@
663 registry.Registry.register(self, format_string, directive)682 registry.Registry.register(self, format_string, directive)
664683
665684
666_format_registry = MergeDirectiveFormatRegistry()685class BzrMergeDirectiveProber(MergeDirectiveProber):
667_format_registry.register(MergeDirective)686
668_format_registry.register(MergeDirective2)687 formats = BzrMergeDirectiveFormatRegistry()
688
689 def probe_lines(self, lines):
690 line_iter = iter(lines)
691 firstline = ""
692 for line in line_iter:
693 if line.startswith('# Bazaar merge directive format '):
694 return self.formats.get(line[2:].rstrip())._from_lines(
695 line_iter)
696 firstline = firstline or line.strip()
697 raise errors.NotAMergeDirective(firstline)
698
699
700BzrMergeDirectiveProber.formats.register(BzrMergeDirective1)
701BzrMergeDirectiveProber.formats.register(BzrMergeDirective2)
669# 0.19 never existed. It got renamed to 0.90. But by that point, there were702# 0.19 never existed. It got renamed to 0.90. But by that point, there were
670# already merge directives in the wild that used 0.19. Registering with the old703# already merge directives in the wild that used 0.19. Registering with the old
671# format string to retain compatibility with those merge directives.704# format string to retain compatibility with those merge directives.
672_format_registry.register(MergeDirective2,705BzrMergeDirectiveProber.formats.register(BzrMergeDirective2,
673 'Bazaar merge directive format 2 (Bazaar 0.19)')706 'Bazaar merge directive format 2 (Bazaar 0.19)')
707
708
709MergeDirective.register_prober(BzrMergeDirectiveProber)
674710
=== modified file 'bzrlib/send.py'
--- bzrlib/send.py 2010-04-22 14:18:17 +0000
+++ bzrlib/send.py 2010-08-04 22:41:44 +0000
@@ -156,7 +156,7 @@
156def _send_4(branch, revision_id, submit_branch, public_branch,156def _send_4(branch, revision_id, submit_branch, public_branch,
157 no_patch, no_bundle, message, base_revision_id):157 no_patch, no_bundle, message, base_revision_id):
158 from bzrlib import merge_directive158 from bzrlib import merge_directive
159 return merge_directive.MergeDirective2.from_objects(159 return merge_directive.BzrMergeDirective2.from_objects(
160 branch.repository, revision_id, time.time(),160 branch.repository, revision_id, time.time(),
161 osutils.local_time_offset(), submit_branch,161 osutils.local_time_offset(), submit_branch,
162 public_branch=public_branch, include_patch=not no_patch,162 public_branch=public_branch, include_patch=not no_patch,
@@ -178,7 +178,7 @@
178 else:178 else:
179 patch_type = None179 patch_type = None
180 from bzrlib import merge_directive180 from bzrlib import merge_directive
181 return merge_directive.MergeDirective.from_objects(181 return merge_directive.BzrMergeDirective1.from_objects(
182 branch.repository, revision_id, time.time(),182 branch.repository, revision_id, time.time(),
183 osutils.local_time_offset(), submit_branch,183 osutils.local_time_offset(), submit_branch,
184 public_branch=public_branch, patch_type=patch_type,184 public_branch=public_branch, patch_type=patch_type,
185185
=== modified file 'bzrlib/tests/blackbox/test_bundle_info.py'
--- bzrlib/tests/blackbox/test_bundle_info.py 2009-08-06 05:20:04 +0000
+++ bzrlib/tests/blackbox/test_bundle_info.py 2010-08-04 22:41:44 +0000
@@ -43,7 +43,7 @@
43 self.run_bzr_error(['--verbose requires a merge directive'],43 self.run_bzr_error(['--verbose requires a merge directive'],
44 'bundle-info -v bundle')44 'bundle-info -v bundle')
45 target = self.make_branch('target')45 target = self.make_branch('target')
46 md = merge_directive.MergeDirective2.from_objects(46 md = merge_directive.BzrMergeDirective2.from_objects(
47 source.branch.repository, 'rev1', 0, 0, 'target',47 source.branch.repository, 'rev1', 0, 0, 'target',
48 base_revision_id='null:')48 base_revision_id='null:')
49 directive = open('directive', 'wb')49 directive = open('directive', 'wb')
5050
=== modified file 'bzrlib/tests/blackbox/test_merge.py'
--- bzrlib/tests/blackbox/test_merge.py 2010-05-20 18:23:17 +0000
+++ bzrlib/tests/blackbox/test_merge.py 2010-08-04 22:41:44 +0000
@@ -414,7 +414,7 @@
414414
415 def write_directive(self, filename, source, target, revision_id,415 def write_directive(self, filename, source, target, revision_id,
416 base_revision_id=None, mangle_patch=False):416 base_revision_id=None, mangle_patch=False):
417 md = merge_directive.MergeDirective2.from_objects(417 md = merge_directive.BzrMergeDirective2.from_objects(
418 source.repository, revision_id, 0, 0, target,418 source.repository, revision_id, 0, 0, target,
419 base_revision_id=base_revision_id)419 base_revision_id=base_revision_id)
420 if mangle_patch:420 if mangle_patch:
421421
=== modified file 'bzrlib/tests/blackbox/test_send.py'
--- bzrlib/tests/blackbox/test_send.py 2010-04-28 10:30:48 +0000
+++ bzrlib/tests/blackbox/test_send.py 2010-08-04 22:41:44 +0000
@@ -234,7 +234,7 @@
234234
235 def test_format(self):235 def test_format(self):
236 md = self.get_MD(['--format=4'])236 md = self.get_MD(['--format=4'])
237 self.assertIs(merge_directive.MergeDirective2, md.__class__)237 self.assertIs(merge_directive.BzrMergeDirective2, md.__class__)
238 self.assertFormatIs('# Bazaar revision bundle v4', md)238 self.assertFormatIs('# Bazaar revision bundle v4', md)
239239
240 md = self.get_MD(['--format=0.9'])240 md = self.get_MD(['--format=0.9'])
@@ -242,7 +242,7 @@
242242
243 md = self.get_MD(['--format=0.9'], cmd=['bundle'])243 md = self.get_MD(['--format=0.9'], cmd=['bundle'])
244 self.assertFormatIs('# Bazaar revision bundle v0.9', md)244 self.assertFormatIs('# Bazaar revision bundle v0.9', md)
245 self.assertIs(merge_directive.MergeDirective, md.__class__)245 self.assertIs(merge_directive.BzrMergeDirective1, md.__class__)
246246
247 self.run_bzr_error(['Bad value .* for option .format.'],247 self.run_bzr_error(['Bad value .* for option .format.'],
248 'send -f branch -o- --format=0.999')[0]248 'send -f branch -o- --format=0.999')[0]
@@ -251,7 +251,7 @@
251 parent_config = branch.Branch.open('parent').get_config()251 parent_config = branch.Branch.open('parent').get_config()
252 parent_config.set_user_option('child_submit_format', '4')252 parent_config.set_user_option('child_submit_format', '4')
253 md = self.get_MD([])253 md = self.get_MD([])
254 self.assertIs(merge_directive.MergeDirective2, md.__class__)254 self.assertIs(merge_directive.BzrMergeDirective2, md.__class__)
255255
256 parent_config.set_user_option('child_submit_format', '0.9')256 parent_config.set_user_option('child_submit_format', '0.9')
257 md = self.get_MD([])257 md = self.get_MD([])
@@ -259,7 +259,7 @@
259259
260 md = self.get_MD([], cmd=['bundle'])260 md = self.get_MD([], cmd=['bundle'])
261 self.assertFormatIs('# Bazaar revision bundle v0.9', md)261 self.assertFormatIs('# Bazaar revision bundle v0.9', md)
262 self.assertIs(merge_directive.MergeDirective, md.__class__)262 self.assertIs(merge_directive.BzrMergeDirective1, md.__class__)
263263
264 parent_config.set_user_option('child_submit_format', '0.999')264 parent_config.set_user_option('child_submit_format', '0.999')
265 self.run_bzr_error(["No such send format '0.999'"],265 self.run_bzr_error(["No such send format '0.999'"],
266266
=== modified file 'bzrlib/tests/per_repository/test_merge_directive.py'
--- bzrlib/tests/per_repository/test_merge_directive.py 2009-07-22 17:22:06 +0000
+++ bzrlib/tests/per_repository/test_merge_directive.py 2010-08-04 22:41:44 +0000
@@ -48,7 +48,7 @@
48 return b1, b248 return b1, b2
4949
50 def create_merge_directive(self, source_branch, submit_url):50 def create_merge_directive(self, source_branch, submit_url):
51 return merge_directive.MergeDirective2.from_objects(51 return merge_directive.BzrMergeDirective2.from_objects(
52 source_branch.repository,52 source_branch.repository,
53 source_branch.last_revision(),53 source_branch.last_revision(),
54 time=1247775710, timezone=0,54 time=1247775710, timezone=0,
@@ -58,7 +58,7 @@
58 source_branch, target_branch = self.make_two_branches()58 source_branch, target_branch = self.make_two_branches()
59 directive = self.create_merge_directive(source_branch,59 directive = self.create_merge_directive(source_branch,
60 target_branch.base)60 target_branch.base)
61 self.assertIsInstance(directive, merge_directive.MergeDirective2)61 self.assertIsInstance(directive, merge_directive.BzrMergeDirective2)
6262
6363
64 def test_create_and_install_directive(self):64 def test_create_and_install_directive(self):
6565
=== modified file 'bzrlib/tests/test_merge_core.py'
--- bzrlib/tests/test_merge_core.py 2010-02-23 07:43:11 +0000
+++ bzrlib/tests/test_merge_core.py 2010-08-04 22:41:44 +0000
@@ -795,7 +795,7 @@
795795
796 def test_from_mergeable(self):796 def test_from_mergeable(self):
797 this, other = self.prepare_for_merging()797 this, other = self.prepare_for_merging()
798 md = merge_directive.MergeDirective2.from_objects(798 md = merge_directive.BzrMergeDirective2.from_objects(
799 other.branch.repository, 'rev3', 0, 0, 'this')799 other.branch.repository, 'rev3', 0, 0, 'this')
800 other.lock_read()800 other.lock_read()
801 self.addCleanup(other.unlock)801 self.addCleanup(other.unlock)
@@ -816,7 +816,7 @@
816 this, other = self.prepare_for_merging()816 this, other = self.prepare_for_merging()
817 other.lock_write()817 other.lock_write()
818 self.addCleanup(other.unlock)818 self.addCleanup(other.unlock)
819 md = merge_directive.MergeDirective.from_objects(819 md = merge_directive.BzrMergeDirective1.from_objects(
820 other.branch.repository, 'rev3', 0, 0, 'this')820 other.branch.repository, 'rev3', 0, 0, 'this')
821 merger, verified = Merger.from_mergeable(this, md,821 merger, verified = Merger.from_mergeable(this, md,
822 None)822 None)
823823
=== modified file 'bzrlib/tests/test_merge_directive.py'
--- bzrlib/tests/test_merge_directive.py 2009-05-11 19:11:14 +0000
+++ bzrlib/tests/test_merge_directive.py 2010-08-04 22:41:44 +0000
@@ -247,7 +247,7 @@
247 def make_merge_directive(self, revision_id, testament_sha1, time, timezone,247 def make_merge_directive(self, revision_id, testament_sha1, time, timezone,
248 target_branch, patch=None, patch_type=None,248 target_branch, patch=None, patch_type=None,
249 source_branch=None, message=None):249 source_branch=None, message=None):
250 return merge_directive.MergeDirective(revision_id, testament_sha1,250 return merge_directive.BzrMergeDirective1(revision_id, testament_sha1,
251 time, timezone, target_branch, patch, patch_type,251 time, timezone, target_branch, patch, patch_type,
252 source_branch, message)252 source_branch, message)
253253
@@ -258,11 +258,12 @@
258 def test_require_patch(self):258 def test_require_patch(self):
259 time = 500.0259 time = 500.0
260 timezone = 120260 timezone = 120
261 self.assertRaises(errors.PatchMissing, merge_directive.MergeDirective,261 self.assertRaises(errors.PatchMissing,
262 merge_directive.BzrMergeDirective1,
262 'example:', 'sha', time, timezone, 'http://example.com',263 'example:', 'sha', time, timezone, 'http://example.com',
263 patch_type='bundle')264 patch_type='bundle')
264 md = merge_directive.MergeDirective('example:', 'sha1', time, timezone,265 md = merge_directive.BzrMergeDirective1('example:', 'sha1', time,
265 'http://example.com', source_branch="http://example.org",266 timezone, 'http://example.com', source_branch="http://example.org",
266 patch='', patch_type='diff')267 patch='', patch_type='diff')
267 self.assertEqual(md.patch, '')268 self.assertEqual(md.patch, '')
268269
@@ -284,7 +285,7 @@
284 patch = None285 patch = None
285 else:286 else:
286 bundle = None287 bundle = None
287 return merge_directive.MergeDirective2(revision_id, testament_sha1,288 return merge_directive.BzrMergeDirective2(revision_id, testament_sha1,
288 time, timezone, target_branch, patch, source_branch, message,289 time, timezone, target_branch, patch, source_branch, message,
289 bundle, base_revision_id)290 bundle, base_revision_id)
290291
@@ -509,7 +510,7 @@
509 md.install_revisions(tree_b.branch.repository)510 md.install_revisions(tree_b.branch.repository)
510 base, revision, verified = md.get_merge_request(511 base, revision, verified = md.get_merge_request(
511 tree_b.branch.repository)512 tree_b.branch.repository)
512 if isinstance(md, merge_directive.MergeDirective):513 if isinstance(md, merge_directive.BzrMergeDirective1):
513 self.assertIs(None, base)514 self.assertIs(None, base)
514 self.assertEqual('inapplicable', verified)515 self.assertEqual('inapplicable', verified)
515 else:516 else:
@@ -522,7 +523,7 @@
522 public_branch=tree_a.branch.base)523 public_branch=tree_a.branch.base)
523 base, revision, verified = md.get_merge_request(524 base, revision, verified = md.get_merge_request(
524 tree_b.branch.repository)525 tree_b.branch.repository)
525 if isinstance(md, merge_directive.MergeDirective):526 if isinstance(md, merge_directive.BzrMergeDirective1):
526 self.assertIs(None, base)527 self.assertIs(None, base)
527 self.assertEqual('inapplicable', verified)528 self.assertEqual('inapplicable', verified)
528 else:529 else:
@@ -533,7 +534,7 @@
533 public_branch=tree_a.branch.base)534 public_branch=tree_a.branch.base)
534 base, revision, verified = md.get_merge_request(535 base, revision, verified = md.get_merge_request(
535 tree_b.branch.repository)536 tree_b.branch.repository)
536 if isinstance(md, merge_directive.MergeDirective):537 if isinstance(md, merge_directive.BzrMergeDirective1):
537 self.assertIs(None, base)538 self.assertIs(None, base)
538 self.assertEqual('inapplicable', verified)539 self.assertEqual('inapplicable', verified)
539 else:540 else:
@@ -542,7 +543,7 @@
542 md.patch='asdf'543 md.patch='asdf'
543 base, revision, verified = md.get_merge_request(544 base, revision, verified = md.get_merge_request(
544 tree_b.branch.repository)545 tree_b.branch.repository)
545 if isinstance(md, merge_directive.MergeDirective):546 if isinstance(md, merge_directive.BzrMergeDirective1):
546 self.assertIs(None, base)547 self.assertIs(None, base)
547 self.assertEqual('inapplicable', verified)548 self.assertEqual('inapplicable', verified)
548 else:549 else:
@@ -606,7 +607,7 @@
606 ' explicit bases.')607 ' explicit bases.')
607 repository.lock_write()608 repository.lock_write()
608 try:609 try:
609 return merge_directive.MergeDirective.from_objects( repository,610 return merge_directive.BzrMergeDirective1.from_objects(repository,
610 revision_id, time, timezone, target_branch, patch_type,611 revision_id, time, timezone, target_branch, patch_type,
611 local_target_branch, public_branch, message)612 local_target_branch, public_branch, message)
612 finally:613 finally:
@@ -615,7 +616,7 @@
615 def make_merge_directive(self, revision_id, testament_sha1, time, timezone,616 def make_merge_directive(self, revision_id, testament_sha1, time, timezone,
616 target_branch, patch=None, patch_type=None,617 target_branch, patch=None, patch_type=None,
617 source_branch=None, message=None):618 source_branch=None, message=None):
618 return merge_directive.MergeDirective(revision_id, testament_sha1,619 return merge_directive.BzrMergeDirective1(revision_id, testament_sha1,
619 time, timezone, target_branch, patch, patch_type,620 time, timezone, target_branch, patch, patch_type,
620 source_branch, message)621 source_branch, message)
621622
@@ -634,7 +635,7 @@
634 include_patch = (patch_type in ('bundle', 'diff'))635 include_patch = (patch_type in ('bundle', 'diff'))
635 include_bundle = (patch_type == 'bundle')636 include_bundle = (patch_type == 'bundle')
636 self.assertTrue(patch_type in ('bundle', 'diff', None))637 self.assertTrue(patch_type in ('bundle', 'diff', None))
637 return merge_directive.MergeDirective2.from_objects(638 return merge_directive.BzrMergeDirective2.from_objects(
638 repository, revision_id, time, timezone, target_branch,639 repository, revision_id, time, timezone, target_branch,
639 include_patch, include_bundle, local_target_branch, public_branch,640 include_patch, include_bundle, local_target_branch, public_branch,
640 message, base_revision_id)641 message, base_revision_id)
@@ -647,7 +648,7 @@
647 patch = None648 patch = None
648 else:649 else:
649 bundle = None650 bundle = None
650 return merge_directive.MergeDirective2(revision_id, testament_sha1,651 return merge_directive.BzrMergeDirective2(revision_id, testament_sha1,
651 time, timezone, target_branch, patch, source_branch, message,652 time, timezone, target_branch, patch, source_branch, message,
652 bundle, base_revision_id)653 bundle, base_revision_id)
653654
@@ -662,7 +663,7 @@
662 public_branch=tree_a.branch.base, base_revision_id='null:')663 public_branch=tree_a.branch.base, base_revision_id='null:')
663 self.assertEqual('null:', md.base_revision_id)664 self.assertEqual('null:', md.base_revision_id)
664 lines = md.to_lines()665 lines = md.to_lines()
665 md2 = merge_directive.MergeDirective.from_lines(lines)666 md2 = merge_directive.BzrMergeDirective1.from_lines(lines)
666 self.assertEqual(md2.base_revision_id, md.base_revision_id)667 self.assertEqual(md2.base_revision_id, md.base_revision_id)
667668
668 def test_patch_verification(self):669 def test_patch_verification(self):
@@ -735,7 +736,7 @@
735 'merge_request_body', test_hook, 'test')736 'merge_request_body', test_hook, 'test')
736 tree = self.make_branch_and_tree('foo')737 tree = self.make_branch_and_tree('foo')
737 tree.commit('foo')738 tree.commit('foo')
738 directive = merge_directive.MergeDirective2(739 directive = merge_directive.BzrMergeDirective2(
739 tree.branch.last_revision(), 'sha', 0, 0, 'sha',740 tree.branch.last_revision(), 'sha', 0, 0, 'sha',
740 source_branch=tree.branch.base,741 source_branch=tree.branch.base,
741 base_revision_id=tree.branch.last_revision(),742 base_revision_id=tree.branch.last_revision(),