Rev 4802: (robertc) Fix performance issue in new merge hook, in file:///home/pqm/archives/thelove/bzr/2.1/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Jan 28 19:06:15 GMT 2010


At file:///home/pqm/archives/thelove/bzr/2.1/

------------------------------------------------------------
revno: 4802 [merge]
revision-id: pqm at pqm.ubuntu.com-20100128190610-72xh44a4nct8aq11
parent: pqm at pqm.ubuntu.com-20100121221710-2vc21zjy3lnxbg6y
parent: robertc at robertcollins.net-20100128181315-8ea4e6jg15nl5ol7
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.1
timestamp: Thu 2010-01-28 19:06:10 +0000
message:
  (robertc) Fix performance issue in new merge hook,
  	refactoring at the same time for reuse. (Robert Collins,
  	John Arbash Meinel, bug 513822)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/merge.py                merge.py-20050513021216-953b65a438527106
  bzrlib/plugins/news_merge/__init__.py __init__.py-20100113072530-qj0i1em8th5pqhat-3
  bzrlib/plugins/news_merge/news_merge.py news_merge.py-20100118070008-sb1qe88ha64es7nv-1
  bzrlib/plugins/news_merge/tests/test_news_merge.py test_news_merge.py-20100120143539-19kxox9ma117rejo-1
  bzrlib/tests/per_merger.py     per_merger.py-20091216002111-bzeo6wx2tcfpuj67-1
  bzrlib/tests/test_merge.py     testmerge.py-20050905070950-c1b5aa49ff911024
=== modified file 'NEWS'
--- a/NEWS	2010-01-21 21:13:09 +0000
+++ b/NEWS	2010-01-28 18:13:15 +0000
@@ -5,6 +5,23 @@
 .. contents:: List of Releases
    :depth: 1
 
