Bazaar

Merge lp:~lifeless/bzr/loomsupport into lp:bzr

  1. loomsupport
  2. Merge into bzr.dev
Proposed by Robert Collins
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 5308
Proposed branch: lp:~lifeless/bzr/loomsupport
Merge into: lp:bzr
Diff against target: 208 lines (+124/-5)
4 files modified
NEWS (+6/-0)
bzrlib/branch.py (+57/-4)
bzrlib/tests/per_branch/__init__.py (+1/-1)
bzrlib/tests/test_branch.py (+60/-0)
To merge this branch: bzr merge lp:~lifeless/bzr/loomsupport
Reviewer Review Type Date Requested Status
John A Meinel Needs Fixing
Review via email: mp+27903@code.launchpad.net

Commit message

A lazy-load support glue for Branch formats.

Description of the change

This is an alternative implementation of lazy format objects. It seemed
simpler to me than another custom registry subclass (needed to handle the
network name stuff properly), and this permits slightly cleaner backwards
compatability code in loom than otherwise.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> Robert Collins has proposed merging lp:~lifeless/bzr/loomsupport into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This is an alternative implementation of lazy format objects. It seemed
> simpler to me than another custom registry subclass (needed to handle the
> network name stuff properly), and this permits slightly cleaner backwards
> compatability code in loom than otherwise.
>

+ # registered as factories.
+ if isinstance(format, MetaDirBranchFormatFactory):
+
network_format_registry.register(format.get_format_string(), format)
+ else:
+ network_format_registry.register(format.get_format_string(),
+ format.__class__)

^- This looks a bit odd.

I believe I understand why you are doing it, but it really makes me wonder.

Why are we passing in instances if what we want is factories...

I think if we add this, we should register at least a couple formats
lazily, so that people have examples to pick from.

Right now, the only example is in the test suite, and it is particularly
bad, because it uses "Class().get_format_string()" which certainly isn't
how you would use it in production. (If you had an instance of the
class, you would just register it directly...)

 review: needsfixing
 merge: approve

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

iEYEARECAAYFAkwbgMoACgkQJdeBCYSNAAN1qgCgm1mof+1ZMB20NZu+b+jTDpHn
D2QAniOCiIaL9OZ+XR0X7zWyv/ISo5sw
=k8Nv
-----END PGP SIGNATURE-----

review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

loom uses it - could I point at the loom use as a reference?

I mean, I'd love to move the old branch formats to be lazy too, but
that seems non trivial.

Revision history for this message
Robert Collins (lifeless) wrote :

As there wasn't any followup to my query, I'm going to land this pretty much as is. I'll change the test suite example to use a constant string, and add a pointer to loom.

