Merge lp:~jelmer/bzr/branchfmt into lp:bzr

[Jelmer Vernooij]

Move the old branch format 5 into a separate module.

Move the old branch format 5 into a separate module.

[Vincent Ladeuil (vila)]

I'm not sure I understand the purpose of the proposal here, it seems like a step towards... something I can't put my finger on ;)

Should branchfmt be named branch_old_formats_you_should_not_use_anymore ?

If not, what kind of modules will be added there ? When ?

No objection per se but I've got the feeling we don't use the right names here nor put this code in the right place.

I think I would feel more comfortable if this code could be handled in the same way the weave code was but if it's too early for that may be we should just wait.

Can you clarify ?

review: Needs Information

[Jelmer Vernooij (jelmer)]

> I'm not sure I understand the purpose of the proposal here, it seems like a
> step towards... something I can't put my finger on ;)
>
> Should branchfmt be named branch_old_formats_you_should_ngot_use_anymore ?
No, as I can imagine us moving other branch formats in there in the future as well.

> If not, what kind of modules will be added there ? When ?
I would imagine adding new formats there in the future if we have a good reason to. We could add the existing formats there too, e.g. in bzrlib.branchfmt.revinfo or something like that (since they store just the revinfo tuple rather than full history).

> No objection per se but I've got the feeling we don't use the right names here
> nor put this code in the right place.
>
> I think I would feel more comfortable if this code could be handled in the
> same way the weave code was but if it's too early for that may be we should
> just wait.
The weave branch format already lives in bzrlib/plugins/weave. This is just about putting the various branch formats in separate files (like bzrlib.repofmt), rather than in bzrlib.branch where it is always loaded.

[Vincent Ladeuil (vila)]

Ok, so, please remove the hideous '.THIS' file for good, add a summary of your comments above (and the additional ones you made on IRC) into bzrlib/branchfmt/__init__.py so the intent get out of your head and we'll be good ;)

review: Approve

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2012-06-30 06:34:22 +0000
+++ bzrlib/branch.py	2012-07-06 17:08:19 +0000
@@ -2039,45 +2039,6 @@
             recommend_upgrade=recommend_upgrade, basedir=basedir)
 
 