+bzr 2.1.0rc2 (not released yet)
+###############################
+
+:Codename: 
+:2.1.0rc2: ???
+
+API Changes
+***********
+
+* The new ``merge_file_content`` hook point has been altered to provide a
+  better API where state for extensions can be stored rather than the
+  too-simple function based approach. This fixes a performance regression
+  where branch configuration would be parsed per-file during merge. As
+  part of this the included news_merger has been refactored into a base
+  helper class ``bzrlib.merge.ConfigurableFileMerger``.
+  (Robert Collins, John Arbash Meinel, #513822)
+
 bzr 2.1.0rc1
 ############
 

=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py	2010-01-20 16:05:28 +0000
+++ b/bzrlib/merge.py	2010-01-28 18:05:44 +0000
@@ -57,23 +57,116 @@
     def __init__(self):
         hooks.Hooks.__init__(self)
         self.create_hook(hooks.HookPoint('merge_file_content',
-            "Called when file content needs to be merged (including when one "
-            "side has deleted the file and the other has changed it)."
-            "merge_file_content is called with a "
-            "bzrlib.merge.MergeHookParams. The function should return a tuple "
-            "of (status, lines), where status is one of 'not_applicable', "
-            "'success', 'conflicted', or 'delete'.  If status is success or "
-            "conflicted, then lines should be an iterable of strings of the "
-            "new file contents.",
+            "Called with a bzrlib.merge.Merger object to create a per file "
+            "merge object when starting a merge. "
+            "Should return either None or a subclass of "
+            "``bzrlib.merge.AbstractPerFileMerger``. "
+            "Such objects will then be called per file "
+            "that needs to be merged (including when one "
+            "side has deleted the file and the other has changed it). "
+            "See the AbstractPerFileMerger API docs for details on how it is "
+            "used by merge.",
             (2, 1), None))
 
 
+class AbstractPerFileMerger(object):
+    """PerFileMerger objects are used by plugins extending merge for bzrlib.
+
+    See ``bzrlib.plugins.news_merge.news_merge`` for an example concrete class.
+    
+    :ivar merger: The Merge3Merger performing the merge.
+    """
+
+    def __init__(self, merger):
+        """Create a PerFileMerger for use with merger."""
+        self.merger = merger
+
+    def merge_contents(self, merge_params):
+        """Attempt to merge the contents of a single file.
+        
+        :param merge_params: A bzrlib.merge.MergeHookParams
+        :return : A tuple of (status, chunks), where status is one of
+            'not_applicable', 'success', 'conflicted', or 'delete'.  If status
+            is 'success' or 'conflicted', then chunks should be an iterable of
+            strings for the new file contents.
+        """
+        return ('not applicable', None)
+
+
+class ConfigurableFileMerger(AbstractPerFileMerger):
+    """Merge individual files when configured via a .conf file.
+
+    This is a base class for concrete custom file merging logic. Concrete
+    classes should implement ``merge_text``.
+
+    :ivar affected_files: The configured file paths to merge.
+    :cvar name_prefix: The prefix to use when looking up configuration
+        details.
+    :cvar default_files: The default file paths to merge when no configuration
+        is present.
+    """
+
+    name_prefix = None
+    default_files = None
+
+    def __init__(self, merger):
+        super(ConfigurableFileMerger, self).__init__(merger)
+        self.affected_files = None
+        self.default_files = self.__class__.default_files or []
+        self.name_prefix = self.__class__.name_prefix
+        if self.name_prefix is None:
+            raise ValueError("name_prefix must be set.")
+
+    def filename_matches_config(self, params):
+        affected_files = self.affected_files
+        if affected_files is None:
+            config = self.merger.this_tree.branch.get_config()
+            # Until bzr provides a better policy for caching the config, we
+            # just add the part we're interested in to the params to avoid
+            # reading the config files repeatedly (bazaar.conf, location.conf,
+            # branch.conf).
+            config_key = self.name_prefix + '_merge_files'
+            affected_files = config.get_user_option_as_list(config_key)
+            if affected_files is None:
+                # If nothing was specified in the config, use the default.
+                affected_files = self.default_files
+            self.affected_files = affected_files
+        if affected_files:
+            filename = self.merger.this_tree.id2path(params.file_id)
+            if filename in affected_files:
+                return True
+        return False
+
+    def merge_contents(self, params):
+        """Merge the contents of a single file."""
+        # First, check whether this custom merge logic should be used.  We
+        # expect most files should not be merged by this handler.
+        if (
+            # OTHER is a straight winner, rely on default merge.
+            params.winner == 'other' or
+            # THIS and OTHER aren't both files.
+            not params.is_file_merge() or
+            # The filename isn't listed in the 'NAME_merge_files' config
+            # option.
+            not self.filename_matches_config(params)):
+            return 'not_applicable', None
+        return self.merge_text(self, params)
+
+    def merge_text(self, params):
+        """Merge the byte contents of a single file.
+
+        This is called after checking that the merge should be performed in
+        merge_contents, and it should behave as per
+        ``bzrlib.merge.AbstractPerFileMerger.merge_contents``.
+        """
+        raise NotImplementedError(self.merge_text)
+
+
 class MergeHookParams(object):
     """Object holding parameters passed to merge_file_content hooks.
 
-    There are 3 fields hooks can access:
+    There are some fields hooks can access:
 
-    :ivar merger: the Merger object
     :ivar file_id: the file ID of the file being merged
     :ivar trans_id: the transform ID for the merge of this file
     :ivar this_kind: kind of file_id in 'this' tree
@@ -83,7 +176,7 @@
 
     def __init__(self, merger, file_id, trans_id, this_kind, other_kind,
             winner):
-        self.merger = merger
+        self._merger = merger
         self.file_id = file_id
         self.trans_id = trans_id
         self.this_kind = this_kind
@@ -97,17 +190,17 @@
     @decorators.cachedproperty
     def base_lines(self):
         """The lines of the 'base' version of the file."""
-        return self.merger.get_lines(self.merger.base_tree, self.file_id)
+        return self._merger.get_lines(self._merger.base_tree, self.file_id)
 
     @decorators.cachedproperty
     def this_lines(self):
         """The lines of the 'this' version of the file."""
-        return self.merger.get_lines(self.merger.this_tree, self.file_id)
+        return self._merger.get_lines(self._merger.this_tree, self.file_id)
 
     @decorators.cachedproperty
     def other_lines(self):
         """The lines of the 'other' version of the file."""
-        return self.merger.get_lines(self.merger.other_tree, self.file_id)
+        return self._merger.get_lines(self._merger.other_tree, self.file_id)
 
 
 class Merger(object):
@@ -708,12 +801,15 @@
             resolver = self._lca_multi_way
         child_pb = ui.ui_factory.nested_progress_bar()
         try:
+            factories = Merger.hooks['merge_file_content']
+            hooks = [factory(self) for factory in factories] + [self]
+            self.active_hooks = [hook for hook in hooks if hook is not None]
             for num, (file_id, changed, parents3, names3,
                       executable3) in enumerate(entries):
                 child_pb.update('Preparing file merge', num, len(entries))
                 self._merge_names(file_id, parents3, names3, resolver=resolver)
                 if changed:
-                    file_status = self.merge_contents(file_id)
+                    file_status = self._do_merge_contents(file_id)
                 else:
                     file_status = 'unmodified'
                 self._merge_executable(file_id,
@@ -1158,7 +1254,7 @@
             self.tt.adjust_path(names[self.winner_idx[name_winner]],
                                 parent_trans_id, trans_id)
 
-    def merge_contents(self, file_id):
+    def _do_merge_contents(self, file_id):
         """Performs a merge on file_id contents."""
         def contents_pair(tree):
             if file_id not in tree:
@@ -1198,11 +1294,10 @@
         trans_id = self.tt.trans_id_file_id(file_id)
         params = MergeHookParams(self, file_id, trans_id, this_pair[0],
             other_pair[0], winner)
-        hooks = Merger.hooks['merge_file_content']
-        hooks = list(hooks) + [self.default_text_merge]
+        hooks = self.active_hooks
         hook_status = 'not_applicable'
         for hook in hooks:
-            hook_status, lines = hook(params)
+            hook_status, lines = hook.merge_contents(params)
             if hook_status != 'not_applicable':
                 # Don't try any more hooks, this one applies.
                 break
@@ -1279,7 +1374,11 @@
                 'winner is OTHER, but file_id %r not in THIS or OTHER tree'
                 % (file_id,))
 
-    def default_text_merge(self, merge_hook_params):
+    def merge_contents(self, merge_hook_params):
+        """Fallback merge logic after user installed hooks."""
+        # This function is used in merge hooks as the fallback instance.
+        # Perhaps making this function and the functions it calls be a 
+        # a separate class would be better.
         if merge_hook_params.winner == 'other':
             # OTHER is a straight winner, so replace this contents with other
             return self._default_other_winner_merge(merge_hook_params)

=== modified file 'bzrlib/plugins/news_merge/__init__.py'
--- a/bzrlib/plugins/news_merge/__init__.py	2010-01-20 16:05:28 +0000
+++ b/bzrlib/plugins/news_merge/__init__.py	2010-01-28 17:27:16 +0000
@@ -39,46 +39,16 @@
 # overhead of this plugin as minimal as possible.
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
-from bzrlib.plugins.news_merge.news_merge import news_merger
+from bzrlib.plugins.news_merge import news_merge as _mod_news_merge
 """)
 
 from bzrlib.merge import Merger
 
 
