Rev 4907: Finish the patch based on reviews. in file:///home/vila/src/bzr/reviews/per-file-merge-hook/

Vincent Ladeuil v.ladeuil+lp at free.fr
Wed Jan 20 16:05:28 GMT 2010


At file:///home/vila/src/bzr/reviews/per-file-merge-hook/

------------------------------------------------------------
revno: 4907
revision-id: v.ladeuil+lp at free.fr-20100120160528-5yo3fbggdp07eovc
parent: andrew.bennetts at canonical.com-20100118105921-io8o417isn20y5nl
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: per-file-merge-hook
timestamp: Wed 2010-01-20 17:05:28 +0100
message:
  Finish the patch based on reviews.
  
  * bzrlib/tests/per_merger.py:
  Fix line too long and spurious spaces.
  
  * bzrlib/plugins/news_merge/tests/test_news_merge.py:
  (TestFilenameMatchesConfig): Ensure that the params get updated.
  
  * bzrlib/plugins/news_merge/__init__.py:
  (filename_matches_config): Save the relevant config variable in
  the hook params.
  (install_hook): Wrap the hook installation so we can reuse it for
  tests.
  
  * bzrlib/plugins/news_merge/README: 
  Update the instructions by pointing to the plugin help.
  
  * bzrlib/merge.py:
  (MergeHookParams): Delete spurious spaces.
  
  * bzrlib/decorators.py:
  (use_pretty_decorators): Mention that we get clearance to copy
  launchpad code here (since canonical has copyrights on both code
  bases).
-------------- next part --------------
=== modified file 'bzrlib/decorators.py'
--- a/bzrlib/decorators.py	2010-01-18 10:57:35 +0000
+++ b/bzrlib/decorators.py	2010-01-20 16:05:28 +0000
@@ -256,7 +256,8 @@
 
 
 # This implementation of cachedproperty is copied from Launchpad's
-# canonical.launchpad.cachedproperty module.
+# canonical.launchpad.cachedproperty module (with permission from flacoste)
+# -- spiv & vila 100120
 def cachedproperty(attrname_or_fn):
     """A decorator for methods that makes them properties with their return
     value cached.

=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py	2010-01-18 07:58:34 +0000
+++ b/bzrlib/merge.py	2010-01-20 16:05:28 +0000
@@ -89,11 +89,11 @@
         self.this_kind = this_kind
         self.other_kind = other_kind
         self.winner = winner
-        
+
     def is_file_merge(self):
         """True if this_kind and other_kind are both 'file'."""
         return self.this_kind == 'file' and self.other_kind == 'file'
-    
+
     @decorators.cachedproperty
     def base_lines(self):
         """The lines of the 'base' version of the file."""

=== modified file 'bzrlib/plugins/news_merge/README'
--- a/bzrlib/plugins/news_merge/README	2010-01-13 07:27:57 +0000
+++ b/bzrlib/plugins/news_merge/README	2010-01-20 16:05:28 +0000
@@ -1,8 +1,6 @@
 A plugin for merging bzr's NEWS file.
 
-Install as usual (e.g. symlink this directory into ~/.bazaar/plugins), and any
-file with a path of NEWS that is in bzr's NEWS format will be merged with this
-hook.
+This plugin is activated via configuration variables, see 'bzr help news_merge'.
 
 This hook can resolve conflicts where both sides add entries at the same place.
 If it encounters a more difficult conflict it gives up and bzr will fallback to

=== modified file 'bzrlib/plugins/news_merge/__init__.py'
--- a/bzrlib/plugins/news_merge/__init__.py	2010-01-18 07:16:02 +0000
+++ b/bzrlib/plugins/news_merge/__init__.py	2010-01-20 16:05:28 +0000
@@ -32,6 +32,9 @@
   how to resolve that, so bzr will fallback to the default line-based merge.
 """
 
+# Since we are a built-in plugin we share the bzrlib version
+from bzrlib import version_info
+
 # Put most of the code in a separate module that we lazy-import to keep the
 # overhead of this plugin as minimal as possible.
 from bzrlib.lazy_import import lazy_import
@@ -59,15 +62,35 @@
 
 
 def filename_matches_config(params):
-    config = params.merger.this_branch.get_config()
-    affected_files = config.get_user_option_as_list('news_merge_files')
+    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
 