-class BzrBranchFormat5(BranchFormatMetadir):
-    """Bzr branch format 5.
-
-    This format has:
-     - a revision-history file.
-     - a format string
-     - a lock dir guarding the branch itself
-     - all of this stored in a branch/ subdirectory
-     - works with shared repositories.
-
-    This format is new in bzr 0.8.
-    """
-
-    def _branch_class(self):
-        return BzrBranch5
-
-    @classmethod
-    def get_format_string(cls):
-        """See BranchFormat.get_format_string()."""
-        return "Bazaar-NG branch format 5\n"
-
-    def get_format_description(self):
-        """See BranchFormat.get_format_description()."""
-        return "Branch format 5"
-
-    def initialize(self, a_bzrdir, name=None, repository=None,
-                   append_revisions_only=None):
-        """Create a branch of this format in a_bzrdir."""
-        if append_revisions_only:
-            raise errors.UpgradeRequired(a_bzrdir.user_url)
-        utf8_files = [('revision-history', ''),
-                      ('branch-name', ''),
-                      ]
-        return self._initialize_helper(a_bzrdir, utf8_files, name, repository)
-
-    def supports_tags(self):
-        return False
-
-
 class BzrBranchFormat6(BranchFormatMetadir):
     """Branch format with last-revision and tags.
 
@@ -2332,11 +2293,11 @@
 
 # formats which have no format string are not discoverable
 # and not independently creatable, so are not registered.
-__format5 = BzrBranchFormat5()
 __format6 = BzrBranchFormat6()
 __format7 = BzrBranchFormat7()
 __format8 = BzrBranchFormat8()
-format_registry.register(__format5)
+format_registry.register_lazy(
+    "Bazaar-NG branch format 5\n", "bzrlib.branchfmt.fullhistory", "BzrBranchFormat5")
 format_registry.register(BranchReferenceFormat())
 format_registry.register(__format6)
 format_registry.register(__format7)
@@ -2662,106 +2623,6 @@
         self.control_transport.put_bytes('format', self._format.as_string())
 
 
-class FullHistoryBzrBranch(BzrBranch):
-    """Bzr branch which contains the full revision history."""
-
-    @needs_write_lock
-    def set_last_revision_info(self, revno, revision_id):
-        if not revision_id or not isinstance(revision_id, basestring):
-            raise errors.InvalidRevisionId(revision_id=revision_id, branch=self)
-        revision_id = _mod_revision.ensure_null(revision_id)
-        # this old format stores the full history, but this api doesn't
-        # provide it, so we must generate, and might as well check it's
-        # correct
-        history = self._lefthand_history(revision_id)
-        if len(history) != revno:
-            raise AssertionError('%d != %d' % (len(history), revno))
-        self._set_revision_history(history)
-
-    def _read_last_revision_info(self):
-        rh = self._revision_history()
-        revno = len(rh)
-        if revno:
-            return (revno, rh[-1])
-        else:
-            return (0, _mod_revision.NULL_REVISION)
-
-    def _set_revision_history(self, rev_history):
-        if 'evil' in debug.debug_flags:
-            mutter_callsite(3, "set_revision_history scales with history.")
-        check_not_reserved_id = _mod_revision.check_not_reserved_id
-        for rev_id in rev_history:
-            check_not_reserved_id(rev_id)
-        if Branch.hooks['post_change_branch_tip']:
-            # Don't calculate the last_revision_info() if there are no hooks
-            # that will use it.
-            old_revno, old_revid = self.last_revision_info()
-        if len(rev_history) == 0:
-            revid = _mod_revision.NULL_REVISION
-        else:
-            revid = rev_history[-1]
-        self._run_pre_change_branch_tip_hooks(len(rev_history), revid)
-        self._write_revision_history(rev_history)
-        self._clear_cached_state()
-        self._cache_revision_history(rev_history)
-        if Branch.hooks['post_change_branch_tip']:
-            self._run_post_change_branch_tip_hooks(old_revno, old_revid)
-
-    def _write_revision_history(self, history):
-        """Factored out of set_revision_history.
-
-        This performs the actual writing to disk.
-        It is intended to be called by set_revision_history."""
-        self._transport.put_bytes(
-            'revision-history', '\n'.join(history),
-            mode=self.bzrdir._get_file_mode())
-
-    def _gen_revision_history(self):
-        history = self._transport.get_bytes('revision-history').split('\n')
-        if history[-1:] == ['']:
-            # There shouldn't be a trailing newline, but just in case.
-            history.pop()
-        return history
-
-    def _synchronize_history(self, destination, revision_id):
-        if not isinstance(destination, FullHistoryBzrBranch):
-            super(BzrBranch, self)._synchronize_history(
-                destination, revision_id)
-            return
-        if revision_id == _mod_revision.NULL_REVISION:
-            new_history = []
-        else:
-            new_history = self._revision_history()
-        if revision_id is not None and new_history != []:
-            try:
-                new_history = new_history[:new_history.index(revision_id) + 1]
-            except ValueError:
-                rev = self.repository.get_revision(revision_id)
-                new_history = rev.get_history(self.repository)[1:]
-        destination._set_revision_history(new_history)
-
-    @needs_write_lock
-    def generate_revision_history(self, revision_id, last_rev=None,
-        other_branch=None):
-        """Create a new revision history that will finish with revision_id.
-
-        :param revision_id: the new tip to use.
-        :param last_rev: The previous last_revision. If not None, then this
-            must be a ancestory of revision_id, or DivergedBranches is raised.
-        :param other_branch: The other branch that DivergedBranches should
-            raise with respect to.
-        """
-        self._set_revision_history(self._lefthand_history(revision_id,
-            last_rev, other_branch))
-
-
-class BzrBranch5(FullHistoryBzrBranch):
-    """A format 5 branch. This supports new features over plain branches.
-
-    It has support for a master_branch which is the data for bound branches.
-    """
-
-
 class BzrBranch8(BzrBranch):
     """A branch that stores tree-reference locations."""
 

=== added directory 'bzrlib/branchfmt'
=== added file 'bzrlib/branchfmt/__init__.py'
--- bzrlib/branchfmt/__init__.py	1970-01-01 00:00:00 +0000
+++ bzrlib/branchfmt/__init__.py	2012-07-06 17:08:19 +0000
@@ -0,0 +1,25 @@
+# Copyright (C) 2012 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Branch formats.
+
+This package contains various branch format implementations. Ideally
+all specific format implementations will be moved out of bzrlib.branch
+into this package.
+"""
+
+from __future__ import absolute_import
+

