Rev 4803: Refactor NewsMerger into a reusable base class merge.ConfigurableFileMerger. in http://bazaar.launchpad.net/~lifeless/bzr/bug-513822
Robert Collins
robertc at robertcollins.net
Thu Jan 28 18:01:12 GMT 2010
At http://bazaar.launchpad.net/~lifeless/bzr/bug-513822
------------------------------------------------------------
revno: 4803
revision-id: robertc at robertcollins.net-20100128180103-kj106kj55vsbqxu1
parent: robertc at robertcollins.net-20100128172716-i6s7itz6flgxl66i
committer: Robert Collins <robertc at robertcollins.net>
branch nick: bug-513822
timestamp: Fri 2010-01-29 05:01:03 +1100
message:
Refactor NewsMerger into a reusable base class merge.ConfigurableFileMerger.
=== modified file 'NEWS'
--- a/NEWS 2010-01-28 17:27:16 +0000
+++ b/NEWS 2010-01-28 18:01:03 +0000
@@ -14,7 +14,9 @@
* 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.
+ 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-28 17:27:16 +0000
+++ b/bzrlib/merge.py 2010-01-28 18:01:03 +0000
@@ -93,6 +93,70 @@
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.
+ :ivar name_prefix: The prefix to use when looking up configuration
+ details.
+ :ivar default_files: The default file paths to merge when no configuration
+ is present.
+ """
+
+ def __init__(self, merger, name_prefix, default_files=None):
+ super(ConfigurableFileMerger, self).__init__(merger)
+ self.affected_files = None
+ self.default_files = default_files or []
+ self.name_prefix = name_prefix
+
+ 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.
=== modified file 'bzrlib/plugins/news_merge/news_merge.py'
--- a/bzrlib/plugins/news_merge/news_merge.py 2010-01-28 17:27:16 +0000
+++ b/bzrlib/plugins/news_merge/news_merge.py 2010-01-28 18:01:03 +0000
@@ -24,35 +24,19 @@
magic_marker = '|NEWS-MERGE-MAGIC-MARKER|'
-class NewsMerger(merge.AbstractPerFileMerger):
- """Merge bzr NEWS files.
-
- :ivar: affected_files.
- """
+class NewsMerger(merge.ConfigurableFileMerger):
+ """Merge bzr NEWS files."""
def __init__(self, merger):
- super(NewsMerger, self).__init__(merger)
- self.affected_files = None
+ super(NewsMerger, self).__init__(merger, 'news')
- def merge_contents(self, params):
+ 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.
"""
- # 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 self.filename_matches_config(params):
- # The filename isn't listed in the 'news_merge_files' config
- # option.
- return 'not_applicable', None
# 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.
@@ -91,26 +75,6 @@
# Transform the merged elements back into real blocks of lines.
return 'success', list(fakelines_to_blocks(result_lines))
- 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).
- 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 = []
- self.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 blocks_to_fakelines(blocks):
for kind, text in blocks:
=== modified file 'bzrlib/plugins/news_merge/tests/test_news_merge.py'
--- a/bzrlib/plugins/news_merge/tests/test_news_merge.py 2010-01-28 17:27:16 +0000
+++ b/bzrlib/plugins/news_merge/tests/test_news_merge.py 2010-01-28 18:01:03 +0000
@@ -16,6 +16,8 @@
# 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,
@@ -25,22 +27,3 @@
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"""
- def make_news_hook(merger):
- result = news_merge.news_merge.NewsMerger(merger)
- self.merger = result
- return result
- Merger.hooks.install_named_hook( 'merge_file_content', make_news_hook,
- 'test NEWS file merge')
- 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.merger.affected_files)
=== 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:01:03 +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,26 @@
'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):
+ def merge_text(self, params):
+ return ('not applicable', None)
+ def factory(merger):
+ result = SimplePlan(merger, "foo", ["my default"])
+ 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