-
-Merger.hooks.install_named_hook(
-    'merge_file_content', news_merge_hook, 'NEWS file merge')
+def install_hook():
+    Merger.hooks.install_named_hook(
+        'merge_file_content', news_merge_hook, 'NEWS file merge')
+install_hook()
+
+
+def load_tests(basic_tests, module, loader):
+    testmod_names = [
+        'tests',
+        ]
+    basic_tests.addTest(loader.loadTestsFromModuleNames(
+            ["%s.%s" % (__name__, tmn) for tmn in testmod_names]))
+    return basic_tests
 

=== modified file 'bzrlib/plugins/news_merge/news_merge.py'
--- a/bzrlib/plugins/news_merge/news_merge.py	2010-01-18 07:20:16 +0000
+++ b/bzrlib/plugins/news_merge/news_merge.py	2010-01-20 16:05:28 +0000
@@ -85,4 +85,3 @@
 
 def sort_key(s):
     return s.replace('`', '').lower()
-    

=== added directory 'bzrlib/plugins/news_merge/tests'
=== added file 'bzrlib/plugins/news_merge/tests/__init__.py'
--- a/bzrlib/plugins/news_merge/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/plugins/news_merge/tests/__init__.py	2010-01-20 16:05:28 +0000
@@ -0,0 +1,23 @@
+# Copyright (C) 2010 by Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+def load_tests(basic_tests, module, loader):
+    testmod_names = [
+        'test_news_merge',
+        ]
+    basic_tests.addTest(loader.loadTestsFromModuleNames(
+            ["%s.%s" % (__name__, tmn) for tmn in testmod_names]))
+    return basic_tests

=== added file 'bzrlib/plugins/news_merge/tests/test_news_merge.py'
--- a/bzrlib/plugins/news_merge/tests/test_news_merge.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/plugins/news_merge/tests/test_news_merge.py	2010-01-20 16:05:28 +0000
@@ -0,0 +1,51 @@
+# Copyright (C) 2010 by Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# FIXME: This is totally incomplete but I'm only the patch pilot :-)
+# -- vila 100120
+
+from bzrlib import (
+    option,
+    tests,
+    )
+from bzrlib.plugins import 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	2009-12-22 06:37:26 +0000
+++ b/bzrlib/tests/per_merger.py	2010-01-20 16:05:28 +0000
@@ -142,7 +142,8 @@
         this_tree = branch.bzrdir.create_workingtree()
         this_tree.lock_write()
         self.addCleanup(this_tree.unlock)
-        other_tree = this_tree.bzrdir.sprout('other', 'OTHER-id').open_workingtree()
+        other_tree = this_tree.bzrdir.sprout('other',
+                                             'OTHER-id').open_workingtree()
         self.do_merge(this_tree, other_tree)
         if self.merge_type is _mod_merge.LCAMerger:
             self.expectFailure("lca merge doesn't track deleted lines",
@@ -174,7 +175,7 @@
         deletiondir = transform._deletiondir
         transform.finalize()
         return (limbodir, deletiondir)
-    
+
     def test_merge_with_existing_limbo(self):
         wt = self.make_branch_and_tree('this')
         (limbodir, deletiondir) =  self.get_limbodir_deletiondir(wt)
@@ -196,7 +197,7 @@
     def setUp(self):
         TestCaseWithTransport.setUp(self)
         self.hook_log = []
-        
+
     def install_hook_noop(self):
         def hook_na(merge_params):
             # This hook unconditionally does nothing.
@@ -213,7 +214,7 @@
             return 'not_applicable', None
         _mod_merge.Merger.hooks.install_named_hook(
             'merge_file_content', hook_success, 'test hook (success)')
-        
+
     def install_hook_conflict(self):
         def hook_conflict(merge_params):
             self.hook_log.append(('conflict',))
@@ -222,7 +223,7 @@
             return 'not_applicable', None
         _mod_merge.Merger.hooks.install_named_hook(
             'merge_file_content', hook_conflict, 'test hook (delete)')
-        
+
     def install_hook_delete(self):
         def hook_delete(merge_params):
             self.hook_log.append(('delete',))
@@ -231,7 +232,7 @@
             return 'not_applicable', None
         _mod_merge.Merger.hooks.install_named_hook(
             'merge_file_content', hook_delete, '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.
@@ -255,7 +256,7 @@
     def create_file_needing_contents_merge(self, builder, file_id):
         builder.add_file(file_id, builder.tree_root, "name1", "text1", True)
         builder.change_contents(file_id, other="text4", this="text3")
-        
+
     def test_change_vs_change(self):
         """Hook is used for (changed, changed)"""
         self.install_hook_success()



More information about the bazaar-commits mailing list