=== added file 'bzrlib/branchfmt/fullhistory.py'
--- bzrlib/branchfmt/fullhistory.py	1970-01-01 00:00:00 +0000
+++ bzrlib/branchfmt/fullhistory.py	2012-07-06 17:08:19 +0000
@@ -0,0 +1,178 @@
+# Copyright (C) 2006-2012 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Full history branch formats."""
+
+from __future__ import absolute_import
+
+from bzrlib import (
+    debug,
+    errors,
+    revision as _mod_revision,
+    )
+
+from bzrlib.branch import (
+    Branch,
+    BranchFormatMetadir,
+    BzrBranch,
+    )
+
+from bzrlib.decorators import (
+    needs_write_lock,
+    )
+from bzrlib.trace import mutter_callsite
+
+
+class FullHistoryBzrBranch(BzrBranch):
+    """Bzr branch which contains the full revision history."""
+
+    @needs_write_lock
+    def set_last_revision_info(self, revno, revision_id):
+        if not revision_id or not isinstance(revision_id, basestring):
+            raise errors.InvalidRevisionId(revision_id=revision_id, branch=self)
+        revision_id = _mod_revision.ensure_null(revision_id)
+        # this old format stores the full history, but this api doesn't
+        # provide it, so we must generate, and might as well check it's
+        # correct
+        history = self._lefthand_history(revision_id)
+        if len(history) != revno:
+            raise AssertionError('%d != %d' % (len(history), revno))
+        self._set_revision_history(history)
+
+    def _read_last_revision_info(self):
+        rh = self._revision_history()
+        revno = len(rh)
+        if revno:
+            return (revno, rh[-1])
+        else:
+            return (0, _mod_revision.NULL_REVISION)
+
+    def _set_revision_history(self, rev_history):
+        if 'evil' in debug.debug_flags:
+            mutter_callsite(3, "set_revision_history scales with history.")
+        check_not_reserved_id = _mod_revision.check_not_reserved_id
+        for rev_id in rev_history:
+            check_not_reserved_id(rev_id)
+        if Branch.hooks['post_change_branch_tip']:
+            # Don't calculate the last_revision_info() if there are no hooks
+            # that will use it.
+            old_revno, old_revid = self.last_revision_info()
+        if len(rev_history) == 0:
+            revid = _mod_revision.NULL_REVISION
+        else:
+            revid = rev_history[-1]
+        self._run_pre_change_branch_tip_hooks(len(rev_history), revid)
+        self._write_revision_history(rev_history)
+        self._clear_cached_state()
+        self._cache_revision_history(rev_history)
+        if Branch.hooks['post_change_branch_tip']:
+            self._run_post_change_branch_tip_hooks(old_revno, old_revid)
+
+    def _write_revision_history(self, history):
+        """Factored out of set_revision_history.
+
+        This performs the actual writing to disk.
+        It is intended to be called by set_revision_history."""
+        self._transport.put_bytes(
+            'revision-history', '\n'.join(history),
+            mode=self.bzrdir._get_file_mode())
+
+    def _gen_revision_history(self):
+        history = self._transport.get_bytes('revision-history').split('\n')
+        if history[-1:] == ['']:
+            # There shouldn't be a trailing newline, but just in case.
+            history.pop()
+        return history
+
+    def _synchronize_history(self, destination, revision_id):
+        if not isinstance(destination, FullHistoryBzrBranch):
+            super(BzrBranch, self)._synchronize_history(
+                destination, revision_id)
+            return
+        if revision_id == _mod_revision.NULL_REVISION:
+            new_history = []
+        else:
+            new_history = self._revision_history()
+        if revision_id is not None and new_history != []:
+            try:
+                new_history = new_history[:new_history.index(revision_id) + 1]
+            except ValueError:
+                rev = self.repository.get_revision(revision_id)
+                new_history = rev.get_history(self.repository)[1:]
+        destination._set_revision_history(new_history)
+
+    @needs_write_lock
+    def generate_revision_history(self, revision_id, last_rev=None,
+        other_branch=None):
+        """Create a new revision history that will finish with revision_id.
+
+        :param revision_id: the new tip to use.
+        :param last_rev: The previous last_revision. If not None, then this
+            must be a ancestory of revision_id, or DivergedBranches is raised.
+        :param other_branch: The other branch that DivergedBranches should
+            raise with respect to.
+        """
+        self._set_revision_history(self._lefthand_history(revision_id,
+            last_rev, other_branch))
+
+
+class BzrBranch5(FullHistoryBzrBranch):
+    """A format 5 branch. This supports new features over plain branches.
+
+    It has support for a master_branch which is the data for bound branches.
+    """
+
+
+class BzrBranchFormat5(BranchFormatMetadir):
+    """Bzr branch format 5.
+
+    This format has:
+     - a revision-history file.
+     - a format string
+     - a lock dir guarding the branch itself
+     - all of this stored in a branch/ subdirectory
+     - works with shared repositories.
+
+    This format is new in bzr 0.8.
+    """
+
+    def _branch_class(self):
+        return BzrBranch5
+
+    @classmethod
+    def get_format_string(cls):
+        """See BranchFormat.get_format_string()."""
+        return "Bazaar-NG branch format 5\n"
+
+    def get_format_description(self):
+        """See BranchFormat.get_format_description()."""
+        return "Branch format 5"
+
+    def initialize(self, a_bzrdir, name=None, repository=None,
+                   append_revisions_only=None):
+        """Create a branch of this format in a_bzrdir."""
+        if append_revisions_only:
+            raise errors.UpgradeRequired(a_bzrdir.user_url)
+        utf8_files = [('revision-history', ''),
+                      ('branch-name', ''),
+                      ]
+        return self._initialize_helper(a_bzrdir, utf8_files, name, repository)
+
+    def supports_tags(self):
+        return False
+
+
+

