[MERGE] FormatRegistry

John Arbash Meinel john at arbash-meinel.com
Wed Dec 20 22:35:20 GMT 2006


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

Aaron Bentley wrote:
> Hi all,
> 
> This bundle provides a registry for BzrDir formats and sub-formats.
> 
> A format would be BzrDirFormat6.  A subformat would be a particular
> metadir combination, like "BzrDirMetaFormat1 with RepositoryFormatKnit2".
> 
> This allows us to directly specify and use formats like 'weave',
> 'metaweave', 'knit', 'experimental-knit2', and others.  It allows
> plugins to register new formats.
> 
> It introduces a new help topic for "formats", and reimplements
> builtins.get_format_type.
> 
> Aaron


I'm happy to have a Registry, but I think this still needs a few tweaks
first.


...

+class BzrDirFormatRegistry(registry.Registry):
+    """Registry of user-selectable BzrDir subformats.
+
+    Differs from BzrDirFormat._control_formats in that it provides
sub-formats,
+    e.g. BzrDirMeta1 with weave repository.  Also, it's more user-oriented.
+    """
+
+    def register_metadir(self, key, repo, help, native=True,
deprecated=False):
+        """Register a metadir subformat.
+
+        repo is the repository format name as a string.
+        """
+        # This should be expanded to support setting WorkingTree and Branch
+        # formats, once BzrDirMetaFormat1 supports that.
+        def helper(key):
+            import bzrlib.repository
+            repo_format = getattr(bzrlib.repository, repo)
+            bd = BzrDirMetaFormat1()
+            bd.repository_format = repo_format()
+            return bd
+        self.register(key, helper, help, native, deprecated)

^- It seems very restrictive to force all repository formats to be in
'bzrlib.repository'. I think it would be good to give the python module
and path, since that lets us start to split things up.


...

v- Why explictly disable register_lazy? I could understand making a
different helper for it, but part of the point of Registry is so that
people don't *have* to write so many helpers, especially for lazy loading.

+    def register_lazy(self):
+        # prevent use of Registry.register_lazy
+        # lazy-loading can be done by registering a suitable factory.
+        raise NotImplementedError(self.register_lazy)
+

v- Is there a reason you chose to use the key 'default', rather than
just using Registry.default = 'knit' ?

It does change how you would do get(), since you would do:
registry.get(None) (or registry.get()).

Perhaps for compatibility it was easier to do registry.get('default').

But you could also easily do:
if key == 'default':
  key = None


+    def set_default(self, key):
+        """Set the 'default' key to be a clone of the supplied key.
+
+        This method must be called once and only once.
+        """
+        registry.Registry.register(self, 'default', self.get(key),
+            self.get_help(key), info=self.get_info(key))
+
+    def make_bzrdir(self, key):
+        return self.get(key)(key)
+
+    def help_topic(self, topic):
+        output = textwrap.dedent("""\
+            Bazaar directory formats
+            ------------------------
+
+            These formats can be used for creating branches, working
trees, and
+            repositories.
+
+            """)
+        default_help = self.get_help('default')
+        help_pairs = []
+        for key in self.keys():
+            if key == 'default':
+                continue
+            help = self.get_help(key)
+            if help == default_help:
+                default_realkey = key
+            else:
+                help_pairs.append((key, help))
+
+        def wrapped(key, help, info):
+            if info.native:
+                help = '(native) ' + help
+            return '  %s:\n%s\n\n' % (key,
+                    textwrap.fill(help, initial_indent='    ',
+                    subsequent_indent='    '))
+        output += wrapped('%s/default' % default_realkey, default_help,
+                          self.get_info('default'))
+        deprecated_pairs = []
+        for key, help in help_pairs:
+            info = self.get_info(key)
+            if info.deprecated:
+                deprecated_pairs.append((key, help))
+            else:
+                output += wrapped(key, help, info)
+        if len(deprecated_pairs) > 0:
+            output += "Deprecated formats\n------------------\n\n"
+            for key, help in deprecated_pairs:
+                info = self.get_info(key)
+                output += wrapped(key, help, info)
+
+        return output
+
+
+format_registry = BzrDirFormatRegistry()
+format_registry.register_factory('weave', BzrDirFormat6,
+    'Pre-0.8 format.  Slower than knit and does not'
+    ' support checkouts or shared repositories.', deprecated=True)
+format_registry.register_metadir('knit', 'RepositoryFormatKnit1',
+    'Format using knits.  Recommended.')
+format_registry.set_default('knit')
+format_registry.register_metadir('metaweave', 'RepositoryFormat7',
+    'Transitional format in 0.8.  Slower than knit.',
+    deprecated=True)
+format_registry.register_metadir('experimental-knit2',
'RepositoryFormatKnit2',
+    'Experimental successor to knit.  Use at your own risk.')

