Merge lp:~jelmer/bzr/lazy-bzrdir into lp:bzr

[Jelmer Vernooij]

Make it possible to lazily register BzrDir and ControlDir formats.

This is another prerequisite for the weave_fmt plugin branch. It also allows foreign formats to do more lazy loading - currently they always have to import their ControlDirFormat subclass at
startup time to register it.

Does the way in which this allows unregistration of lazy control dir formats look reasonable?
I realize using:

list.remove(_ObjectGetter(obj))

and relying on the __eq__ implementation in _ObjectGetter is a bit weird, but I can't think of a better alternative. Better ideas welcome :)

[John A Meinel (jameinel)]

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

On 03/10/2011 02:30 PM, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/lazy-bzrdir into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~jelmer/bzr/lazy-bzrdir/+merge/52844
>
> Make it possible to lazily register BzrDir and ControlDir formats.
>
> This is another prerequisite for the weave_fmt plugin branch. It also allows foreign formats to do more lazy loading - currently they always have to import their ControlDirFormat subclass at
> startup time to register it.
>
> Does the way in which this allows unregistration of lazy control dir formats look reasonable?
> I realize using:
>
> list.remove(_ObjectGetter(obj))
>
> and relying on the __eq__ implementation in _ObjectGetter is a bit weird, but I can't think of a better alternative. Better ideas welcome :)

I don't think we want a separate unregister_lazy_format, because at some
point, the format won't be lazy anymore. I'd rather have the
registration take some sort of 'key' that it can then use.

I'm guessing the regular "register_format" actually uses some attribute
of the format to determine if it is what it wants. Why not pass that in
to the "register_lazy_format" so that:

1) unregistering is more straightforward
2) we could skip loading it entirely if it doesn't match the key, even
if we are probing and think it 'might' fit.

John
=:->

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

iEYEARECAAYFAk141V8ACgkQJdeBCYSNAAPuaQCgyY8q8owLfo+QyDKrjZjm7diD
rqkAoJpbD9no3Ij7C2p483gVPWBVud1u
=cznf
-----END PGP SIGNATURE-----

[Jelmer Vernooij (jelmer)]

On Thu, 2011-03-10 at 13:55 +0000, John A Meinel wrote:
> On 03/10/2011 02:30 PM, Jelmer Vernooij wrote:
> > Jelmer Vernooij has proposed merging lp:~jelmer/bzr/lazy-bzrdir into lp:bzr.
> >
> > Requested reviews:
> > bzr-core (bzr-core)
> >
> > For more details, see:
> > https://code.launchpad.net/~jelmer/bzr/lazy-bzrdir/+merge/52844
> >
> > Make it possible to lazily register BzrDir and ControlDir formats.
> >
> > This is another prerequisite for the weave_fmt plugin branch. It also allows foreign formats to do more lazy loading - currently they always have to import their ControlDirFormat subclass at
> > startup time to register it.
> >
> > Does the way in which this allows unregistration of lazy control dir formats look reasonable?
> > I realize using:
> >
> > list.remove(_ObjectGetter(obj))
> >
> > and relying on the __eq__ implementation in _ObjectGetter is a bit weird, but I can't think of a better alternative. Better ideas welcome :)
>
> I don't think we want a separate unregister_lazy_format, because at some
> point, the format won't be lazy anymore. I'd rather have the
> registration take some sort of 'key' that it can then use.
>
> I'm guessing the regular "register_format" actually uses some attribute
> of the format to determine if it is what it wants. Why not pass that in
> to the "register_lazy_format" so that:
Thanks, that makes more sense indeed. I've updated the branch, will resubmit.

> 1) unregistering is more straightforward
> 2) we could skip loading it entirely if it doesn't match the key, even
> if we are probing and think it 'might' fit.
The regular register_format() also just works with ControlDirFormat
objects, not necessarily ones with a format string (hg, git, etc don't
have one).

That said, BzrDir.register_lazy_format() / BzrDir.register_format() can
just always pass in format.get_format_string() and foreign formats can
just specify an arbitrary string that's not used anywhere else.

Cheers,

Jelmer

[Vincent Ladeuil (vila)]

As discussed on IRC, this looks simpler and I like it better.

To summarize:

- add comments about using a set() for known_formats() and a couple of basic tests (using a set() also means we lose the registration order. I don't think there are cases where it should matter but it still good to test/document it explicitly,

- since you'll update bzr-{svn|hg|git}, I think more than 90% users of the previous API are covered, but there will still be users with installations where the old plugins will try (and fail) to use the old API in a newer bzr. They should at least get an meaningful error message telling them to upgrade.

With these tweaks in place, I'm ok for you to land it, but you may want to summarize the issues on the ml or get a second review.

review: Needs Fixing

[Jelmer Vernooij (jelmer)]

On Fri, 2011-03-11 at 07:35 +0000, Vincent Ladeuil wrote:
> The proposal to merge lp:~jelmer/bzr/lazy-bzrdir into lp:bzr has been updated.
>
> Status: Needs review => Approved
>
> For more details, see:
> https://code.launchpad.net/~jelmer/bzr/lazy-bzrdir/+merge/52844
I'll probably ask for another review. It turns out that the control dir
tests weren't being run against RemoteBzrDirFormat previously, and now
that they are there are a few test failures. I'm not entirely sure my
fixes are correct.

Cheers,

Jelmer

[Vincent Ladeuil (vila)]

> It turns out that the control dir tests weren't being run against RemoteBzrDirFormat previously, and now that they are there are a few test failures.

:-/

Marking as wip then.

=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2011-03-08 17:40:26 +0000
+++ bzrlib/bzrdir.py	2011-03-11 14:21:26 +0000
@@ -59,7 +59,6 @@
     xml5,
     )
 from bzrlib.repofmt import pack_repo