-def news_merge_hook(params):
-    """Merger.merge_file_content hook function for bzr-format NEWS files."""
-    # First, check whether this custom merge logic should be used.  We expect
-    # most files should not be merged by this file.
-    if params.winner == 'other':
-        # OTHER is a straight winner, rely on default merge.
-        return 'not_applicable', None
-    elif not params.is_file_merge():
-        # THIS and OTHER aren't both files.
-        return 'not_applicable', None
-    elif not filename_matches_config(params):
-        # The filename isn't listed in the 'news_merge_files' config option.
-        return 'not_applicable', None
-    return news_merger(params)
-
-
-def filename_matches_config(params):
-    affected_files = getattr(params, '_news_merge_affected_files', None)
-    if affected_files is None:
-        config = params.merger.this_tree.branch.get_config()
-        # Until bzr provides a better policy for caching the config, we just
-        # add the part we're interested in to the params to avoid reading the
-        # config files repeatedly (bazaar.conf, location.conf, branch.conf).
-        affected_files = config.get_user_option_as_list('news_merge_files')
-        if affected_files is None:
-            # If nothing was specified in the config, we have nothing to do,
-            # but we use None in the params to start the caching.
-            affected_files = []
-        params._news_merge_affected_files = affected_files
-    if affected_files:
-        filename = params.merger.this_tree.id2path(params.file_id)
-        if filename in affected_files:
-            return True
-    return False
+def news_merge_hook(merger):
+    """Merger.merge_file_content hook for bzr-format NEWS files."""
+    return _mod_news_merge.NewsMerger(merger)
+
 
 def install_hook():
     Merger.hooks.install_named_hook(

=== modified file 'bzrlib/plugins/news_merge/news_merge.py'
--- a/bzrlib/plugins/news_merge/news_merge.py	2010-01-20 16:05:28 +0000
+++ b/bzrlib/plugins/news_merge/news_merge.py	2010-01-28 18:05:44 +0000
@@ -18,54 +18,61 @@
 
 
 from bzrlib.plugins.news_merge.parser import simple_parse
-from bzrlib import merge3
+from bzrlib import merge, merge3
 
 
 magic_marker = '|NEWS-MERGE-MAGIC-MARKER|'
 
 
-def news_merger(params):
-    """Perform a simple 3-way merge of a bzr NEWS file.
-    
-    Each section of a bzr NEWS file is essentially an ordered set of bullet
-    points, so we can simply take a set of bullet points, determine which
-    bullets to add and which to remove, sort, and reserialize.
-    """
-    # Transform the different versions of the NEWS file into a bunch of text
-    # lines where each line matches one part of the overall structure, e.g. a
-    # heading or bullet.
-    def munge(lines):
-        return list(blocks_to_fakelines(simple_parse(''.join(lines))))
-    this_lines = munge(params.this_lines)
-    other_lines = munge(params.other_lines)
-    base_lines = munge(params.base_lines)
-    m3 = merge3.Merge3(base_lines, this_lines, other_lines)
-    result_lines = []
-    for group in m3.merge_groups():
-        if group[0] == 'conflict':
-            _, base, a, b = group
-            # Are all the conflicting lines bullets?  If so, we can merge this.
-            for line_set in [base, a, b]:
-                for line in line_set:
-                    if not line.startswith('bullet'):
-                        # Something else :(
-                        # Maybe the default merge can cope.
-                        return 'not_applicable', None
-            # Calculate additions and deletions.
-            new_in_a = set(a).difference(base)
-            new_in_b = set(b).difference(base)
-            all_new = new_in_a.union(new_in_b)
-            deleted_in_a = set(base).difference(a)
-            deleted_in_b = set(base).difference(b)
-            # Combine into the final set of bullet points.
-            final = all_new.difference(deleted_in_a).difference(deleted_in_b)
-            # Sort, and emit.
-            final = sorted(final, key=sort_key)
-            result_lines.extend(final)
-        else:
-            result_lines.extend(group[1])
-    # Transform the merged elements back into real blocks of lines.
-    return 'success', list(fakelines_to_blocks(result_lines))
+class NewsMerger(merge.ConfigurableFileMerger):
+    """Merge bzr NEWS files."""
+
+    name_prefix = "news"
+
+    def merge_text(self, params):
+        """Perform a simple 3-way merge of a bzr NEWS file.
+        
+        Each section of a bzr NEWS file is essentially an ordered set of bullet
+        points, so we can simply take a set of bullet points, determine which
+        bullets to add and which to remove, sort, and reserialize.
+        """
+        # Transform the different versions of the NEWS file into a bunch of
+        # text lines where each line matches one part of the overall
+        # structure, e.g. a heading or bullet.
+        def munge(lines):
+            return list(blocks_to_fakelines(simple_parse(''.join(lines))))
+        this_lines = munge(params.this_lines)
+        other_lines = munge(params.other_lines)
+        base_lines = munge(params.base_lines)
+        m3 = merge3.Merge3(base_lines, this_lines, other_lines)
+        result_lines = []
+        for group in m3.merge_groups():
+            if group[0] == 'conflict':
+                _, base, a, b = group
+                # Are all the conflicting lines bullets?  If so, we can merge
+                # this.
+                for line_set in [base, a, b]:
+                    for line in line_set:
+                        if not line.startswith('bullet'):
+                            # Something else :(
+                            # Maybe the default merge can cope.
+                            return 'not_applicable', None
+                # Calculate additions and deletions.
+                new_in_a = set(a).difference(base)
+                new_in_b = set(b).difference(base)
+                all_new = new_in_a.union(new_in_b)
+                deleted_in_a = set(base).difference(a)
+                deleted_in_b = set(base).difference(b)
+                # Combine into the final set of bullet points.
+                final = all_new.difference(deleted_in_a).difference(
+                    deleted_in_b)
+                # Sort, and emit.
+                final = sorted(final, key=sort_key)
+                result_lines.extend(final)
+            else:
+                result_lines.extend(group[1])
+        # Transform the merged elements back into real blocks of lines.
+        return 'success', list(fakelines_to_blocks(result_lines))
 
 
 def blocks_to_fakelines(blocks):

=== modified file 'bzrlib/plugins/news_merge/tests/test_news_merge.py'
--- a/bzrlib/plugins/news_merge/tests/test_news_merge.py	2010-01-20 16:05:28 +0000
+++ b/bzrlib/plugins/news_merge/tests/test_news_merge.py	2010-01-28 18:01:03 +0000
@@ -16,36 +16,14 @@
 
 # FIXME: This is totally incomplete but I'm only the patch pilot :-)
 # -- vila 100120
+# Note that the single test from this file is now in
+# test_merge.TestConfigurableFileMerger -- rbc 20100129.
 
 from bzrlib import (
     option,
     tests,
     )
+from bzrlib.merge import Merger
 from bzrlib.plugins import news_merge
+import bzrlib.plugins.news_merge.news_merge
 from bzrlib.tests import test_merge_core
-
-
-class TestFilenameMatchesConfig(tests.TestCaseWithTransport):
-
-    def test_affected_files_cached(self):
-        """Ensures that the config variable is cached"""
-        news_merge.install_hook()
-        self.affected_files = None
-        orig = news_merge.filename_matches_config
-        def wrapper(params):
-            ret = orig(params)
-            # Capture the value set by the hook
-            self.affected_files = params._news_merge_affected_files
-            return ret
-        def restore():
-            news_merge.filename_matches_config = orig
-        self.addCleanup(restore)
-        news_merge.filename_matches_config = wrapper
-
-        builder = test_merge_core.MergeBuilder(self.test_base_dir)
-        self.addCleanup(builder.cleanup)
-        builder.add_file('NEWS', builder.tree_root, 'name1', 'text1', True)
-        builder.change_contents('NEWS', other='text4', this='text3')
-        conflicts = builder.merge()
-        # The hook should set the variable
-        self.assertIsNot(None, self.affected_files)

=== modified file 'bzrlib/tests/per_merger.py'
--- a/bzrlib/tests/per_merger.py	2010-01-20 16:05:28 +0000
+++ b/bzrlib/tests/per_merger.py	2010-01-28 17:27:16 +0000
@@ -198,55 +198,85 @@
         TestCaseWithTransport.setUp(self)
         self.hook_log = []
 
+    def install_hook_inactive(self):
+        def inactive_factory(merger):
+            # This hook is never active
+            self.hook_log.append(('inactive',))
+            return None
+        _mod_merge.Merger.hooks.install_named_hook(
+            'merge_file_content', inactive_factory, 'test hook (inactive)')
+
     def install_hook_noop(self):
-        def hook_na(merge_params):
-            # This hook unconditionally does nothing.
-            self.hook_log.append(('no-op',))
-            return 'not_applicable', None
+        test = self
+        class HookNA(_mod_merge.AbstractPerFileMerger):
+            def merge_contents(self, merge_params):
+                # This hook unconditionally does nothing.
+                test.hook_log.append(('no-op',))
+                return 'not_applicable', None
+        def hook_na_factory(merger):
+            return HookNA(merger)
         _mod_merge.Merger.hooks.install_named_hook(
-            'merge_file_content', hook_na, 'test hook (no-op)')
+            'merge_file_content', hook_na_factory, 'test hook (no-op)')
 
     def install_hook_success(self):