Revision history for this message
Robert Collins (lifeless) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-06-18 11:57:13 +0000
+++ NEWS 2010-06-20 21:19:24 +0000
@@ -92,6 +92,12 @@
92 plugins to intercept this even when a ``RemoteBranch`` proxy is in use.92 plugins to intercept this even when a ``RemoteBranch`` proxy is in use.
93 (Robert Collins, #201613)93 (Robert Collins, #201613)
9494
95* ``Branch`` formats can now be loaded lazily by registering a
96 ``MetaDirBranchFormatFactory`` rather than an actual format. This will
97 cause the named format class to be loaded only when an enumeration of
98 formats is needed or when the format string for the object is
99 encountered. (Robert Collins, Jelmer Vernooij)
100
95* Use lazy imports in ``bzrlib/merge.py`` so that plugins like ``news_merge``101* Use lazy imports in ``bzrlib/merge.py`` so that plugins like ``news_merge``
96 do not cause modules to be loaded unnecessarily just because the plugin102 do not cause modules to be loaded unnecessarily just because the plugin
97 registers a merge hook. This improves ``bzr rocks`` time by about 25%103 registers a merge hook. This improves ``bzr rocks`` time by about 25%
98104
=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py 2010-06-17 06:30:22 +0000
+++ bzrlib/branch.py 2010-06-20 21:19:24 +0000
@@ -1518,7 +1518,10 @@
1518 try:1518 try:
1519 transport = a_bzrdir.get_branch_transport(None, name=name)1519 transport = a_bzrdir.get_branch_transport(None, name=name)
1520 format_string = transport.get_bytes("format")1520 format_string = transport.get_bytes("format")
1521 return klass._formats[format_string]1521 format = klass._formats[format_string]
1522 if isinstance(format, MetaDirBranchFormatFactory):
1523 return format()
1524 return format
1522 except errors.NoSuchFile:1525 except errors.NoSuchFile:
1523 raise errors.NotBranchError(path=transport.base, bzrdir=a_bzrdir)1526 raise errors.NotBranchError(path=transport.base, bzrdir=a_bzrdir)
1524 except KeyError:1527 except KeyError:
@@ -1529,6 +1532,20 @@
1529 """Return the current default format."""1532 """Return the current default format."""
1530 return klass._default_format1533 return klass._default_format
15311534
1535 @classmethod
1536 def get_formats(klass):
1537 """Get all the known formats.
1538
1539 Warning: This triggers a load of all lazy registered formats: do not
1540 use except when that is desireed.
1541 """
1542 result = []
1543 for fmt in klass._formats.values():
1544 if isinstance(fmt, MetaDirBranchFormatFactory):
1545 fmt = fmt()
1546 result.append(fmt)
1547 return result
1548
1532 def get_reference(self, a_bzrdir, name=None):1549 def get_reference(self, a_bzrdir, name=None):
1533 """Get the target reference of the branch in a_bzrdir.1550 """Get the target reference of the branch in a_bzrdir.
15341551
@@ -1671,11 +1688,19 @@
16711688
1672 @classmethod1689 @classmethod
1673 def register_format(klass, format):1690 def register_format(klass, format):
1674 """Register a metadir format."""1691 """Register a metadir format.
1692
1693 See MetaDirBranchFormatFactory for the ability to register a format
1694 without loading the code the format needs until it is actually used.
1695 """
1675 klass._formats[format.get_format_string()] = format1696 klass._formats[format.get_format_string()] = format
1676 # Metadir formats have a network name of their format string, and get1697 # Metadir formats have a network name of their format string, and get
1677 # registered as class factories.1698 # registered as factories.
1678 network_format_registry.register(format.get_format_string(), format.__class__)1699 if isinstance(format, MetaDirBranchFormatFactory):
1700 network_format_registry.register(format.get_format_string(), format)
1701 else:
1702 network_format_registry.register(format.get_format_string(),
1703 format.__class__)
16791704
1680 @classmethod1705 @classmethod
1681 def set_default_format(klass, format):1706 def set_default_format(klass, format):
@@ -1701,6 +1726,34 @@
1701 return False # by default1726 return False # by default
17021727
17031728
1729class MetaDirBranchFormatFactory(registry._LazyObjectGetter):
1730 """A factory for a BranchFormat object, permitting simple lazy registration.
1731
1732 While none of the built in BranchFormats are lazy registered yet,
1733 bzrlib.tests.test_branch.TestMetaDirBranchFormatFactory demonstrates how to
1734 use it, and the bzr-loom plugin uses it as well (see
1735 bzrlib.plugins.loom.formats).
1736 """
1737
1738 def __init__(self, format_string, module_name, member_name):
1739 """Create a MetaDirBranchFormatFactory.
1740
1741 :param format_string: The format string the format has.
1742 :param module_name: Module to load the format class from.
1743 :param member_name: Attribute name within the module for the format class.
1744 """
1745 registry._LazyObjectGetter.__init__(self, module_name, member_name)
1746 self._format_string = format_string
1747
1748 def get_format_string(self):
1749 """See BranchFormat.get_format_string."""
1750 return self._format_string
1751
1752 def __call__(self):
1753 """Used for network_format_registry support."""
1754 return self.get_obj()()
1755
1756
1704class BranchHooks(Hooks):1757class BranchHooks(Hooks):
1705 """A dictionary mapping hook name to a list of callables for branch hooks.1758 """A dictionary mapping hook name to a list of callables for branch hooks.
17061759
17071760
=== modified file 'bzrlib/tests/per_branch/__init__.py'
--- bzrlib/tests/per_branch/__init__.py 2010-05-13 15:14:41 +0000
+++ bzrlib/tests/per_branch/__init__.py 2010-06-20 21:19:24 +0000
@@ -132,7 +132,7 @@
132 # Generate a list of branch formats and their associated bzrdir formats to132 # Generate a list of branch formats and their associated bzrdir formats to
133 # use.133 # use.
134 combinations = [(format, format._matchingbzrdir) for format in134 combinations = [(format, format._matchingbzrdir) for format in
135 BranchFormat._formats.values() + _legacy_formats]135 BranchFormat.get_formats() + _legacy_formats]
136 scenarios = make_scenarios(136 scenarios = make_scenarios(
137 # None here will cause the default vfs transport server to be used.137 # None here will cause the default vfs transport server to be used.
138 None,138 None,
139139
=== modified file 'bzrlib/tests/test_branch.py'
--- bzrlib/tests/test_branch.py 2010-04-23 07:15:23 +0000
+++ bzrlib/tests/test_branch.py 2010-06-20 21:19:24 +0000
@@ -136,6 +136,27 @@
136 return "opened branch."136 return "opened branch."
137137
138138
139# Demonstrating how lazy loading is often implemented:
140# A constant string is created.
141SampleSupportedBranchFormatString = "Sample supported branch format."
142
143# And the format class can then reference the constant to avoid skew.
144class SampleSupportedBranchFormat(_mod_branch.BranchFormat):
145 """A sample supported format."""
146
147 def get_format_string(self):
148 """See BzrBranchFormat.get_format_string()."""
149 return SampleSupportedBranchFormatString
150
151 def initialize(self, a_bzrdir, name=None):
152 t = a_bzrdir.get_branch_transport(self, name=name)
153 t.put_bytes('format', self.get_format_string())
154 return 'A branch'
155
156 def open(self, transport, name=None, _found=False, ignore_fallbacks=False):
157 return "opened supported branch."
158
159
139class TestBzrBranchFormat(tests.TestCaseWithTransport):160class TestBzrBranchFormat(tests.TestCaseWithTransport):
140 """Tests for the BzrBranchFormat facility."""161 """Tests for the BzrBranchFormat facility."""
141162
@@ -152,6 +173,17 @@
152 self.failUnless(isinstance(found_format, format.__class__))173 self.failUnless(isinstance(found_format, format.__class__))
153 check_format(_mod_branch.BzrBranchFormat5(), "bar")174 check_format(_mod_branch.BzrBranchFormat5(), "bar")
154175
176 def test_find_format_factory(self):
177 dir = bzrdir.BzrDirMetaFormat1().initialize(self.get_url())
178 SampleSupportedBranchFormat().initialize(dir)
179 factory = _mod_branch.MetaDirBranchFormatFactory(
180 SampleSupportedBranchFormatString,
181 "bzrlib.tests.test_branch", "SampleSupportedBranchFormat")
182 _mod_branch.BranchFormat.register_format(factory)
183 self.addCleanup(_mod_branch.BranchFormat.unregister_format, factory)
184 b = _mod_branch.Branch.open(self.get_url())
185 self.assertEqual(b, "opened supported branch.")
186
155 def test_find_format_not_branch(self):187 def test_find_format_not_branch(self):
156 dir = bzrdir.BzrDirMetaFormat1().initialize(self.get_url())188 dir = bzrdir.BzrDirMetaFormat1().initialize(self.get_url())
157 self.assertRaises(errors.NotBranchError,189 self.assertRaises(errors.NotBranchError,
@@ -186,6 +218,34 @@
186 self.make_branch_and_tree('bar')218 self.make_branch_and_tree('bar')
187219
188220
221#Used by TestMetaDirBranchFormatFactory
222FakeLazyFormat = None
223
224
225class TestMetaDirBranchFormatFactory(tests.TestCase):
226
227 def test_get_format_string_does_not_load(self):
228 """Formats have a static format string."""
229 factory = _mod_branch.MetaDirBranchFormatFactory("yo", None, None)
230 self.assertEqual("yo", factory.get_format_string())
231
232 def test_call_loads(self):
233 # __call__ is used by the network_format_registry interface to get a
234 # Format.
235 global FakeLazyFormat
236 del FakeLazyFormat
237 factory = _mod_branch.MetaDirBranchFormatFactory(None,
238 "bzrlib.tests.test_branch", "FakeLazyFormat")
239 self.assertRaises(AttributeError, factory)
240
241 def test_call_returns_call_of_referenced_object(self):
242 global FakeLazyFormat
243 FakeLazyFormat = lambda:'called'
244 factory = _mod_branch.MetaDirBranchFormatFactory(None,
245 "bzrlib.tests.test_branch", "FakeLazyFormat")
246 self.assertEqual('called', factory())
247
248
189class TestBranch67(object):249class TestBranch67(object):
190 """Common tests for both branch 6 and 7 which are mostly the same."""250 """Common tests for both branch 6 and 7 which are mostly the same."""
191251