=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2012-06-18 11:43:07 +0000
+++ bzrlib/bzrdir.py	2012-07-06 17:08:19 +0000
@@ -53,6 +53,7 @@
     workingtree_3,
     workingtree_4,
     )
+from bzrlib.branchfmt import fullhistory as fullhistorybranch
 from bzrlib.repofmt import knitpack_repo
 from bzrlib.transport import (
     do_catching_redirections,
@@ -1811,7 +1812,7 @@
             old = branch._format.__class__
             new = self.target_format.get_branch_format().__class__
             while old != new:
-                if (old == _mod_branch.BzrBranchFormat5 and
+                if (old == fullhistorybranch.BzrBranchFormat5 and
                     new in (_mod_branch.BzrBranchFormat6,
                         _mod_branch.BzrBranchFormat7,
                         _mod_branch.BzrBranchFormat8)):
@@ -2113,7 +2114,7 @@
 register_metadir(controldir.format_registry, 'knit',
     'bzrlib.repofmt.knitrepo.RepositoryFormatKnit1',
     'Format using knits.  Recommended for interoperation with bzr <= 0.14.',
-    branch_format='bzrlib.branch.BzrBranchFormat5',
+    branch_format='bzrlib.branchfmt.fullhistory.BzrBranchFormat5',
     tree_format='bzrlib.workingtree_3.WorkingTreeFormat3',
     hidden=True,
     deprecated=True)
@@ -2122,7 +2123,7 @@
     help='Format using dirstate for working trees. '
         'Compatible with bzr 0.8 and '
         'above when accessed over the network. Introduced in bzr 0.15.',
-    branch_format='bzrlib.branch.BzrBranchFormat5',
+    branch_format='bzrlib.branchfmt.fullhistory.BzrBranchFormat5',
     tree_format='bzrlib.workingtree_4.WorkingTreeFormat4',
     hidden=True,
     deprecated=True)

=== modified file 'bzrlib/plugins/weave_fmt/__init__.py'
--- bzrlib/plugins/weave_fmt/__init__.py	2011-12-18 12:46:49 +0000
+++ bzrlib/plugins/weave_fmt/__init__.py	2012-07-06 17:08:19 +0000
@@ -86,7 +86,7 @@
 register_metadir(controldir.format_registry, 'metaweave',
     'bzrlib.plugins.weave_fmt.repository.RepositoryFormat7',
     'Transitional format in 0.8.  Slower than knit.',
-    branch_format='bzrlib.branch.BzrBranchFormat5',
+    branch_format='bzrlib.branchfmt.fullhistory.BzrBranchFormat5',
     tree_format='bzrlib.workingtree_3.WorkingTreeFormat3',
     hidden=True,
     deprecated=True)

=== modified file 'bzrlib/plugins/weave_fmt/branch.py'
--- bzrlib/plugins/weave_fmt/branch.py	2012-07-02 10:42:23 +0000
+++ bzrlib/plugins/weave_fmt/branch.py	2012-07-06 17:08:19 +0000
@@ -34,6 +34,8 @@
 from bzrlib.branch import (
     BranchFormat,
     BranchWriteLockResult,
+    )
+from bzrlib.branchfmt.fullhistory import (
     FullHistoryBzrBranch,
     )
 

=== modified file 'bzrlib/plugins/weave_fmt/bzrdir.py'
--- bzrlib/plugins/weave_fmt/bzrdir.py	2012-02-23 23:26:35 +0000
+++ bzrlib/plugins/weave_fmt/bzrdir.py	2012-07-06 17:08:19 +0000
@@ -545,7 +545,7 @@
     def convert(self, to_convert, pb):
         """See Converter.convert()."""
         from bzrlib.plugins.weave_fmt.repository import RepositoryFormat7
-        from bzrlib.branch import BzrBranchFormat5
+        from bzrlib.branchfmt.fullhistory import BzrBranchFormat5
         self.bzrdir = to_convert
         self.pb = ui.ui_factory.nested_progress_bar()
         self.count = 0

=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- bzrlib/tests/per_repository/test_repository.py	2012-01-27 16:27:26 +0000
+++ bzrlib/tests/per_repository/test_repository.py	2012-07-06 17:08:19 +0000
@@ -628,7 +628,7 @@
             self.get_vfs_only_url('remote')).open_repository()
         # Make a branch in that repo in an old format that isn't the default
         # branch format for the repo.