-        def hook_success(merge_params):
-            self.hook_log.append(('success',))
-            if merge_params.file_id == '1':
-                return 'success', ['text-merged-by-hook']
-            return 'not_applicable', None
+        test = self
+        class HookSuccess(_mod_merge.AbstractPerFileMerger):
+            def merge_contents(self, merge_params):
+                test.hook_log.append(('success',))
+                if merge_params.file_id == '1':
+                    return 'success', ['text-merged-by-hook']
+                return 'not_applicable', None
+        def hook_success_factory(merger):
+            return HookSuccess(merger)
         _mod_merge.Merger.hooks.install_named_hook(
-            'merge_file_content', hook_success, 'test hook (success)')
+            'merge_file_content', hook_success_factory, 'test hook (success)')
 
     def install_hook_conflict(self):
-        def hook_conflict(merge_params):
-            self.hook_log.append(('conflict',))
-            if merge_params.file_id == '1':
-                return 'conflicted', ['text-with-conflict-markers-from-hook']
-            return 'not_applicable', None
+        test = self
+        class HookConflict(_mod_merge.AbstractPerFileMerger):
+            def merge_contents(self, merge_params):
+                test.hook_log.append(('conflict',))
+                if merge_params.file_id == '1':
+                    return ('conflicted',
+                        ['text-with-conflict-markers-from-hook'])
+                return 'not_applicable', None
+        def hook_conflict_factory(merger):
+            return HookConflict(merger)
         _mod_merge.Merger.hooks.install_named_hook(
-            'merge_file_content', hook_conflict, 'test hook (delete)')
+            'merge_file_content', hook_conflict_factory, 'test hook (delete)')
 
     def install_hook_delete(self):