=== modified file bzrlib/help_topics.py //
last-changed:abentley at panoramicfeedb
... ack.com-20061220163858-zwom59l3xvcqu2yd
- --- bzrlib/help_topics.py
+++ bzrlib/help_topics.py
@@ -132,3 +132,7 @@
                         "Explain how to use --revision")
 topic_registry.register('basic', _basic_help, "Basic commands")
 topic_registry.register('topics', _help_on_topics, "Topics list")
+def get_format_topic(topic):
+    import bzrdir
+    return bzrdir.format_registry.help_topic(topic)
+topic_registry.register('formats', get_format_topic, 'directory formats')

^- Like here, instead of creating a wrapper which has to import
something, you could just do:

topic_registry.register_lazy('formats', 'bzrlib.bzrdir',
			     'format_registry.help_topic',
			      'directory formats')

Though since you are nested 2 deep, that might need:

=== modified file 'bzrlib/registry.py'
- --- bzrlib/registry.py  2006-10-16 01:50:48 +0000
+++ bzrlib/registry.py  2006-12-20 22:31:31 +0000
@@ -65,7 +65,8 @@
         obj = __import__(self._module_name, globals(), locals(),
                          [self._member_name])
         if self._member_name:
- -            obj = getattr(obj, self._member_name)
+            for piece in self._member_name.split('.'):
+                obj = getattr(obj, piece)
         self._obj = obj
         self._imported = True


At the very least, though, you should be doing an absolute import. Either:

 from bzrlib import bzrdir

or
 import bzrlib.bzrdir


v-  You should be using 'assertIsInstance()' rather than
assertTrue(isinstance)

+
+    def test_format_registry(self):
+        my_format_registry = self.make_format_registry()
+        my_bzrdir = my_format_registry.make_bzrdir('direct')
+        self.assertTrue(isinstance(my_bzrdir, bzrdir.BzrDirFormat6))
+        my_bzrdir = my_format_registry.make_bzrdir('weave')
+        self.assertTrue(isinstance(my_bzrdir, bzrdir.BzrDirFormat6))
+        my_bzrdir = my_format_registry.make_bzrdir('default')
+        self.assertTrue(isinstance(my_bzrdir.repository_format,
+            repository.RepositoryFormatKnit1))
+        my_bzrdir = my_format_registry.make_bzrdir('knit')
+        self.assertTrue(isinstance(my_bzrdir.repository_format,
+            repository.RepositoryFormatKnit1))
+        my_bzrdir = my_format_registry.make_bzrdir('metaweave')
+        self.assertTrue(isinstance(my_bzrdir.repository_format,
+            repository.RepositoryFormat7))
+
+    def test_lazy(self):
+        my_format_registry = bzrdir.BzrDirFormatRegistry()
+        self.assertRaises(NotImplementedError,
+                          my_format_registry.register_lazy)
+

Otherwise, I think it looks pretty good. Though I might prefer it if we
switched the actual BzrDir._control_formats list, rather than having
*another* registry just for a help topic.

It seems like having 2 lists is going to make it easy for them to get
out of sync.

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

iD8DBQFFibqoJdeBCYSNAAMRApssAKChTQwCuepnNl4mkNyhQ44Frjm9dQCfZpmr
PbksVIFfPEpy88iJ8pWR2Dg=
=o6hD
-----END PGP SIGNATURE-----




More information about the bazaar mailing list