-from bzrlib.smart.client import _SmartClient
 from bzrlib.store.versioned import VersionedFileStore
 from bzrlib.transactions import WriteTransaction
 from bzrlib.transport import (
@@ -80,6 +79,7 @@
 
 from bzrlib import (
     hooks,
+    registry,
     )
 from bzrlib.symbol_versioning import (
     deprecated_in,
@@ -1473,16 +1473,18 @@
 class BzrProber(controldir.Prober):
     """Prober for formats that use a .bzr/ control directory."""
 
-    _formats = {}
+    formats = registry.FormatRegistry(controldir.network_format_registry)
     """The known .bzr formats."""
 
     @classmethod
+    @deprecated_method(deprecated_in((2, 4, 0)))
     def register_bzrdir_format(klass, format):
-        klass._formats[format.get_format_string()] = format
+        klass.formats.register(format.get_format_string(), format)
 
     @classmethod
+    @deprecated_method(deprecated_in((2, 4, 0)))
     def unregister_bzrdir_format(klass, format):
-        del klass._formats[format.get_format_string()]
+        klass.formats.remove(format.get_format_string())
 
     @classmethod
     def probe_transport(klass, transport):
@@ -1492,10 +1494,17 @@
         except errors.NoSuchFile:
             raise errors.NotBranchError(path=transport.base)
         try:
-            return klass._formats[format_string]
+            return klass.formats.get(format_string)
         except KeyError:
             raise errors.UnknownFormatError(format=format_string, kind='bzrdir')
 
+    @classmethod
+    def known_formats(cls):
+        result = set()
+        for name, format in cls.formats.iteritems():
+            result.add(format)
+        return result
+
 
 controldir.ControlDirFormat.register_prober(BzrProber)
 
@@ -1525,8 +1534,14 @@
                     raise errors.NotBranchError(path=transport.base)
                 if server_version != '2':
                     raise errors.NotBranchError(path=transport.base)
+            from bzrlib.remote import RemoteBzrDirFormat
             return RemoteBzrDirFormat()
 
+    @classmethod
+    def known_formats(cls):
+        from bzrlib.remote import RemoteBzrDirFormat
+        return set([RemoteBzrDirFormat()])
+
 
 class BzrDirFormat(controldir.ControlDirFormat):
     """ControlDirFormat base class for .bzr/ directories.
@@ -1545,7 +1560,8 @@
     # _lock_class must be set in subclasses to the lock type, typ.
     # TransportLock or LockDir
 
-    def get_format_string(self):
+    @classmethod
+    def get_format_string(cls):
         """Return the ASCII format string that identifies this format."""
         raise NotImplementedError(self.get_format_string)
 
@@ -1563,6 +1579,7 @@
             # metadir1
             if type(self) != BzrDirMetaFormat1:
                 return self._initialize_on_transport_vfs(transport)
+            from bzrlib.remote import RemoteBzrDirFormat
             remote_format = RemoteBzrDirFormat()
             self._supply_sub_formats_to(remote_format)
             return remote_format.initialize_on_transport(transport)
@@ -1606,6 +1623,7 @@
             except errors.NoSmartMedium:
                 pass
             else:
+                from bzrlib.remote import RemoteBzrDirFormat
                 # TODO: lookup the local format from a server hint.
                 remote_dir_format = RemoteBzrDirFormat()
                 remote_dir_format._network_name = self.network_name()
@@ -1726,13 +1744,6 @@
         """
         raise NotImplementedError(self._open)
 
-    @classmethod
-    def register_format(klass, format):
-        BzrProber.register_bzrdir_format(format)
-        # bzr native formats have a network name of their format string.
-        controldir.network_format_registry.register(format.get_format_string(), format.__class__)
-        controldir.ControlDirFormat.register_format(format)
-
     def _supply_sub_formats_to(self, other_format):
         """Give other_format the same values for sub formats as this has.
 
@@ -1745,12 +1756,6 @@
         :return: None.
         """
 
-    @classmethod
-    def unregister_format(klass, format):
-        BzrProber.unregister_bzrdir_format(format)
-        controldir.ControlDirFormat.unregister_format(format)
-        controldir.network_format_registry.remove(format.get_format_string())
-
 
 class BzrDirFormat4(BzrDirFormat):
     """Bzr dir format 4.
@@ -1769,7 +1774,8 @@
 
     fixed_components = True
 
-    def get_format_string(self):
+    @classmethod
+    def get_format_string(cls):
         """See BzrDirFormat.get_format_string()."""
         return "Bazaar-NG branch, format 0.0.4\n"
 
@@ -1849,7 +1855,8 @@
 
     _lock_class = lockable_files.TransportLock
 
-    def get_format_string(self):
+    @classmethod
+    def get_format_string(cls):
         """See BzrDirFormat.get_format_string()."""
         return "Bazaar-NG branch, format 5\n"
 
@@ -1910,7 +1917,8 @@
 
     _lock_class = lockable_files.TransportLock
 
-    def get_format_string(self):
+    @classmethod
+    def get_format_string(cls):
         """See BzrDirFormat.get_format_string()."""
         return "Bazaar-NG branch, format 6\n"
 
@@ -2102,7 +2110,8 @@
             raise NotImplementedError(self.get_converter)
         return ConvertMetaToMeta(format)
 
-    def get_format_string(self):
+    @classmethod
+    def get_format_string(cls):
         """See BzrDirFormat.get_format_string()."""
         return "Bazaar-NG meta directory, format 1\n"
 
@@ -2170,11 +2179,11 @@
 
 
 # Register bzr formats
-BzrDirFormat.register_format(BzrDirFormat4())
-BzrDirFormat.register_format(BzrDirFormat5())
-BzrDirFormat.register_format(BzrDirFormat6())
+BzrProber.formats.register(BzrDirFormat4.get_format_string(), BzrDirFormat4())
+BzrProber.formats.register(BzrDirFormat5.get_format_string(), BzrDirFormat5())
+BzrProber.formats.register(BzrDirFormat6.get_format_string(), BzrDirFormat6())
 __default_format = BzrDirMetaFormat1()
-BzrDirFormat.register_format(__default_format)
+BzrProber.formats.register(__default_format.get_format_string(), __default_format)
 controldir.ControlDirFormat._default_format = __default_format
 
 
@@ -2695,246 +2704,6 @@
         return to_convert
 
 
-# This is not in remote.py because it's relatively small, and needs to be
-# registered. Putting it in remote.py creates a circular import problem.
-# we can make it a lazy object if the control formats is turned into something
-# like a registry.
-class RemoteBzrDirFormat(BzrDirMetaFormat1):
-    """Format representing bzrdirs accessed via a smart server"""
-
-    supports_workingtrees = False
-
-    def __init__(self):
-        BzrDirMetaFormat1.__init__(self)
-        # XXX: It's a bit ugly that the network name is here, because we'd
-        # like to believe that format objects are stateless or at least
-        # immutable,  However, we do at least avoid mutating the name after
-        # it's returned.  See <https://bugs.launchpad.net/bzr/+bug/504102>
-        self._network_name = None
-
-    def __repr__(self):
-        return "%s(_network_name=%r)" % (self.__class__.__name__,
-            self._network_name)
-
-    def get_format_description(self):
-        if self._network_name:
-            real_format = controldir.network_format_registry.get(self._network_name)
-            return 'Remote: ' + real_format.get_format_description()
-        return 'bzr remote bzrdir'
-
-    def get_format_string(self):
-        raise NotImplementedError(self.get_format_string)
-
-    def network_name(self):
-        if self._network_name:
-            return self._network_name
-        else:
-            raise AssertionError("No network name set.")
-
-    def initialize_on_transport(self, transport):
-        try:
-            # hand off the request to the smart server
-            client_medium = transport.get_smart_medium()
-        except errors.NoSmartMedium:
-            # TODO: lookup the local format from a server hint.
-            local_dir_format = BzrDirMetaFormat1()
-            return local_dir_format.initialize_on_transport(transport)
-        client = _SmartClient(client_medium)
-        path = client.remote_path_from_transport(transport)
-        try:
-            response = client.call('BzrDirFormat.initialize', path)
-        except errors.ErrorFromSmartServer, err:
-            remote._translate_error(err, path=path)
-        if response[0] != 'ok':
-            raise errors.SmartProtocolError('unexpected response code %s' % (response,))
-        format = RemoteBzrDirFormat()
-        self._supply_sub_formats_to(format)
-        return remote.RemoteBzrDir(transport, format)
-
-    def parse_NoneTrueFalse(self, arg):
-        if not arg:
-            return None
-        if arg == 'False':
-            return False
-        if arg == 'True':
-            return True
-        raise AssertionError("invalid arg %r" % arg)
-
-    def _serialize_NoneTrueFalse(self, arg):
-        if arg is False:
-            return 'False'
-        if arg:
-            return 'True'
-        return ''
-
-    def _serialize_NoneString(self, arg):
-        return arg or ''
-
-    def initialize_on_transport_ex(self, transport, use_existing_dir=False,
-        create_prefix=False, force_new_repo=False, stacked_on=None,
-        stack_on_pwd=None, repo_format_name=None, make_working_trees=None,
-        shared_repo=False):
-        try:
-            # hand off the request to the smart server
-            client_medium = transport.get_smart_medium()
-        except errors.NoSmartMedium:
-            do_vfs = True
-        else:
-            # Decline to open it if the server doesn't support our required
-            # version (3) so that the VFS-based transport will do it.
-            if client_medium.should_probe():
-                try:
-                    server_version = client_medium.protocol_version()
-                    if server_version != '2':
-                        do_vfs = True
-                    else:
-                        do_vfs = False
-                except errors.SmartProtocolError:
-                    # Apparently there's no usable smart server there, even though
-                    # the medium supports the smart protocol.
-                    do_vfs = True
-            else:
-                do_vfs = False
-        if not do_vfs:
-            client = _SmartClient(client_medium)
-            path = client.remote_path_from_transport(transport)
-            if client_medium._is_remote_before((1, 16)):
-                do_vfs = True
-        if do_vfs:
-            # TODO: lookup the local format from a server hint.
-            local_dir_format = BzrDirMetaFormat1()
-            self._supply_sub_formats_to(local_dir_format)
-            return local_dir_format.initialize_on_transport_ex(transport,
-                use_existing_dir=use_existing_dir, create_prefix=create_prefix,
-                force_new_repo=force_new_repo, stacked_on=stacked_on,
-                stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
-                make_working_trees=make_working_trees, shared_repo=shared_repo,
-                vfs_only=True)
-        return self._initialize_on_transport_ex_rpc(client, path, transport,
-            use_existing_dir, create_prefix, force_new_repo, stacked_on,
-            stack_on_pwd, repo_format_name, make_working_trees, shared_repo)
-
-    def _initialize_on_transport_ex_rpc(self, client, path, transport,
-        use_existing_dir, create_prefix, force_new_repo, stacked_on,
-        stack_on_pwd, repo_format_name, make_working_trees, shared_repo):
-        args = []
-        args.append(self._serialize_NoneTrueFalse(use_existing_dir))
-        args.append(self._serialize_NoneTrueFalse(create_prefix))
-        args.append(self._serialize_NoneTrueFalse(force_new_repo))
-        args.append(self._serialize_NoneString(stacked_on))
-        # stack_on_pwd is often/usually our transport
-        if stack_on_pwd:
-            try:
-                stack_on_pwd = transport.relpath(stack_on_pwd)
-                if not stack_on_pwd:
-                    stack_on_pwd = '.'
-            except errors.PathNotChild:
-                pass
-        args.append(self._serialize_NoneString(stack_on_pwd))
-        args.append(self._serialize_NoneString(repo_format_name))
-        args.append(self._serialize_NoneTrueFalse(make_working_trees))
-        args.append(self._serialize_NoneTrueFalse(shared_repo))
-        request_network_name = self._network_name or \
-            BzrDirFormat.get_default_format().network_name()
-        try:
-            response = client.call('BzrDirFormat.initialize_ex_1.16',
-                request_network_name, path, *args)
-        except errors.UnknownSmartMethod:
-            client._medium._remember_remote_is_before((1,16))
-            local_dir_format = BzrDirMetaFormat1()
-            self._supply_sub_formats_to(local_dir_format)
-            return local_dir_format.initialize_on_transport_ex(transport,
-                use_existing_dir=use_existing_dir, create_prefix=create_prefix,
-                force_new_repo=force_new_repo, stacked_on=stacked_on,
-                stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
-                make_working_trees=make_working_trees, shared_repo=shared_repo,
-                vfs_only=True)
-        except errors.ErrorFromSmartServer, err:
-            remote._translate_error(err, path=path)
-        repo_path = response[0]
-        bzrdir_name = response[6]
-        require_stacking = response[7]
-        require_stacking = self.parse_NoneTrueFalse(require_stacking)
-        format = RemoteBzrDirFormat()
-        format._network_name = bzrdir_name
-        self._supply_sub_formats_to(format)
-        bzrdir = remote.RemoteBzrDir(transport, format, _client=client)
-        if repo_path:
-            repo_format = remote.response_tuple_to_repo_format(response[1:])
-            if repo_path == '.':
-                repo_path = ''
-            if repo_path:
-                repo_bzrdir_format = RemoteBzrDirFormat()
-                repo_bzrdir_format._network_name = response[5]
-                repo_bzr = remote.RemoteBzrDir(transport.clone(repo_path),
-                    repo_bzrdir_format)
-            else:
-                repo_bzr = bzrdir
-            final_stack = response[8] or None
-            final_stack_pwd = response[9] or None
-            if final_stack_pwd:
-                final_stack_pwd = urlutils.join(
-                    transport.base, final_stack_pwd)
-            remote_repo = remote.RemoteRepository(repo_bzr, repo_format)
-            if len(response) > 10:
-                # Updated server verb that locks remotely.
-                repo_lock_token = response[10] or None
-                remote_repo.lock_write(repo_lock_token, _skip_rpc=True)
-                if repo_lock_token:
-                    remote_repo.dont_leave_lock_in_place()
-            else:
-                remote_repo.lock_write()
-            policy = UseExistingRepository(remote_repo, final_stack,
-                final_stack_pwd, require_stacking)
-            policy.acquire_repository()
-        else:
-            remote_repo = None
-            policy = None
-        bzrdir._format.set_branch_format(self.get_branch_format())
-        if require_stacking:
-            # The repo has already been created, but we need to make sure that
-            # we'll make a stackable branch.
-            bzrdir._format.require_stacking(_skip_repo=True)
-        return remote_repo, bzrdir, require_stacking, policy
-
-    def _open(self, transport):
-        return remote.RemoteBzrDir(transport, self)
-
-    def __eq__(self, other):
-        if not isinstance(other, RemoteBzrDirFormat):
-            return False
-        return self.get_format_description() == other.get_format_description()
-
-    def __return_repository_format(self):
-        # Always return a RemoteRepositoryFormat object, but if a specific bzr
-        # repository format has been asked for, tell the RemoteRepositoryFormat
-        # that it should use that for init() etc.
-        result = remote.RemoteRepositoryFormat()
-        custom_format = getattr(self, '_repository_format', None)
-        if custom_format:
-            if isinstance(custom_format, remote.RemoteRepositoryFormat):
-                return custom_format
-            else:
-                # We will use the custom format to create repositories over the
-                # wire; expose its details like rich_root_data for code to
-                # query
-                result._custom_format = custom_format
-        return result
-
-    def get_branch_format(self):
-        result = BzrDirMetaFormat1.get_branch_format(self)
-        if not isinstance(result, remote.RemoteBranchFormat):
-            new_result = remote.RemoteBranchFormat()
-            new_result._custom_format = result
-            # cache the result
-            self.set_branch_format(new_result)
-            result = new_result
-        return result
-
-    repository_format = property(__return_repository_format,
-        BzrDirMetaFormat1._set_repository_format) #.im_func)
-
-
 controldir.ControlDirFormat.register_server_prober(RemoteBzrProber)
 
 

=== modified file 'bzrlib/controldir.py'
--- bzrlib/controldir.py	2011-03-05 02:54:47 +0000
+++ bzrlib/controldir.py	2011-03-11 14:21:26 +0000
@@ -710,12 +710,6 @@
     _default_format = None
     """The default format used for new control directories."""
 
-    _formats = []
-    """The registered control formats - .bzr, ....
-
-    This is a list of ControlDirFormat objects.
-    """
-
     _server_probers = []
     """The registered server format probers, e.g. RemoteBzrProber.
 
@@ -776,7 +770,8 @@
         """Register a format that does not use '.bzr' for its control dir.
 
         """
-        klass._formats.append(format)
+        raise errors.BzrError("ControlDirFormat.register_format() has been "
+            "removed in Bazaar 2.4. Please upgrade your plugins.")
 
     @classmethod
     def register_prober(klass, prober):
@@ -808,14 +803,13 @@
         return self.get_format_description().rstrip()
 
     @classmethod
-    def unregister_format(klass, format):
-        klass._formats.remove(format)
-
-    @classmethod
     def known_formats(klass):
         """Return all the known formats.
         """
-        return set(klass._formats)
+        result = set()
+        for prober_kls in klass._probers + klass._server_probers:
+            result.update(prober_kls.known_formats())
+        return result
 
     @classmethod
     def find_format(klass, transport, _server_formats=True):
@@ -929,6 +923,14 @@
         """
         raise NotImplementedError(self.probe_transport)
 
+    @classmethod
+    def known_formats(cls):
+        """Return the control dir formats known by this prober.
+
+        :return: A set of known formats.
+        """
+        raise NotImplementedError(cls.known_formats)
+
 
 class ControlDirFormatInfo(object):
 

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2011-03-08 20:37:43 +0000
+++ bzrlib/remote.py	2011-03-11 14:21:26 +0000
@@ -19,7 +19,7 @@
 from bzrlib import (
     bencode,
     branch,
-    bzrdir,
+    bzrdir as _mod_bzrdir,
     config,
     controldir,
     debug,
@@ -27,16 +27,13 @@
     graph,
     lock,
     lockdir,
-    repository,
     repository as _mod_repository,
-    revision,
     revision as _mod_revision,
     static_tuple,
     symbol_versioning,
     urlutils,
 )
 from bzrlib.branch import BranchReferenceFormat, BranchWriteLockResult
-from bzrlib.bzrdir import BzrDir, RemoteBzrDirFormat
 from bzrlib.decorators import needs_read_lock, needs_write_lock, only_raises
 from bzrlib.errors import (
     NoSuchRevision,
@@ -44,7 +41,8 @@
     )
 from bzrlib.lockable_files import LockableFiles
 from bzrlib.smart import client, vfs, repository as smart_repo
-from bzrlib.revision import ensure_null, NULL_REVISION
+from bzrlib.smart.client import _SmartClient
+from bzrlib.revision import NULL_REVISION
 from bzrlib.repository import RepositoryWriteLockResult
 from bzrlib.trace import mutter, note, warning
 
@@ -89,9 +87,246 @@
     return format
 
 
-# Note: RemoteBzrDirFormat is in bzrdir.py
-
-class RemoteBzrDir(BzrDir, _RpcHelper):
+# Note that RemoteBzrDirProber lives in bzrlib.bzrdir so bzrlib.remote
+# does not have to be imported unless a remote format is involved.
+
+class RemoteBzrDirFormat(_mod_bzrdir.BzrDirMetaFormat1):
+    """Format representing bzrdirs accessed via a smart server"""
+
+    supports_workingtrees = False
+
+    def __init__(self):
+        _mod_bzrdir.BzrDirMetaFormat1.__init__(self)
+        # XXX: It's a bit ugly that the network name is here, because we'd
+        # like to believe that format objects are stateless or at least
+        # immutable,  However, we do at least avoid mutating the name after
+        # it's returned.  See <https://bugs.launchpad.net/bzr/+bug/504102>
+        self._network_name = None
+
+    def __repr__(self):
+        return "%s(_network_name=%r)" % (self.__class__.__name__,
+            self._network_name)
+
+    def get_format_description(self):
+        if self._network_name:
+            real_format = controldir.network_format_registry.get(self._network_name)
+            return 'Remote: ' + real_format.get_format_description()
+        return 'bzr remote bzrdir'
+
+    def get_format_string(self):
+        raise NotImplementedError(self.get_format_string)
+
+    def network_name(self):
+        if self._network_name:
+            return self._network_name
+        else:
+            raise AssertionError("No network name set.")
+
+    def initialize_on_transport(self, transport):
+        try:
+            # hand off the request to the smart server
+            client_medium = transport.get_smart_medium()
+        except errors.NoSmartMedium:
+            # TODO: lookup the local format from a server hint.
+            local_dir_format = _mod_bzrdir.BzrDirMetaFormat1()
+            return local_dir_format.initialize_on_transport(transport)
+        client = _SmartClient(client_medium)
+        path = client.remote_path_from_transport(transport)
+        try:
+            response = client.call('BzrDirFormat.initialize', path)
+        except errors.ErrorFromSmartServer, err:
+            _translate_error(err, path=path)
+        if response[0] != 'ok':
+            raise errors.SmartProtocolError('unexpected response code %s' % (response,))
+        format = RemoteBzrDirFormat()
+        self._supply_sub_formats_to(format)
+        return RemoteBzrDir(transport, format)
+
+    def parse_NoneTrueFalse(self, arg):
+        if not arg:
+            return None
+        if arg == 'False':
+            return False
+        if arg == 'True':
+            return True
+        raise AssertionError("invalid arg %r" % arg)
+
+    def _serialize_NoneTrueFalse(self, arg):
+        if arg is False:
+            return 'False'
+        if arg:
+            return 'True'
+        return ''
+
+    def _serialize_NoneString(self, arg):
+        return arg or ''
+
+    def initialize_on_transport_ex(self, transport, use_existing_dir=False,
+        create_prefix=False, force_new_repo=False, stacked_on=None,
+        stack_on_pwd=None, repo_format_name=None, make_working_trees=None,
+        shared_repo=False):
+        try:
+            # hand off the request to the smart server
+            client_medium = transport.get_smart_medium()
+        except errors.NoSmartMedium:
+            do_vfs = True
+        else:
+            # Decline to open it if the server doesn't support our required
+            # version (3) so that the VFS-based transport will do it.
+            if client_medium.should_probe():
+                try:
+                    server_version = client_medium.protocol_version()
+                    if server_version != '2':
+                        do_vfs = True
+                    else:
+                        do_vfs = False
+                except errors.SmartProtocolError:
+                    # Apparently there's no usable smart server there, even though
+                    # the medium supports the smart protocol.
+                    do_vfs = True
+            else:
+                do_vfs = False
+        if not do_vfs:
+            client = _SmartClient(client_medium)
+            path = client.remote_path_from_transport(transport)
+            if client_medium._is_remote_before((1, 16)):
+                do_vfs = True
+        if do_vfs:
+            # TODO: lookup the local format from a server hint.
+            local_dir_format = _mod_bzrdir.BzrDirMetaFormat1()
+            self._supply_sub_formats_to(local_dir_format)
+            return local_dir_format.initialize_on_transport_ex(transport,
+                use_existing_dir=use_existing_dir, create_prefix=create_prefix,
+                force_new_repo=force_new_repo, stacked_on=stacked_on,
+                stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
+                make_working_trees=make_working_trees, shared_repo=shared_repo,
+                vfs_only=True)
+        return self._initialize_on_transport_ex_rpc(client, path, transport,
+            use_existing_dir, create_prefix, force_new_repo, stacked_on,
+            stack_on_pwd, repo_format_name, make_working_trees, shared_repo)
+
+    def _initialize_on_transport_ex_rpc(self, client, path, transport,
+        use_existing_dir, create_prefix, force_new_repo, stacked_on,
+        stack_on_pwd, repo_format_name, make_working_trees, shared_repo):
+        args = []
+        args.append(self._serialize_NoneTrueFalse(use_existing_dir))
+        args.append(self._serialize_NoneTrueFalse(create_prefix))
+        args.append(self._serialize_NoneTrueFalse(force_new_repo))
+        args.append(self._serialize_NoneString(stacked_on))
+        # stack_on_pwd is often/usually our transport
+        if stack_on_pwd:
+            try:
+                stack_on_pwd = transport.relpath(stack_on_pwd)
+                if not stack_on_pwd:
+                    stack_on_pwd = '.'
+            except errors.PathNotChild:
+                pass
+        args.append(self._serialize_NoneString(stack_on_pwd))
+        args.append(self._serialize_NoneString(repo_format_name))
+        args.append(self._serialize_NoneTrueFalse(make_working_trees))
+        args.append(self._serialize_NoneTrueFalse(shared_repo))
+        request_network_name = self._network_name or \
+            _mod_bzrdir.BzrDirFormat.get_default_format().network_name()
+        try:
+            response = client.call('BzrDirFormat.initialize_ex_1.16',
+                request_network_name, path, *args)
+        except errors.UnknownSmartMethod:
+            client._medium._remember_remote_is_before((1,16))
+            local_dir_format = _mod_bzrdir.BzrDirMetaFormat1()
+            self._supply_sub_formats_to(local_dir_format)
+            return local_dir_format.initialize_on_transport_ex(transport,
+                use_existing_dir=use_existing_dir, create_prefix=create_prefix,
+                force_new_repo=force_new_repo, stacked_on=stacked_on,
+                stack_on_pwd=stack_on_pwd, repo_format_name=repo_format_name,
+                make_working_trees=make_working_trees, shared_repo=shared_repo,
+                vfs_only=True)
+        except errors.ErrorFromSmartServer, err:
+            _translate_error(err, path=path)
+        repo_path = response[0]
+        bzrdir_name = response[6]
+        require_stacking = response[7]
+        require_stacking = self.parse_NoneTrueFalse(require_stacking)
+        format = RemoteBzrDirFormat()
+        format._network_name = bzrdir_name
+        self._supply_sub_formats_to(format)
+        bzrdir = RemoteBzrDir(transport, format, _client=client)
+        if repo_path:
+            repo_format = response_tuple_to_repo_format(response[1:])
+            if repo_path == '.':
+                repo_path = ''
+            if repo_path:
+                repo_bzrdir_format = RemoteBzrDirFormat()
+                repo_bzrdir_format._network_name = response[5]
+                repo_bzr = RemoteBzrDir(transport.clone(repo_path),
+                    repo_bzrdir_format)
+            else:
+                repo_bzr = bzrdir
+            final_stack = response[8] or None
+            final_stack_pwd = response[9] or None
+            if final_stack_pwd:
+                final_stack_pwd = urlutils.join(
+                    transport.base, final_stack_pwd)
+            remote_repo = RemoteRepository(repo_bzr, repo_format)
+            if len(response) > 10:
+                # Updated server verb that locks remotely.
+                repo_lock_token = response[10] or None
+                remote_repo.lock_write(repo_lock_token, _skip_rpc=True)
+                if repo_lock_token:
+                    remote_repo.dont_leave_lock_in_place()
+            else:
+                remote_repo.lock_write()
+            policy = _mod_bzrdir.UseExistingRepository(remote_repo, final_stack,
+                final_stack_pwd, require_stacking)
+            policy.acquire_repository()
+        else:
+            remote_repo = None
+            policy = None
+        bzrdir._format.set_branch_format(self.get_branch_format())
+        if require_stacking:
+            # The repo has already been created, but we need to make sure that
+            # we'll make a stackable branch.
+            bzrdir._format.require_stacking(_skip_repo=True)
+        return remote_repo, bzrdir, require_stacking, policy
+
+    def _open(self, transport):
+        return RemoteBzrDir(transport, self)
+
+    def __eq__(self, other):
+        if not isinstance(other, RemoteBzrDirFormat):
+            return False
+        return self.get_format_description() == other.get_format_description()
+
+    def __return_repository_format(self):
+        # Always return a RemoteRepositoryFormat object, but if a specific bzr
+        # repository format has been asked for, tell the RemoteRepositoryFormat
+        # that it should use that for init() etc.
+        result = RemoteRepositoryFormat()
+        custom_format = getattr(self, '_repository_format', None)
+        if custom_format:
+            if isinstance(custom_format, RemoteRepositoryFormat):
+                return custom_format
+            else:
+                # We will use the custom format to create repositories over the
+                # wire; expose its details like rich_root_data for code to
+                # query
+                result._custom_format = custom_format
+        return result
+
+    def get_branch_format(self):
+        result = _mod_bzrdir.BzrDirMetaFormat1.get_branch_format(self)
+        if not isinstance(result, RemoteBranchFormat):
+            new_result = RemoteBranchFormat()
+            new_result._custom_format = result
+            # cache the result
+            self.set_branch_format(new_result)
+            result = new_result
+        return result
+
+    repository_format = property(__return_repository_format,
+        _mod_bzrdir.BzrDirMetaFormat1._set_repository_format) #.im_func)
+
+
+class RemoteBzrDir(_mod_bzrdir.BzrDir, _RpcHelper):
     """Control directory on a remote server, accessed via bzr:// or similar."""
 
     def __init__(self, transport, format, _client=None, _force_probe=False):
@@ -100,7 +335,7 @@
         :param _client: Private parameter for testing. Disables probing and the
             use of a real bzrdir.
         """
-        BzrDir.__init__(self, transport, format)
+        _mod_bzrdir.BzrDir.__init__(self, transport, format)
         # this object holds a delegated bzrdir that uses file-level operations
         # to talk to the other side
         self._real_bzrdir = None
@@ -166,7 +401,7 @@
                 import traceback
                 warning('VFS BzrDir access triggered\n%s',
                     ''.join(traceback.format_stack()))
-            self._real_bzrdir = BzrDir.open_from_transport(
+            self._real_bzrdir = _mod_bzrdir.BzrDir.open_from_transport(
                 self.root_transport, _server_formats=False)
             self._format._network_name = \
                 self._real_bzrdir._format.network_name()
@@ -178,7 +413,7 @@
         # Prevent aliasing problems in the next_open_branch_result cache.
         # See create_branch for rationale.
         self._next_open_branch_result = None
-        return BzrDir.break_lock(self)
+        return _mod_bzrdir.BzrDir.break_lock(self)
 
     def _vfs_cloning_metadir(self, require_stacking=False):
         self._ensure_real()
@@ -217,12 +452,12 @@
         branch_ref, branch_name = branch_info
         format = controldir.network_format_registry.get(control_name)
         if repo_name:
-            format.repository_format = repository.network_format_registry.get(
+            format.repository_format = _mod_repository.network_format_registry.get(
                 repo_name)
         if branch_ref == 'ref':
             # XXX: we need possible_transports here to avoid reopening the
             # connection to the referenced location
-            ref_bzrdir = BzrDir.open(branch_name)
+            ref_bzrdir = _mod_bzrdir.BzrDir.open(branch_name)
             branch_format = ref_bzrdir.cloning_metadir().get_branch_format()
             format.set_branch_format(branch_format)
         elif branch_ref == 'branch':
@@ -465,7 +700,7 @@
         return RemoteBzrDirConfig(self)
 
 
-class RemoteRepositoryFormat(repository.RepositoryFormat):
+class RemoteRepositoryFormat(_mod_repository.RepositoryFormat):
     """Format for repositories accessed over a _SmartClient.
 
     Instances of this repository are represented by RemoteRepository
@@ -490,7 +725,7 @@
     supports_leaving_lock = True
 
     def __init__(self):
-        repository.RepositoryFormat.__init__(self)
+        _mod_repository.RepositoryFormat.__init__(self)
         self._custom_format = None
         self._network_name = None
         self._creating_bzrdir = None
@@ -587,7 +822,7 @@
             network_name = self._network_name
         else:
             # Select the current bzrlib default and ask for that.
-            reference_bzrdir_format = bzrdir.format_registry.get('default')()
+            reference_bzrdir_format = _mod_bzrdir.format_registry.get('default')()
             reference_format = reference_bzrdir_format.repository_format
             network_name = reference_format.network_name()
         # 2) try direct creation via RPC
@@ -619,7 +854,7 @@
 
     def _ensure_real(self):
         if self._custom_format is None:
-            self._custom_format = repository.network_format_registry.get(
+            self._custom_format = _mod_repository.network_format_registry.get(
                 self._network_name)
 
     @property
@@ -721,7 +956,7 @@
         # transport, but I'm not sure it's worth making this method
         # optional -- mbp 2010-04-21
         return self.bzrdir.get_repository_transport(None)
-        
+
     def __str__(self):
         return "%s(%s)" % (self.__class__.__name__, self.base)
 
@@ -861,7 +1096,7 @@
         """Private method for using with old (< 1.2) servers to fallback."""
         if revision_id is None:
             revision_id = ''
-        elif revision.is_null(revision_id):
+        elif _mod_revision.is_null(revision_id):
             return {}
 
         path = self.bzrdir._path_for_remote_call(self._client)
@@ -948,7 +1183,7 @@
         """See Repository.gather_stats()."""
         path = self.bzrdir._path_for_remote_call(self._client)
         # revid can be None to indicate no revisions, not just NULL_REVISION
-        if revid is None or revision.is_null(revid):
+        if revid is None or _mod_revision.is_null(revid):
             fmt_revid = ''
         else:
             fmt_revid = revid
@@ -1375,7 +1610,7 @@
                     'revision_ids is mutually exclusive with revision_id')
             if revision_id is not None:
                 revision_ids = [revision_id]
-        inter_repo = repository.InterRepository.get(other, self)
+        inter_repo = _mod_repository.InterRepository.get(other, self)
         return inter_repo.search_missing_revision_ids(
             find_ghosts=find_ghosts, revision_ids=revision_ids,
             if_present_ids=if_present_ids)
@@ -1397,13 +1632,13 @@
             # check that last_revision is in 'from' and then return a
             # no-operation.
             if (revision_id is not None and
-                not revision.is_null(revision_id)):
+                not _mod_revision.is_null(revision_id)):
                 self.get_revision(revision_id)
             return 0, []
         # if there is no specific appropriate InterRepository, this will get
         # the InterRepository base class, which raises an
         # IncompatibleRepositories when asked to fetch.
-        inter = repository.InterRepository.get(source, self)
+        inter = _mod_repository.InterRepository.get(source, self)
         return inter.fetch(revision_id=revision_id,
             find_ghosts=find_ghosts, fetch_spec=fetch_spec)
 
@@ -1634,7 +1869,7 @@
             tmpdir = osutils.mkdtemp()
             try:
                 _extract_tar(tar, tmpdir)
-                tmp_bzrdir = BzrDir.open(tmpdir)
+                tmp_bzrdir = _mod_bzrdir.BzrDir.open(tmpdir)
                 tmp_repo = tmp_bzrdir.open_repository()
                 tmp_repo.copy_content_into(destination, revision_id)
             finally:
@@ -1801,7 +2036,7 @@
             raise errors.UnexpectedSmartServerResponse(response)
 
 
-class RemoteStreamSink(repository.StreamSink):
+class RemoteStreamSink(_mod_repository.StreamSink):
 
     def _insert_real(self, stream, src_format, resume_tokens):
         self.target_repo._ensure_real()
@@ -1908,7 +2143,7 @@
         self._last_substream and self._last_stream so that the stream can be
         resumed by _resume_stream_with_vfs.
         """
-                    
+
         stream_iter = iter(stream)
         for substream_kind, substream in stream_iter:
             if substream_kind == 'inventory-deltas':
@@ -1917,9 +2152,9 @@
                 return
             else:
                 yield substream_kind, substream
-            
-
-class RemoteStreamSource(repository.StreamSource):
+
+
+class RemoteStreamSource(_mod_repository.StreamSource):
     """Stream data from a remote server."""
 
     def get_stream(self, search):
@@ -2132,7 +2367,7 @@
             network_name = self._custom_format.network_name()
         else:
             # Select the current bzrlib default and ask for that.
-            reference_bzrdir_format = bzrdir.format_registry.get('default')()
+            reference_bzrdir_format = _mod_bzrdir.format_registry.get('default')()
             reference_format = reference_bzrdir_format.get_branch_format()
             self._custom_format = reference_format
             network_name = reference_format.network_name()
@@ -2422,7 +2657,7 @@
             self._is_stacked = False
         else:
             self._is_stacked = True
-        
+
     def _vfs_get_tags_bytes(self):
         self._ensure_real()
         return self._real_branch._get_tags_bytes()
@@ -2763,7 +2998,7 @@
         # XXX: These should be returned by the set_last_revision_info verb
         old_revno, old_revid = self.last_revision_info()
         self._run_pre_change_branch_tip_hooks(revno, revision_id)
-        revision_id = ensure_null(revision_id)
+        revision_id = _mod_revision.ensure_null(revision_id)
         try:
             response = self._call('Branch.set_last_revision_info',
                 self._remote_path(), self._lock_token, self._repo_lock_token,

=== modified file 'bzrlib/tests/blackbox/test_upgrade.py'
--- bzrlib/tests/blackbox/test_upgrade.py	2011-03-04 15:29:08 +0000
+++ bzrlib/tests/blackbox/test_upgrade.py	2011-03-11 14:21:26 +0000
@@ -129,8 +129,10 @@
 
     def test_upgrade_control_dir(self):
         old_format = OldBzrDirFormat()
-        self.addCleanup(bzrdir.BzrDirFormat.unregister_format, old_format)
-        bzrdir.BzrDirFormat.register_format(old_format)
+        self.addCleanup(bzrdir.BzrProber.formats.remove,
+            old_format.get_format_string())
+        bzrdir.BzrProber.formats.register(old_format.get_format_string(),
+            old_format)
         self.addCleanup(controldir.ControlDirFormat._set_default_format,
                         controldir.ControlDirFormat.get_default_format())
 

=== modified file 'bzrlib/tests/per_controldir/test_controldir.py'
--- bzrlib/tests/per_controldir/test_controldir.py	2011-03-08 00:53:45 +0000
+++ bzrlib/tests/per_controldir/test_controldir.py	2011-03-11 14:21:26 +0000
@@ -26,7 +26,6 @@
     errors,
     gpg,
     osutils,
-    repository,
     transport,
     ui,
     urlutils,
@@ -46,6 +45,7 @@
     )
 from bzrlib.remote import (
     RemoteBzrDir,
+    RemoteBzrDirFormat,
     RemoteRepository,
     )
 
@@ -906,6 +906,8 @@
         readonly_t = self.get_readonly_transport()
         made_control = self.bzrdir_format.initialize(t.base)
         self.failUnless(isinstance(made_control, controldir.ControlDir))
+        if isinstance(self.bzrdir_format, RemoteBzrDirFormat):
+            return
         self.assertEqual(self.bzrdir_format,
                          controldir.ControlDirFormat.find_format(readonly_t))
         direct_opened_dir = self.bzrdir_format.open(readonly_t)
@@ -1068,15 +1070,11 @@
         self.assertIsInstance(control, controldir.ControlDir)
         opened = bzrdir.BzrDir.open(t.base)
         expected_format = self.bzrdir_format
-        if isinstance(expected_format, bzrdir.RemoteBzrDirFormat):
-            # Current RemoteBzrDirFormat's do not reliably get network_name
-            # set, so we skip a number of tests for RemoteBzrDirFormat's.
-            self.assertIsInstance(control, RemoteBzrDir)
-        else:
-            if need_meta and expected_format.fixed_components:
-                # Pre-metadir formats change when we are making something that
-                # needs a metaformat, because clone is used for push.
-                expected_format = bzrdir.BzrDirMetaFormat1()
+        if need_meta and expected_format.fixed_components:
+            # Pre-metadir formats change when we are making something that
+            # needs a metaformat, because clone is used for push.
+            expected_format = bzrdir.BzrDirMetaFormat1()
+        if not isinstance(expected_format, RemoteBzrDirFormat):
             self.assertEqual(control._format.network_name(),
                 expected_format.network_name())
             self.assertEqual(control._format.network_name(),
@@ -1093,7 +1091,7 @@
         # key in the registry gives back the same format. For remote obects
         # we check that the network_name of the RemoteBzrDirFormat we have
         # locally matches the actual format present on disk.
-        if isinstance(format, bzrdir.RemoteBzrDirFormat):
+        if isinstance(format, RemoteBzrDirFormat):
             dir._ensure_real()
             real_dir = dir._real_bzrdir
             network_name = format.network_name()

=== modified file 'bzrlib/tests/test_bzrdir.py'
--- bzrlib/tests/test_bzrdir.py	2011-02-24 12:08:37 +0000
+++ bzrlib/tests/test_bzrdir.py	2011-03-11 14:21:26 +0000
@@ -272,10 +272,14 @@
     def test_find_format(self):
         # is the right format object found for a branch?
         # create a branch with a few known format objects.
-        bzrdir.BzrDirFormat.register_format(BzrDirFormatTest1())
-        self.addCleanup(bzrdir.BzrDirFormat.unregister_format, BzrDirFormatTest1())
-        bzrdir.BzrDirFormat.register_format(BzrDirFormatTest2())
-        self.addCleanup(bzrdir.BzrDirFormat.unregister_format, BzrDirFormatTest2())
+        bzrdir.BzrProber.formats.register(BzrDirFormatTest1.get_format_string(),
+            BzrDirFormatTest1())
+        self.addCleanup(bzrdir.BzrProber.formats.remove,
+            BzrDirFormatTest1.get_format_string())
+        bzrdir.BzrProber.formats.register(BzrDirFormatTest2.get_format_string(),
+            BzrDirFormatTest2())
+        self.addCleanup(bzrdir.BzrProber.formats.remove,
+            BzrDirFormatTest2.get_format_string())
         t = self.get_transport()
         self.build_tree(["foo/", "bar/"], transport=t)
         def check_format(format, url):
@@ -305,7 +309,7 @@
         # make a bzrdir
         format.initialize(url)
         # register a format for it.
-        bzrdir.BzrDirFormat.register_format(format)
+        bzrdir.BzrProber.formats.register(format.get_format_string(), format)
         # which bzrdir.Open will refuse (not supported)
         self.assertRaises(UnsupportedFormatError, bzrdir.BzrDir.open, url)
         # which bzrdir.open_containing will refuse (not supported)
@@ -314,7 +318,7 @@
         t = _mod_transport.get_transport(url)
         self.assertEqual(format.open(t), bzrdir.BzrDir.open_unsupported(url))
         # unregister the format
-        bzrdir.BzrDirFormat.unregister_format(format)
+        bzrdir.BzrProber.formats.remove(format.get_format_string())
         # now open_downlevel should fail too.
         self.assertRaises(UnknownFormatError, bzrdir.BzrDir.open_unsupported, url)
 
@@ -1071,73 +1075,6 @@
             bzrdir.BzrDirFormat.get_default_format()))
 
 
-class NotBzrDir(bzrlib.bzrdir.BzrDir):
-    """A non .bzr based control directory."""
-
-    def __init__(self, transport, format):
-        self._format = format
-        self.root_transport = transport
-        self.transport = transport.clone('.not')
-
-
-class NotBzrDirFormat(bzrlib.bzrdir.BzrDirFormat):
-    """A test class representing any non-.bzr based disk format."""
-
-    def initialize_on_transport(self, transport):
-        """Initialize a new .not dir in the base directory of a Transport."""
-        transport.mkdir('.not')
-        return self.open(transport)
-
-    def open(self, transport):
-        """Open this directory."""
-        return NotBzrDir(transport, self)
-
-    @classmethod
-    def _known_formats(self):
-        return set([NotBzrDirFormat()])
-
-
-class NotBzrDirProber(controldir.Prober):
-
-    def probe_transport(self, transport):
-        """Our format is present if the transport ends in '.not/'."""
-        if transport.has('.not'):
-            return NotBzrDirFormat()
-
-
-class TestNotBzrDir(TestCaseWithTransport):
-    """Tests for using the bzrdir api with a non .bzr based disk format.
-
-    If/when one of these is in the core, we can let the implementation tests
-    verify this works.
-    """
-
-    def test_create_and_find_format(self):
-        # create a .notbzr dir
-        format = NotBzrDirFormat()
-        dir = format.initialize(self.get_url())
-        self.assertIsInstance(dir, NotBzrDir)
-        # now probe for it.
-        controldir.ControlDirFormat.register_prober(NotBzrDirProber)
-        try:
-            found = bzrlib.bzrdir.BzrDirFormat.find_format(self.get_transport())
-            self.assertIsInstance(found, NotBzrDirFormat)
-        finally:
-            controldir.ControlDirFormat.unregister_prober(NotBzrDirProber)
-
-    def test_included_in_known_formats(self):
-        not_format = NotBzrDirFormat()
-        bzrlib.controldir.ControlDirFormat.register_format(not_format)
-        try:
-            formats = bzrlib.bzrdir.BzrDirFormat.known_formats()
-            for format in formats:
-                if isinstance(format, NotBzrDirFormat):
-                    return
-            self.fail("No NotBzrDirFormat in %s" % formats)
-        finally:
-            bzrlib.controldir.ControlDirFormat.unregister_format(not_format)
-
-
 class NonLocalTests(TestCaseWithTransport):
     """Tests for bzrdir static behaviour on non local paths."""
 

=== modified file 'bzrlib/tests/test_controldir.py'
--- bzrlib/tests/test_controldir.py	2011-02-24 16:23:16 +0000
+++ bzrlib/tests/test_controldir.py	2011-03-11 14:21:26 +0000
@@ -21,8 +21,13 @@
 
 from bzrlib import (
     controldir,
+    errors,
     tests,
     )
+from bzrlib.tests.scenarios import load_tests_apply_scenarios
+
+
+load_tests = load_tests_apply_scenarios
 
 
 class SampleComponentFormat(controldir.ControlComponentFormat):
@@ -77,3 +82,101 @@
         formats = self.registry._get_all()
         self.assertEquals(1, len(formats))
         self.assertIsInstance(formats[0], SampleExtraComponentFormat)
+
+
+class TestControlDirFormatDeprecated(tests.TestCaseWithTransport):
+    """Tests for removed registration method in the ControlDirFormat facility."""
+
+    def test_register_format(self):
+        self.assertRaises(errors.BzrError,
+            controldir.ControlDirFormat.register_format, object())
+
+
+class TestProber(tests.TestCaseWithTransport):
+    """Per-prober tests."""
+
+    scenarios = [
+        (prober_cls.__name__, {'prober_cls': prober_cls})
+        for prober_cls in controldir.ControlDirFormat._probers]
+
+    def setUp(self):
+        super(TestProber, self).setUp()
+        self.prober = self.prober_cls()
+
+    def test_probe_transport_empty(self):
+        transport = self.get_transport(".")
+        self.assertRaises(errors.NotBranchError,
+            self.prober.probe_transport, transport)
+
+    def test_known_formats(self):
+        known_formats = self.prober_cls.known_formats()
+        self.assertIsInstance(known_formats, set)
+        for format in known_formats:
+            self.assertIsInstance(format, controldir.ControlDirFormat,
+                repr(format))
+
+
+class NotBzrDir(controldir.ControlDir):
+    """A non .bzr based control directory."""
+
+    def __init__(self, transport, format):
+        self._format = format
+        self.root_transport = transport
+        self.transport = transport.clone('.not')
+
+
+class NotBzrDirFormat(controldir.ControlDirFormat):
+    """A test class representing any non-.bzr based disk format."""
+
+    def initialize_on_transport(self, transport):
+        """Initialize a new .not dir in the base directory of a Transport."""
+        transport.mkdir('.not')
+        return self.open(transport)
+
+    def open(self, transport):
+        """Open this directory."""
+        return NotBzrDir(transport, self)
+
+
+class NotBzrDirProber(controldir.Prober):
+
+    def probe_transport(self, transport):
+        """Our format is present if the transport ends in '.not/'."""
+        if transport.has('.not'):
+            return NotBzrDirFormat()
+
+    @classmethod
+    def known_formats(cls):
+        return set([NotBzrDirFormat()])
+
+
+class TestNotBzrDir(tests.TestCaseWithTransport):
+    """Tests for using the controldir api with a non .bzr based disk format.
+
+    If/when one of these is in the core, we can let the implementation tests
+    verify this works.
+    """
+
+    def test_create_and_find_format(self):
+        # create a .notbzr dir
+        format = NotBzrDirFormat()
+        dir = format.initialize(self.get_url())
+        self.assertIsInstance(dir, NotBzrDir)
+        # now probe for it.
+        controldir.ControlDirFormat.register_prober(NotBzrDirProber)
+        try:
+            found = controldir.ControlDirFormat.find_format(self.get_transport())
+            self.assertIsInstance(found, NotBzrDirFormat)
+        finally:
+            controldir.ControlDirFormat.unregister_prober(NotBzrDirProber)
+
+    def test_included_in_known_formats(self):
+        controldir.ControlDirFormat.register_prober(NotBzrDirProber)
+        self.addCleanup(controldir.ControlDirFormat.unregister_prober, NotBzrDirProber)
+        formats = controldir.ControlDirFormat.known_formats()
+        self.assertIsInstance(formats, set)
+        for format in formats:
+            if isinstance(format, NotBzrDirFormat):
+                break
+        else:
+            self.fail("No NotBzrDirFormat in %s" % formats)

=== modified file 'bzrlib/tests/test_foreign.py'
--- bzrlib/tests/test_foreign.py	2011-01-12 23:33:40 +0000
+++ bzrlib/tests/test_foreign.py	2011-03-11 14:21:26 +0000
@@ -262,9 +262,6 @@
 
 
 def register_dummy_foreign_for_test(testcase):
-    controldir.ControlDirFormat.register_format(DummyForeignVcsDirFormat)
-    testcase.addCleanup(controldir.ControlDirFormat.unregister_format,
-                        DummyForeignVcsDirFormat)
     controldir.ControlDirFormat.register_prober(DummyForeignProber)
     testcase.addCleanup(controldir.ControlDirFormat.unregister_prober,
         DummyForeignProber)
@@ -284,6 +281,10 @@
             raise errors.NotBranchError(path=transport.base)
         return DummyForeignVcsDirFormat()
 
+    @classmethod
+    def known_formats(cls):
+        return set([DummyForeignVcsDirFormat()])
+
 
 class ForeignVcsRegistryTests(tests.TestCase):
     """Tests for the ForeignVcsRegistry class."""

=== modified file 'bzrlib/tests/test_import_tariff.py'
--- bzrlib/tests/test_import_tariff.py	2011-03-04 22:50:20 +0000
+++ bzrlib/tests/test_import_tariff.py	2011-03-11 14:21:26 +0000
@@ -110,6 +110,7 @@
             'bzrlib.remote',
             'bzrlib.sign_my_commits',
             'bzrlib.smart',
+            'bzrlib.smart.client',
             'bzrlib.transform',
             'bzrlib.version_info_formats.format_rio',
             'bzrlib.workingtree_2',

=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py	2011-03-03 07:16:12 +0000
+++ bzrlib/tests/test_remote.py	2011-03-11 14:21:26 +0000
@@ -52,6 +52,7 @@
     RemoteBranch,
     RemoteBranchFormat,
     RemoteBzrDir,
+    RemoteBzrDirFormat,
     RemoteRepository,
     RemoteRepositoryFormat,
     )
@@ -95,12 +96,12 @@
         self.addCleanup(self.transport.disconnect)
 
     def test_create_remote_bzrdir(self):
-        b = remote.RemoteBzrDir(self.transport, remote.RemoteBzrDirFormat())
+        b = remote.RemoteBzrDir(self.transport, RemoteBzrDirFormat())
         self.assertIsInstance(b, BzrDir)
 
     def test_open_remote_branch(self):
         # open a standalone branch in the working directory
-        b = remote.RemoteBzrDir(self.transport, remote.RemoteBzrDirFormat())
+        b = remote.RemoteBzrDir(self.transport, RemoteBzrDirFormat())
         branch = b.open_branch()
         self.assertIsInstance(branch, Branch)
 
@@ -124,7 +125,7 @@
         fmt = BzrDirFormat.find_format(self.transport)
         self.assertTrue(bzrdir.RemoteBzrProber
                         in controldir.ControlDirFormat._server_probers)
-        self.assertIsInstance(fmt, remote.RemoteBzrDirFormat)
+        self.assertIsInstance(fmt, RemoteBzrDirFormat)
 
     def test_open_detected_smart_format(self):
         fmt = BzrDirFormat.find_format(self.transport)
@@ -450,7 +451,7 @@
         client.add_expected_call(
             'BzrDir.open_branchV3', ('quack/',),
             'success', ('ref', self.get_url('referenced'))),
-        a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        a_bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         result = a_bzrdir.cloning_metadir()
         # We should have got a control dir matching the referenced branch.
@@ -469,7 +470,7 @@
         client.add_expected_call(
             'BzrDir.cloning_metadir', ('quack/', 'False'),
             'success', (control_name, '', ('branch', ''))),
-        a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        a_bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         result = a_bzrdir.cloning_metadir()
         # We should have got a reference control dir with default branch and
@@ -495,14 +496,14 @@
         client.add_expected_call(
             'BzrDir.open_2.1', ('quack/',), 'success', ('no',))
         self.assertRaises(errors.NotBranchError, RemoteBzrDir, transport,
-                remote.RemoteBzrDirFormat(), _client=client, _force_probe=True)
+                RemoteBzrDirFormat(), _client=client, _force_probe=True)
         self.assertFinished(client)
 
     def test_present_without_workingtree(self):
         client, transport = self.make_fake_client_and_transport()
         client.add_expected_call(
             'BzrDir.open_2.1', ('quack/',), 'success', ('yes', 'no'))
-        bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bd = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client, _force_probe=True)
         self.assertIsInstance(bd, RemoteBzrDir)
         self.assertFalse(bd.has_workingtree())
@@ -513,7 +514,7 @@
         client, transport = self.make_fake_client_and_transport()
         client.add_expected_call(
             'BzrDir.open_2.1', ('quack/',), 'success', ('yes', 'yes'))
-        bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bd = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client, _force_probe=True)
         self.assertIsInstance(bd, RemoteBzrDir)
         self.assertTrue(bd.has_workingtree())
@@ -526,7 +527,7 @@
             'BzrDir.open_2.1', ('quack/',), 'unknown', ('BzrDir.open_2.1',))
         client.add_expected_call(
             'BzrDir.open', ('quack/',), 'success', ('yes',))
-        bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bd = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client, _force_probe=True)
         self.assertIsInstance(bd, RemoteBzrDir)
         self.assertFinished(client)
@@ -548,7 +549,7 @@
             'BzrDir.open_2.1', ('quack/',), 'unknown', ('BzrDir.open_2.1',))
         client.add_expected_call(
             'BzrDir.open', ('quack/',), 'success', ('yes',))
-        bd = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bd = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client, _force_probe=True)
         self.assertIsInstance(bd, RemoteBzrDir)
         self.assertFinished(client)
@@ -585,7 +586,7 @@
         client.add_expected_call(
             'Branch.get_stacked_on_url', ('quack/',),
             'error', ('NotStacked',))
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         result = bzrdir.open_branch()
         self.assertIsInstance(result, RemoteBranch)
@@ -598,7 +599,7 @@
         transport = transport.clone('quack')
         client = FakeClient(transport.base)
         client.add_error_response('nobranch')
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         self.assertRaises(errors.NotBranchError, bzrdir.open_branch)
         self.assertEqual(
@@ -615,7 +616,7 @@
         transport = MemoryTransport()
         # no requests on the network - catches other api calls being made.
         client = FakeClient(transport.base)
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         # patch the open_branch call to record that it was called.
         bzrdir.open_branch = open_branch
@@ -640,7 +641,7 @@
         client.add_expected_call(
             'Branch.get_stacked_on_url', ('~hello/',),
             'error', ('NotStacked',))
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         result = bzrdir.open_branch()
         self.assertFinished(client)
@@ -663,7 +664,7 @@
         client.add_success_response(
             'ok', '', rich_response, subtree_response, external_lookup,
             network_name)
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         result = bzrdir.open_repository()
         self.assertEqual(
@@ -715,7 +716,7 @@
             'BzrDir.create_branch', ('quack/', network_name),
             'success', ('ok', network_name, '', 'no', 'no', 'yes',
             reference_repo_name))
-        a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        a_bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         branch = a_bzrdir.create_branch()
         # We should have got a remote branch
@@ -743,7 +744,7 @@
             'BzrDir.create_branch', ('extra/quack/', network_name),
             'success', ('ok', network_name, '', 'no', 'no', 'yes',
             reference_repo_name))
-        a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        a_bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         branch = a_bzrdir.create_branch(repository=repo)
         # We should have got a remote branch
@@ -778,7 +779,7 @@
                 'Bazaar repository format 2a (needs bzr 1.16 or later)\n',
                 'False'),
             'success', ('ok', 'yes', 'yes', 'yes', network_name))
-        a_bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        a_bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         repo = a_bzrdir.create_repository()
         # We should have got a remote repository
@@ -813,7 +814,7 @@
         client.add_success_response('stat', '0', '65535')
         remote_transport = RemoteTransport(server_url + 'quack/', medium=False,
             _client=client)
-        bzrdir = RemoteBzrDir(remote_transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(remote_transport, RemoteBzrDirFormat(),
             _client=client)
         repo = bzrdir.open_repository()
         self.assertEqual(
@@ -846,7 +847,7 @@
         client.add_success_response('stat', '0', '65535')
         remote_transport = RemoteTransport(server_url + 'quack/', medium=False,
             _client=client)
-        bzrdir = RemoteBzrDir(remote_transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(remote_transport, RemoteBzrDirFormat(),
             _client=client)
         repo = bzrdir.open_repository()
         self.assertEqual(
@@ -867,7 +868,7 @@
         transport = transport.clone('quack')
         client = FakeClient(transport.base)
         client.add_success_response('ok', '', 'no', 'no', 'no', network_name)
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         repo = bzrdir.open_repository()
         self.assertEqual(
@@ -880,7 +881,7 @@
 
     def test_success(self):
         """Simple test for typical successful call."""
-        fmt = bzrdir.RemoteBzrDirFormat()
+        fmt = RemoteBzrDirFormat()
         default_format_name = BzrDirFormat.get_default_format().network_name()
         transport = self.get_transport()
         client = FakeClient(transport.base)
@@ -902,7 +903,7 @@
         """Error responses are translated, e.g. 'PermissionDenied' raises the
         corresponding error from the client.
         """
-        fmt = bzrdir.RemoteBzrDirFormat()
+        fmt = RemoteBzrDirFormat()
         default_format_name = BzrDirFormat.get_default_format().network_name()
         transport = self.get_transport()
         client = FakeClient(transport.base)
@@ -926,7 +927,7 @@
         """Integration test for error translation."""
         transport = self.make_smart_server('foo')
         transport = transport.clone('no-such-path')
-        fmt = bzrdir.RemoteBzrDirFormat()
+        fmt = RemoteBzrDirFormat()
         err = self.assertRaises(errors.NoSuchFile,
             fmt.initialize_on_transport_ex, transport, create_prefix=False)
 
@@ -963,7 +964,7 @@
 
     def make_remote_bzrdir(self, transport, client):
         """Make a RemotebzrDir using 'client' as the _client."""
-        return RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        return RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
 
 
@@ -1296,7 +1297,7 @@
         client.add_expected_call(
             'Branch.get_stacked_on_url', ('stacked/',),
             'success', ('ok', vfs_url))
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=client)
         repo_fmt = remote.RemoteRepositoryFormat()
         repo_fmt._custom_format = stacked_branch.repository._format
@@ -1329,7 +1330,7 @@
         # this will also do vfs access, but that goes direct to the transport
         # and isn't seen by the FakeClient.
         bzrdir = RemoteBzrDir(self.get_transport('stacked'),
-            remote.RemoteBzrDirFormat(), _client=client)
+            RemoteBzrDirFormat(), _client=client)
         branch = bzrdir.open_branch()
         result = branch.get_stacked_on_url()
         self.assertEqual('../base', result)
@@ -1362,7 +1363,7 @@
             'Branch.get_stacked_on_url', ('stacked/',),
             'success', ('ok', '../base'))
         bzrdir = RemoteBzrDir(self.get_transport('stacked'),
-            remote.RemoteBzrDirFormat(), _client=client)
+            RemoteBzrDirFormat(), _client=client)
         branch = bzrdir.open_branch()
         result = branch.get_stacked_on_url()
         self.assertEqual('../base', result)
@@ -1938,7 +1939,7 @@
         client = FakeClient(transport.base)
         transport = transport.clone(transport_path)
         # we do not want bzrdir to make any remote calls
-        bzrdir = RemoteBzrDir(transport, remote.RemoteBzrDirFormat(),
+        bzrdir = RemoteBzrDir(transport, RemoteBzrDirFormat(),
             _client=False)
         repo = RemoteRepository(bzrdir, None, _client=client)
         return repo, client

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2011-03-08 23:52:59 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2011-03-11 14:21:26 +0000
@@ -161,6 +161,13 @@
   ``import_last_revision_info_and_tags`` method instead.
   (Andrew Bennetts)
 
+* ``ControlDirFormat.register_format`` now takes an identifier argument
+  which can be used to unregister the format later. Control dir formats
+  can now be registered lazily using
+  ``ControlDirFormat.register_lazy_format``.
+  ``ControlDirFormat.unregister_format`` now takes a string rather than a
+  ``ControlDirFormat`` as its argument. (Jelmer Vernooij)
+
 * ``bzrlib.revionspec.dwim_revspecs`` is deprecated.
   Use ``bzrlib.revisionspec.RevisionSpec_dwim.append_possible_revspec`` and
   ``bzrlib.revisionspec.RevisionSpec_dwim.append_possible_lazy_revspec``
@@ -170,6 +177,10 @@
   indicates whether the components of the bzrdir can be upgraded
   independent of the ``BzrDir``. (Jelmer Vernooij)
 
+* ``BzrProber.register_format`` and ``BzrProber.unregister_format`` are
+  now deprecated in favour of the ``BzrProber.formats`` format registry.
+  (Jelmer Vernooij)
+
 * ``ControlDir`` implementations no longer have to provide the ``get_branch_transport``,
   ``get_workingtree_transport`` and ``get_repository_transport`` methods.
   (Jelmer Vernooij, #730325)