-        def hook_delete(merge_params):
-            self.hook_log.append(('delete',))
-            if merge_params.file_id == '1':
-                return 'delete', None
-            return 'not_applicable', None
+        test = self
+        class HookDelete(_mod_merge.AbstractPerFileMerger):
+            def merge_contents(self, merge_params):
+                test.hook_log.append(('delete',))
+                if merge_params.file_id == '1':
+                    return 'delete', None
+                return 'not_applicable', None
+        def hook_delete_factory(merger):
+            return HookDelete(merger)
         _mod_merge.Merger.hooks.install_named_hook(
-            'merge_file_content', hook_delete, 'test hook (delete)')
+            'merge_file_content', hook_delete_factory, 'test hook (delete)')
 
     def install_hook_log_lines(self):
         """Install a hook that saves the get_lines for the this, base and other
         versions of the file.
         """
-        def hook_log_lines(merge_params):
-            self.hook_log.append((
-                'log_lines',
-                merge_params.this_lines,
-                merge_params.other_lines,
-                merge_params.base_lines,
-                ))
-            return 'not_applicable', None
+        test = self
+        class HookLogLines(_mod_merge.AbstractPerFileMerger):
+            def merge_contents(self, merge_params):
+                test.hook_log.append((
+                    'log_lines',
+                    merge_params.this_lines,
+                    merge_params.other_lines,
+                    merge_params.base_lines,
+                    ))
+                return 'not_applicable', None
+        def hook_log_lines_factory(merger):
+            return HookLogLines(merger)
         _mod_merge.Merger.hooks.install_named_hook(
-            'merge_file_content', hook_log_lines, 'test hook (log_lines)')
+            'merge_file_content', hook_log_lines_factory,
+            'test hook (log_lines)')
 
     def make_merge_builder(self):
         builder = MergeBuilder(self.test_base_dir)