-        from bzrlib.branch import BzrBranchFormat5
+        from bzrlib.branchfmt.fullhistory import BzrBranchFormat5
         format = remote_backing_repo.bzrdir.cloning_metadir()
         format._branch_format = BzrBranchFormat5()
         remote_transport = remote_repo.bzrdir.root_transport.clone('branch')

=== modified file 'bzrlib/tests/test_branch.py'
--- bzrlib/tests/test_branch.py	2012-03-28 16:13:49 +0000
+++ bzrlib/tests/test_branch.py	2012-07-06 17:08:19 +0000
@@ -30,11 +30,14 @@
     config,
     controldir,
     errors,
-    symbol_versioning,
     tests,
     trace,
     urlutils,
     )
+from bzrlib.branchfmt.fullhistory import (
+    BzrBranch5,
+    BzrBranchFormat5,
+    )
 
 
 class TestDefaultFormat(tests.TestCase):
@@ -75,10 +78,10 @@
         url = self.get_url()
         bdir = bzrdir.BzrDirMetaFormat1().initialize(url)
         bdir.create_repository()
-        branch = _mod_branch.BzrBranchFormat5().initialize(bdir)
+        branch = BzrBranchFormat5().initialize(bdir)
         t = self.get_transport()
         self.log("branch instance is %r" % branch)
-        self.assert_(isinstance(branch, _mod_branch.BzrBranch5))
+        self.assert_(isinstance(branch, BzrBranch5))
         self.assertIsDirectory('.', t)
         self.assertIsDirectory('.bzr/branch', t)
         self.assertIsDirectory('.bzr/branch/lock', t)
@@ -185,7 +188,7 @@
             format.initialize(dir)
             found_format = _mod_branch.BranchFormatMetadir.find_format(dir)
             self.assertIsInstance(found_format, format.__class__)
-        check_format(_mod_branch.BzrBranchFormat5(), "bar")
+        check_format(BzrBranchFormat5(), "bar")
 
     def test_find_format_factory(self):
         dir = bzrdir.BzrDirMetaFormat1().initialize(self.get_url())

=== modified file 'bzrlib/tests/test_bzrdir.py'
--- bzrlib/tests/test_bzrdir.py	2012-06-18 11:43:07 +0000
+++ bzrlib/tests/test_bzrdir.py	2012-07-06 17:08:19 +0000
@@ -35,7 +35,6 @@
     revision as _mod_revision,
     osutils,
     remote,
-    symbol_versioning,
     transport as _mod_transport,
     urlutils,
     win32utils,
@@ -43,6 +42,7 @@
     workingtree_4,
     )
 import bzrlib.branch
+from bzrlib.branchfmt.fullhistory import BzrBranchFormat5
 from bzrlib.errors import (
     NotBranchError,
     NoColocatedBranchSupport,
@@ -992,7 +992,7 @@
         branch_base = t.clone('branch').base
         self.assertEqual(branch_base, dir.get_branch_transport(None).base)
         self.assertEqual(branch_base,
-                         dir.get_branch_transport(bzrlib.branch.BzrBranchFormat5()).base)
+                         dir.get_branch_transport(BzrBranchFormat5()).base)
         repository_base = t.clone('repository').base
         self.assertEqual(repository_base, dir.get_repository_transport(None).base)
         repository_format = repository.format_registry.get_default()