@@ -315,6 +345,18 @@
         self.assertEqual(
             [('log_lines', ['text2'], ['text3'], ['text1'])], self.hook_log)
 
+    def test_chain_when_not_active(self):
+        """When a hook function returns None, merging still works."""
+        self.install_hook_inactive()
+        self.install_hook_success()
+        builder = self.make_merge_builder()
+        self.create_file_needing_contents_merge(builder, "1")
+        conflicts = builder.merge(self.merge_type)
+        self.assertEqual(conflicts, [])
+        self.assertEqual(
+            builder.this.get_file('1').read(), 'text-merged-by-hook')
+        self.assertEqual([('inactive',), ('success',)], self.hook_log)
+
     def test_chain_when_not_applicable(self):
         """When a hook function returns not_applicable, the next function is
         tried (when one exists).

=== modified file 'bzrlib/tests/test_merge.py'
--- a/bzrlib/tests/test_merge.py	2009-12-22 23:09:50 +0000
+++ b/bzrlib/tests/test_merge.py	2010-01-28 18:05:44 +0000
@@ -33,7 +33,11 @@
 from bzrlib.errors import UnrelatedBranches, NoCommits
 from bzrlib.merge import transform_tree, merge_inner, _PlanMerge
 from bzrlib.osutils import pathjoin, file_kind
-from bzrlib.tests import TestCaseWithTransport, TestCaseWithMemoryTransport
+from bzrlib.tests import (
+    TestCaseWithMemoryTransport,
+    TestCaseWithTransport,
+    test_merge_core,
+    )
 from bzrlib.workingtree import WorkingTree
 
 
@@ -2833,3 +2837,28 @@
             'bval', ['lca1val', 'lca2val', 'lca2val'], 'oval', 'tval')
         self.assertLCAMultiWay('conflict',
             'bval', ['lca1val', 'lca2val', 'lca3val'], 'oval', 'tval')
+
+
+class TestConfigurableFileMerger(tests.TestCaseWithTransport):
+
+    def test_affected_files_cached(self):
+        """Ensures that the config variable is cached"""
+        class SimplePlan(_mod_merge.ConfigurableFileMerger):
+            name_prefix = "foo"
+            default_files = ["my default"]
+            def merge_text(self, params):
+                return ('not applicable', None)
+        def factory(merger):
+            result = SimplePlan(merger)
+            self.assertEqual(None, result.affected_files)
+            self.merger = result
+            return result
+        _mod_merge.Merger.hooks.install_named_hook('merge_file_content',
+            factory, 'test factory')
+        builder = test_merge_core.MergeBuilder(self.test_base_dir)
+        self.addCleanup(builder.cleanup)
+        builder.add_file('NEWS', builder.tree_root, 'name1', 'text1', True)
+        builder.change_contents('NEWS', other='text4', this='text3')
+        conflicts = builder.merge()
+        # The hook should set the variable
+        self.assertEqual(["my default"], self.merger.affected_files)




More information about the bazaar-commits mailing list