[RFC][MERGE] bzrdir phase 3
John A Meinel
john at arbash-meinel.com
Thu Feb 16 15:39:54 GMT 2006
Robert Collins wrote:
> On Thu, 2006-02-16 at 20:46 +1100, Robert Collins wrote:
>> Okies, this is the end of the bzrdir get-things-roughly-right work.
>
>
>> Hmm, I'm going to do one more change before merging this, but it should
>> not affect the review - I'm going to add ui support for 'bzr upgrade
>> --format=metadir' so that we bleeding edge junkies can upgrade from the
>> console rather than needing to use the library.
>
> Thats done, new diff attached.
>
> Rob
>
>
reviewing ...
>
> ------------------------------------------------------------------------
>
> === added file 'bzrlib/tests/blackbox/test_info.py'
looks good
> === added file 'bzrlib/tests/workingtree_implementations/test_is_control_filename.py'
...
> +
> +
> +class TestIsControlFilename(TestCaseWithWorkingTree):
> +
> + def validate_tree_is_controlfilename(self, tree):
> + """check that 'tree' obeys the contract for is_control_filename."""
> + bzrdirname = basename(tree.bzrdir.transport.base[:-1])
> + self.assertTrue(tree.is_control_filename(bzrdirname))
> + self.assertTrue(tree.is_control_filename(bzrdirname + '/subdir'))
> + self.assertFalse(tree.is_control_filename('dir/' + bzrdirname))
> + self.assertFalse(tree.is_control_filename('dir/' + bzrdirname + '/sub'))
What is the contract for 'is_control_filename', because most likely
'dir/.bzr/sub' would be a control file for a different branch. I don't
think we want an outer branch to be controlling an inner branch's meta
files.
> +
> + def test_dotbzr_is_control_in_cwd(self):
> + tree = self.make_branch_and_tree('.')
> + self.validate_tree_is_controlfilename(tree)
> +
> + def test_dotbzr_is_control_in_subdir(self):
> + tree = self.make_branch_and_tree('subdir')
> + self.validate_tree_is_controlfilename(tree)
> +
Why are you checking 'subdir' if you don't have an outer tree?
>
> === modified file 'bzrlib/__init__.py'
> --- bzrlib/__init__.py
> +++ bzrlib/__init__.py
> @@ -16,7 +16,6 @@
>
> """bzr library"""
>
> -BZRDIR = ".bzr"
Is this in preparation for allowing '_bzr'?
>
> # please keep these sorted (in C locale order) to aid merging
> DEFAULT_IGNORE = [
Side note:
When we get the new ignore stuff, I think we should move DEFAULT_IGNORES
out from bzrlib.DEFAULT_IGNORE.
>
> === modified file 'bzrlib/add.py'
good
> === modified file 'bzrlib/branch.py'
...
> - def _find_modes(self, t):
> - """Determine the appropriate modes for files and directories.
This should be on LockableFiles anyway.
...
> @@ -597,10 +574,8 @@
> if not _found:
> # we are being called directly and must probe.
> raise NotImplementedError
> - transport = a_bzrdir.get_branch_transport(self)
> - control_files = LockableFiles(transport, 'branch-lock')
> return BzrBranch(_format=self,
> - _control_files=control_files,
> + _control_files=a_bzrdir._control_files,
> a_bzrdir=a_bzrdir,
> _repository=a_bzrdir.open_repository())
Is this the right place to do it? If this section is only for format
4,5,6 branches, I'm okay with it. I certainly don't want to deny new
formats from having separate control files.
>
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py
> +++ bzrlib/builtins.py
> @@ -715,8 +715,7 @@
> @display_command
> def run(self, branch=None):
> import info
> - b = WorkingTree.open_containing(branch)[0].branch
> - info.show_info(b)
> + info.show_bzrdir_info(bzrdir.BzrDir.open_containing(branch)[0])
Shouldn't this be fixed to 'import bzrlib.info'?
>
> class cmd_remove(Command):
> @@ -819,7 +818,7 @@
> # locations if the user supplies an extended path
> if not os.path.exists(location):
> os.mkdir(location)
> - WorkingTree.create_standalone(location)
> + bzrdir.BzrDir.create_standalone_workingtree(location)
>
>
> class cmd_diff(Command):
> @@ -1451,11 +1450,29 @@
> this command. When the default format has changed you may also be warned
> during other operations to upgrade.
> """
> + # NB: this is used from the class without creating an instance, which is
> + # why it does not have a self parameter.
> + def get_format_type(typestring):
> + """Parse and return a format specifier."""
> + if typestring == "metadir":
> + return bzrdir.BzrDirMetaFormat1
> + msg = "No known bzr-dir format %s. Supported types are: metadir\n" %\
> + (typestring)
> + raise BzrCommandError(msg)
> +
Doesn't this need to be declared 'staticmethod'?
> takes_args = ['url?']
> -
> - def run(self, url='.'):
> + takes_options = [
> + Option('format',
> + help='Upgrade to a specific format rather than the'
> + ' current default format. Currently this '
> + ' option only accepts =metadir',
> + type=get_format_type),
> + ]
> +
> +
> + def run(self, url='.', format=None):
> from bzrlib.upgrade import upgrade
> - upgrade(url)
> + upgrade(url, format)
>
>
> class cmd_whoami(Command):
>
> === modified file 'bzrlib/bzrdir.py'
> --- bzrlib/bzrdir.py
> +++ bzrlib/bzrdir.py
> @@ -21,18 +21,32 @@
> """
>
> from copy import deepcopy
> +import os
> from cStringIO import StringIO
> from unittest import TestSuite
> -
>
> import bzrlib
> import bzrlib.errors as errors
> from bzrlib.lockable_files import LockableFiles
> from bzrlib.osutils import safe_unicode
> +from bzrlib.osutils import (
> + abspath,
> + pathjoin,
> + safe_unicode,
> + sha_strings,
> + sha_string,
> + )
Why do you have the closing ')' line up with the entries, rather than
lining up with the opening '('. This isn't the only place where occurs
(as I recall the test name listing does this as well). I think it would
look better with the () lined up.
> +from bzrlib.store.text import TextStore
> +from bzrlib.store.weave import WeaveStore
> +from bzrlib.symbol_versioning import *
> from bzrlib.trace import mutter
> -from bzrlib.symbol_versioning import *
> +from bzrlib.transactions import PassThroughTransaction
> from bzrlib.transport import get_transport
> from bzrlib.transport.local import LocalTransport
> +from bzrlib.weave import Weave
> +from bzrlib.weavefile import read_weave, write_weave
> +from bzrlib.xml4 import serializer_v4
> +from bzrlib.xml5 import serializer_v5
>
>
> class BzrDir(object):
> @@ -46,6 +60,10 @@
> root_transport
> a transport connected to the directory this bzr was opened from.
> """
> +
> + def can_update_format(self):
> + """Return true if this bzrdir is one whose format we can update."""
> + return True
>
> def _check_supported(self, format, allow_unsupported):
> """Check whether format is a supported format.
> @@ -343,6 +361,22 @@
> self._format = _format
> self.transport = _transport.clone('.bzr')
> self.root_transport = _transport
> +
> + def needs_format_update(self, format=None):
> + """Return true if this bzrdir needs update_format run on it.
> +
> + For instance, if the repository format is out of date but the
> + branch and working tree are not, this should return True.
> +
> + :param format: Optional parameter indicating a specific desired
> + format we want to end up at.
> + """
> + # for now, if the format is not the same as the system default,
> + # an upgrade is needed. In the future we will want to scan
> + # the individual repository/branch/checkout formats too
> + if format is None:
> + format = BzrDirFormat.get_default_format().__class__
> + return not isinstance(self._format, format)
The problem is 'BzrFormat6.needs_format_upgrade(BzrFormat4) would return
True, even though it is 'downgrading' the format.
>
> @staticmethod
> def open_unsupported(base):
> @@ -499,6 +533,12 @@
> class BzrDirPreSplitOut(BzrDir):
> """A common class for the all-in-one formats."""
>
> + def __init__(self, _transport, _format):
> + """See BzrDir.__init__."""
> + super(BzrDirPreSplitOut, self).__init__(_transport, _format)
> + self._control_files = LockableFiles(self.get_branch_transport(None),
> + 'branch-lock')
> +
> def clone(self, url, revision_id=None, basis=None, force_new_repo=False):
> """See BzrDir.clone()."""
> from bzrlib.workingtree import WorkingTreeFormat2
> @@ -608,6 +648,10 @@
> from bzrlib.repository import RepositoryFormat4
> return RepositoryFormat4().initialize(self, shared)
>
> + def needs_format_update(self, format=None):
> + """Format 4 dirs are always in need of updating."""
> + return True
> +
> def open_repository(self):
> """See BzrDir.open_repository."""
> from bzrlib.repository import RepositoryFormat4
> @@ -655,6 +699,10 @@
> individual formats are really split out.
> """
>
> + def can_update_format(self):
> + """See BzrDir.can_update_format()."""
> + return False
> +
> def create_branch(self):
> """See BzrDir.create_branch."""
> from bzrlib.branch import BranchFormat
> @@ -711,6 +759,11 @@
> except errors.FileExists:
> pass
> return self.transport.clone('checkout')
> +
> + def needs_format_update(self, format=None):
> + """See BzrDir.needs_format_update()."""
> + # currently there are no possible updates to meta1 formats.
> + return False
>
> def open_branch(self, unsupported=False):
> """See BzrDir.open_branch."""
> @@ -776,6 +829,20 @@
> def get_format_string(self):
> """Return the ASCII format string that identifies this format."""
> raise NotImplementedError(self.get_format_string)
> +
> + def get_updater(self, format=None):
> + """Return the updater to use to convert bzrdirs needing updates.
> +
> + This returns a bzrlib.bzrdir.Converter object.
> +
> + This should return the best upgrader to step this format towards the
> + current default format. In the case of plugins we can/shouold provide
> + some means for them to extend the range of returnable converters.
Small typo. And also why I thought the target format should be the one
who knows how to upgrade.
> +
> + :param format: Optional format to override the default foramt of the
> + library.
> + """
> + raise NotImplementedError(self.get_updater)
>
> def initialize(self, url):
> """Create a bzr control dir at this url and return an opened copy."""
> @@ -843,6 +910,9 @@
> def set_default_format(klass, format):
> klass._default_format = format
>
> + def __str__(self):
> + return self.get_format_string()[:-1]
> +
> @classmethod
> def unregister_format(klass, format):
> assert klass._formats[format.get_format_string()] is format
> @@ -866,6 +936,11 @@
> """See BzrDirFormat.get_format_string()."""
> return "Bazaar-NG branch, format 0.0.4\n"
>
> + def get_updater(self, format=None):
> + """See BzrDirFormat.get_updater()."""
> + # there is one and only one upgrade path here.
> + return ConvertBzrDir4To5()
> +
> def initialize(self, url):
> """Format 4 branches cannot be created."""
> raise errors.UninitializableFormat(self)
> @@ -899,6 +974,11 @@
> """See BzrDirFormat.get_format_string()."""
> return "Bazaar-NG branch, format 5\n"
>
> + def get_updater(self, format=None):
> + """See BzrDirFormat.get_updater()."""
> + # there is one and only one upgrade path here.
> + return ConvertBzrDir5To6()
> +
> def initialize(self, url, _cloning=False):
> """Format 5 dirs always have working tree, branch and repository.
>
> @@ -933,6 +1013,11 @@
> """See BzrDirFormat.get_format_string()."""
> return "Bazaar-NG branch, format 6\n"
>
> + def get_updater(self, format=None):
> + """See BzrDirFormat.get_updater()."""
> + # there is one and only one upgrade path here.
> + return ConvertBzrDir6ToMeta()
> +
> def initialize(self, url, _cloning=False):
> """Format 6 dirs always have working tree, branch and repository.
>
> @@ -1079,3 +1164,405 @@
> copytree(self.base, base, symlinks=True)
> return ScratchDir(
> transport=bzrlib.transport.local.ScratchTransport(base))
> +
> +
> +class Converter(object):
> + """Converts a disk format object from one format to another."""
> +
> + def convert(self, to_convert, pb):
> + """Perform the conversion of to_convert, giving feedback via pb.
> +
> + :param to_convert: The disk object to convert.
> + :param pb: a progress bar to use for progress information.
> + """
> +
We probably want some sort of NotImplemented() exception here, rather
than nothing.
> +
> +class ConvertBzrDir4To5(Converter):
> + """Converts format 4 bzr dirs to format 5."""
> +
> + def __init__(self):
> + super(ConvertBzrDir4To5, self).__init__()
> + self.converted_revs = set()
> + self.absent_revisions = set()
> + self.text_count = 0
> + self.revisions = {}
> +
> + def convert(self, to_convert, pb):
> + """See Converter.convert()."""
> + self.bzrdir = to_convert
> + self.pb = pb
> + self.pb.note('starting upgrade from format 4 to 5')
> + if isinstance(self.bzrdir.transport, LocalTransport):
> + self.bzrdir.get_workingtree_transport(None).delete('stat-cache')
> + self._convert_to_weaves()
> + return BzrDir.open(self.bzrdir.root_transport.base)
> +
> + def _convert_to_weaves(self):
> + self.pb.note('note: upgrade may be faster if all store files are ungzipped first')
> + try:
> + # TODO permissions
> + stat = self.bzrdir.transport.stat('weaves')
> + if not S_ISDIR(stat.st_mode):
> + self.bzrdir.transport.delete('weaves')
> + self.bzrdir.transport.mkdir('weaves')
> + except errors.NoSuchFile:
> + self.bzrdir.transport.mkdir('weaves')
> + self.inv_weave = Weave('inventory')
> + # holds in-memory weaves for all files
> + self.text_weaves = {}
> + self.bzrdir.transport.delete('branch-format')
> + self.branch = self.bzrdir.open_branch()
> + self._convert_working_inv()
> + rev_history = self.branch.revision_history()
> + # to_read is a stack holding the revisions we still need to process;
> + # appending to it adds new highest-priority revisions
> + self.known_revisions = set(rev_history)
> + self.to_read = rev_history[-1:]
> + while self.to_read:
> + rev_id = self.to_read.pop()
> + if (rev_id not in self.revisions
> + and rev_id not in self.absent_revisions):
> + self._load_one_rev(rev_id)
> + self.pb.clear()
> + to_import = self._make_order()
> + for i, rev_id in enumerate(to_import):
> + self.pb.update('converting revision', i, len(to_import))
> + self._convert_one_rev(rev_id)
> + self.pb.clear()
> + self._write_all_weaves()
> + self._write_all_revs()
> + self.pb.note('upgraded to weaves:')
> + self.pb.note(' %6d revisions and inventories', len(self.revisions))
> + self.pb.note(' %6d revisions not present', len(self.absent_revisions))
> + self.pb.note(' %6d texts', self.text_count)
> + self._cleanup_spare_files_after_format4()
> + self.branch.control_files.put_utf8('branch-format', BzrDirFormat5().get_format_string())
> +
> + def _cleanup_spare_files_after_format4(self):
> + # FIXME working tree upgrade foo.
> + for n in 'merged-patches', 'pending-merged-patches':
> + try:
> + ## assert os.path.getsize(p) == 0
> + self.bzrdir.transport.delete(n)
> + except errors.NoSuchFile:
> + pass
> + self.bzrdir.transport.delete_tree('inventory-store')
> + self.bzrdir.transport.delete_tree('text-store')
> +
> + def _convert_working_inv(self):
> + inv = serializer_v4.read_inventory(self.branch.control_files.get('inventory'))
> + new_inv_xml = serializer_v5.write_inventory_to_string(inv)
> + # FIXME inventory is a working tree change.
> + self.branch.control_files.put('inventory', new_inv_xml)
> +
> + def _write_all_weaves(self):
> + controlweaves = WeaveStore(self.bzrdir.transport, prefixed=False)
> + weave_transport = self.bzrdir.transport.clone('weaves')
> + weaves = WeaveStore(weave_transport, prefixed=False)
> + transaction = PassThroughTransaction()
> +
> + controlweaves.put_weave('inventory', self.inv_weave, transaction)
> + i = 0
> + try:
> + for file_id, file_weave in self.text_weaves.items():
> + self.pb.update('writing weave', i, len(self.text_weaves))
> + weaves.put_weave(file_id, file_weave, transaction)
> + i += 1
> + finally:
> + self.pb.clear()
> +
> + def _write_all_revs(self):
> + """Write all revisions out in new form."""
> + self.bzrdir.transport.delete_tree('revision-store')
> + self.bzrdir.transport.mkdir('revision-store')
> + revision_transport = self.bzrdir.transport.clone('revision-store')
> + # TODO permissions
> + revision_store = TextStore(revision_transport,
> + prefixed=False,
> + compressed=True)
> + try:
> + for i, rev_id in enumerate(self.converted_revs):
> + self.pb.update('write revision', i, len(self.converted_revs))
> + rev_tmp = StringIO()
> + serializer_v5.write_revision(self.revisions[rev_id], rev_tmp)
> + rev_tmp.seek(0)
> + revision_store.add(rev_tmp, rev_id)
> + finally:
> + self.pb.clear()
> +
> + def _load_one_rev(self, rev_id):
> + """Load a revision object into memory.
> +
> + Any parents not either loaded or abandoned get queued to be
> + loaded."""
> + self.pb.update('loading revision',
> + len(self.revisions),
> + len(self.known_revisions))
> + if not self.branch.repository.revision_store.has_id(rev_id):
> + self.pb.clear()
> + self.pb.note('revision {%s} not present in branch; '
> + 'will be converted as a ghost',
> + rev_id)
> + self.absent_revisions.add(rev_id)
> + else:
> + rev_xml = self.branch.repository.revision_store.get(rev_id).read()
> + rev = serializer_v4.read_revision_from_string(rev_xml)
> + for parent_id in rev.parent_ids:
> + self.known_revisions.add(parent_id)
> + self.to_read.append(parent_id)
> + self.revisions[rev_id] = rev
> +
> + def _load_old_inventory(self, rev_id):
> + assert rev_id not in self.converted_revs
> + old_inv_xml = self.branch.repository.inventory_store.get(rev_id).read()
> + inv = serializer_v4.read_inventory_from_string(old_inv_xml)
> + rev = self.revisions[rev_id]
> + if rev.inventory_sha1:
> + assert rev.inventory_sha1 == sha_string(old_inv_xml), \
> + 'inventory sha mismatch for {%s}' % rev_id
> + return inv
> +
> + def _load_updated_inventory(self, rev_id):
> + assert rev_id in self.converted_revs
> + inv_xml = self.inv_weave.get_text(rev_id)
> + inv = serializer_v5.read_inventory_from_string(inv_xml)
> + return inv
> +
> + def _convert_one_rev(self, rev_id):
> + """Convert revision and all referenced objects to new format."""
> + rev = self.revisions[rev_id]
> + inv = self._load_old_inventory(rev_id)
> + present_parents = [p for p in rev.parent_ids
> + if p not in self.absent_revisions]
> + self._convert_revision_contents(rev, inv, present_parents)
> + self._store_new_weave(rev, inv, present_parents)
> + self.converted_revs.add(rev_id)
> +
> + def _store_new_weave(self, rev, inv, present_parents):
> + # the XML is now updated with text versions
> + if __debug__:
> + for file_id in inv:
> + ie = inv[file_id]
> + if ie.kind == 'root_directory':
> + continue
> + assert hasattr(ie, 'revision'), \
> + 'no revision on {%s} in {%s}' % \
> + (file_id, rev.revision_id)
> + new_inv_xml = serializer_v5.write_inventory_to_string(inv)
> + new_inv_sha1 = sha_string(new_inv_xml)
> + self.inv_weave.add(rev.revision_id,
> + present_parents,
> + new_inv_xml.splitlines(True),
> + new_inv_sha1)
> + rev.inventory_sha1 = new_inv_sha1
> +
> + def _convert_revision_contents(self, rev, inv, present_parents):
> + """Convert all the files within a revision.
> +
> + Also upgrade the inventory to refer to the text revision ids."""
> + rev_id = rev.revision_id
> + mutter('converting texts of revision {%s}',
> + rev_id)
> + parent_invs = map(self._load_updated_inventory, present_parents)
> + for file_id in inv:
> + ie = inv[file_id]
> + self._convert_file_version(rev, ie, parent_invs)
> +
> + def _convert_file_version(self, rev, ie, parent_invs):
> + """Convert one version of one file.
> +
> + The file needs to be added into the weave if it is a merge
> + of >=2 parents or if it's changed from its parent.
> + """
> + if ie.kind == 'root_directory':
> + return
> + file_id = ie.file_id
> + rev_id = rev.revision_id
> + w = self.text_weaves.get(file_id)
> + if w is None:
> + w = Weave(file_id)
> + self.text_weaves[file_id] = w
> + text_changed = False
> + previous_entries = ie.find_previous_heads(parent_invs, w)
> + for old_revision in previous_entries:
> + # if this fails, its a ghost ?
> + assert old_revision in self.converted_revs
> + self.snapshot_ie(previous_entries, ie, w, rev_id)
> + del ie.text_id
> + assert getattr(ie, 'revision', None) is not None
> +
> + def snapshot_ie(self, previous_revisions, ie, w, rev_id):
> + # TODO: convert this logic, which is ~= snapshot to
> + # a call to:. This needs the path figured out. rather than a work_tree
> + # a v4 revision_tree can be given, or something that looks enough like
> + # one to give the file content to the entry if it needs it.
> + # and we need something that looks like a weave store for snapshot to
> + # save against.
> + #ie.snapshot(rev, PATH, previous_revisions, REVISION_TREE, InMemoryWeaveStore(self.text_weaves))
> + if len(previous_revisions) == 1:
> + previous_ie = previous_revisions.values()[0]
> + if ie._unchanged(previous_ie):
> + ie.revision = previous_ie.revision
> + return
> + parent_indexes = map(w.lookup, previous_revisions)
> + if ie.has_text():
> + text = self.branch.repository.text_store.get(ie.text_id)
> + file_lines = text.readlines()
> + assert sha_strings(file_lines) == ie.text_sha1
> + assert sum(map(len, file_lines)) == ie.text_size
> + w.add(rev_id, parent_indexes, file_lines, ie.text_sha1)
> + self.text_count += 1
> + else:
> + w.add(rev_id, parent_indexes, [], None)
> + ie.revision = rev_id
> +
> + def _make_order(self):
> + """Return a suitable order for importing revisions.
> +
> + The order must be such that an revision is imported after all
> + its (present) parents.
> + """
> + todo = set(self.revisions.keys())
> + done = self.absent_revisions.copy()
> + order = []
> + while todo:
> + # scan through looking for a revision whose parents
> + # are all done
> + for rev_id in sorted(list(todo)):
> + rev = self.revisions[rev_id]
> + parent_ids = set(rev.parent_ids)
> + if parent_ids.issubset(done):
> + # can take this one now
> + order.append(rev_id)
> + todo.remove(rev_id)
> + done.add(rev_id)
> + return order
> +
> +
> +class ConvertBzrDir5To6(Converter):
> + """Converts format 5 bzr dirs to format 6."""
> +
> + def convert(self, to_convert, pb):
> + """See Converter.convert()."""
> + self.bzrdir = to_convert
> + self.pb = pb
> + self.pb.note('starting upgrade from format 5 to 6')
> + self._convert_to_prefixed()
> + return BzrDir.open(self.bzrdir.root_transport.base)
> +
> + def _convert_to_prefixed(self):
> + from bzrlib.store import hash_prefix
> + self.bzrdir.transport.delete('branch-format')
> + for store_name in ["weaves", "revision-store"]:
> + self.pb.note("adding prefixes to %s" % store_name)
> + store_transport = self.bzrdir.transport.clone(store_name)
> + for filename in store_transport.list_dir('.'):
> + if (filename.endswith(".weave") or
> + filename.endswith(".gz") or
> + filename.endswith(".sig")):
> + file_id = os.path.splitext(filename)[0]
> + else:
> + file_id = filename
> + prefix_dir = hash_prefix(file_id)
> + # FIXME keep track of the dirs made RBC 20060121
> + try:
> + store_transport.move(filename, prefix_dir + '/' + filename)
> + except errors.NoSuchFile: # catches missing dirs strangely enough
> + store_transport.mkdir(prefix_dir)
> + store_transport.move(filename, prefix_dir + '/' + filename)
> + self.bzrdir._control_files.put_utf8('branch-format', BzrDirFormat6().get_format_string())
> +
> +
> +class ConvertBzrDir6ToMeta(Converter):
> + """Converts format 6 bzr dirs to metadirs."""
> +
> + def convert(self, to_convert, pb):
> + """See Converter.convert()."""
> + self.bzrdir = to_convert
> + self.pb = pb
> + self.count = 0
> + self.total = 20 # the steps we know about
> + self.garbage_inventories = []
> +
> + self.pb.note('starting upgrade from format 6 to metadir')
> + self.bzrdir._control_files.put_utf8('branch-format', "Converting to format 6")
> + # its faster to move specific files around than to open and use the apis...
> + # first off, nuke ancestry.weave, it was never used.
> + try:
> + self.step('Removing ancestry.weave')
> + self.bzrdir.transport.delete('ancestry.weave')
> + except errors.NoSuchFile:
> + pass
> + # find out whats there
> + self.step('Finding branch files')
> + last_revision = self.bzrdir.open_workingtree().last_revision()
> + bzrcontents = self.bzrdir.transport.list_dir('.')
> + for name in bzrcontents:
> + if name.startswith('basis-inventory.'):
> + self.garbage_inventories.append(name)
Yay!! We get to delete basis-inventory.* files. :)
> + # create new directories for repository, working tree and branch
> + dir_mode = self.bzrdir._control_files._dir_mode
> + self.file_mode = self.bzrdir._control_files._file_mode
> + repository_names = [('inventory.weave', True),
> + ('revision-store', True),
> + ('weaves', True)]
> + self.step('Upgrading repository ')
> + self.bzrdir.transport.mkdir('repository', mode=dir_mode)
> + self.make_lock('repository')
> + # we hard code the formats here because we are converting into
> + # the meta format. The meta format upgrader can take this to a
> + # future format within each component.
> + self.put_format('repository', bzrlib.repository.RepositoryFormat7())
> + for entry in repository_names:
> + self.move_entry('repository', entry)
> +
> + self.step('Upgrading branch ')
> + self.bzrdir.transport.mkdir('branch', mode=dir_mode)
> + self.make_lock('branch')
> + self.put_format('branch', bzrlib.branch.BzrBranchFormat5())
> + branch_files = [('revision-history', True),
> + ('branch-name', True),
> + ('parent', False)]
> + for entry in branch_files:
> + self.move_entry('branch', entry)
> +
> + self.step('Upgrading working tree')
> + self.bzrdir.transport.mkdir('checkout', mode=dir_mode)
> + self.make_lock('checkout')
> + self.put_format('checkout', bzrlib.workingtree.WorkingTreeFormat3())
> + self.bzrdir.transport.delete_multi(self.garbage_inventories, self.pb)
> + checkout_files = [('pending-merges', True),
> + ('inventory', True),
> + ('stat-cache', False)]
> + for entry in checkout_files:
> + self.move_entry('checkout', entry)
> + if last_revision is not None:
> + self.bzrdir._control_files.put_utf8('checkout/last-revision',
> + last_revision)
> + self.bzrdir._control_files.put_utf8('branch-format', BzrDirMetaFormat1().get_format_string())
> + return BzrDir.open(self.bzrdir.root_transport.base)
> +
> + def make_lock(self, name):
> + """Make a lock for the new control dir name."""
> + self.step('Make %s lock' % name)
> + self.bzrdir.transport.put('%s/lock' % name, StringIO(), mode=self.file_mode)
Ultimately, the lock format is going to change, right? Is this happening
before we start using Meta-1 format?
Though I guess that would be a new Branch format as well (since it needs
to know about the new locking style).
> +
> + def move_entry(self, new_dir, entry):
> + """Move then entry name into new_dir."""
> + name = entry[0]
> + mandatory = entry[1]
> + self.step('Moving %s' % name)
> + try:
> + self.bzrdir.transport.move(name, '%s/%s' % (new_dir, name))
> + except errors.NoSuchFile:
> + if mandatory:
> + raise
> +
> + def put_format(self, dirname, format):
> + self.bzrdir._control_files.put_utf8('%s/format' % dirname, format.get_format_string())
> +
> + def step(self, message):
> + """Update the pb by a step."""
> + self.count +=1
> + self.pb.update('Upgrading repository ', self.count, self.total)
> +
>
> === modified file 'bzrlib/commands.py'
> --- bzrlib/commands.py
> +++ bzrlib/commands.py
> @@ -42,7 +42,6 @@
> BzrOptionError,
> NotBranchError)
> from bzrlib.revisionspec import RevisionSpec
> -from bzrlib import BZRDIR
> from bzrlib.option import Option
>
> plugin_cmds = {}
>
> === modified file 'bzrlib/errors.py'
> --- bzrlib/errors.py
> +++ bzrlib/errors.py
> @@ -294,6 +294,14 @@
> """Upgrade URL cannot work with readonly URL's."""
>
>
> +class UpToDateFormat(BzrNewError):
> + """The branch format %(format)s is already at the most recent format."""
> +
> + def __init__(self, format):
> + BzrNewError.__init__(self)
> + self.format = format
> +
> +
> class StrictCommitFailed(Exception):
> """Commit refused because there are unknowns in the tree."""
>
>
> === modified file 'bzrlib/info.py'
> --- bzrlib/info.py
> +++ bzrlib/info.py
> @@ -16,10 +16,14 @@
> # along with this program; if not, write to the Free Software
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>
> +__all__ = ['show_bzrdir_info']
> +
> import time
>
> +
> +import bzrlib.diff as diff
> from bzrlib.osutils import format_date
> -from bzrlib.workingtree import WorkingTree
> +from bzrlib.symbol_versioning import *
>
>
> def _countiter(it):
> @@ -30,12 +34,26 @@
> return i
>
>
> + at deprecated_function(zero_eight)
> +def show_info(b):
> + """Please see show_bzrdir_info."""
> + return show_bzrdir_info(b.bzrdir)
>
> -def show_info(b):
> - import diff
> +def show_bzrdir_info(a_bzrdir):
> + """Output to stdout the 'info' for a_bzrdir."""
> +
> + working = a_bzrdir.open_workingtree()
> + b = a_bzrdir.open_branch()
>
> - print 'branch format:', b.control_files.get_utf8(
> - 'branch-format').readline().rstrip('\n')
> + if working.bzrdir != b.bzrdir:
> + print 'working tree format:', working._format
> + print 'branch location:', b.bzrdir.root_transport.base
> + try:
> + b._format.get_format_string()
> + format = b._format
> + except NotImplementedError:
> + format = b.bzrdir._format
> + print 'branch format:', format
>
> def plural(n, base='', pl=None):
> if n == 1:
> @@ -47,12 +65,20 @@
>
> count_version_dirs = 0
>
> - working = b.bzrdir.open_workingtree()
> basis = working.basis_tree()
> work_inv = working.inventory
> delta = diff.compare_trees(basis, working, want_unchanged=True)
> + history = b.revision_history()
>
> print
> + if len(history) and working.last_revision() != history[-1]:
> + try:
> + missing_count = len(history) - history.index(working.last_revision())
> + except ValueError:
> + # consider it all out of date
> + missing_count = len(history)
> + print 'Working tree is out of date: missing %d revision%s.' % (
> + missing_count, plural(missing_count))
> print 'in the working tree:'
> print ' %8s unchanged' % len(delta.unchanged)
> print ' %8d modified' % len(delta.modified)
> @@ -80,7 +106,6 @@
>
> print
> print 'branch history:'
> - history = b.revision_history()
> revno = len(history)
> print ' %8d revision%s' % (revno, plural(revno))
> committers = {}
> @@ -107,7 +132,7 @@
> print
> print 'revision store:'
> c, t = b.repository.revision_store.total_size()
> - print ' %8d revisions' % c
> + print ' %8d revision%s' % (c, plural(c))
> print ' %8d kB' % (t/1024)
Isn't there supposed to be a special way of doing pluralization for
internationalization work? (Rather than just hard-coding 's' for
English). Speaking of which, is there some time when we are planning on
going through the code and adding _() around strings so that we can do
i18n work?
Perhaps that is an 0.999 version. But it probably should be done. (And
is going to lead to a lot of conflicts).
>
>
>
> === modified file 'bzrlib/missing.py'
> --- bzrlib/missing.py
> +++ bzrlib/missing.py
> @@ -1,7 +1,7 @@
> """\
> A plugin for displaying what revisions are in 'other' but not in local.
> """
Obviously the comment is incorrect now. But not really your fault.
> -from bzrlib.ui import ui_factory
> +import bzrlib.ui as ui
Definitely, otherwise setting ui_factory will have no effect, since we
have a locally bound variable.
> def iter_log_data(revisions, revision_source, verbose):
> from bzrlib.diff import compare_trees
> from bzrlib.tree import EmptyTree
> @@ -26,7 +26,7 @@
>
>
> def find_unmerged(local_branch, remote_branch):
> - progress = ui_factory.progress_bar()
> + progress = ui.ui_factory.progress_bar()
> local_branch.lock_read()
> try:
> remote_branch.lock_read()
>
> === modified file 'bzrlib/progress.py'
> --- bzrlib/progress.py
> +++ bzrlib/progress.py
> @@ -69,10 +69,11 @@
> show_spinner=False,
> show_eta=True,
> show_bar=True,
> - show_count=True):
> + show_count=True,
> + to_messages_file=sys.stdout):
> object.__init__(self)
> self.to_file = to_file
> -
> + self.to_messages_file = to_messages_file
> self.last_msg = None
> self.last_cnt = None
> self.last_total = None
> @@ -82,6 +83,11 @@
> self.show_bar = show_bar
> self.show_count = show_count
>
> + def note(self, fmt_string, *args, **kwargs):
> + """Record a note without disrupting the progress bar."""
> + self.clear()
> + self.to_messages_file.write(fmt_string % args)
> + self.to_messages_file.write('\n')
>
>
> class DummyProgress(_BaseProgressBar):
> @@ -98,7 +104,10 @@
> def clear(self):
> pass
>
> -
> + def note(self, fmt_string, *args, **kwargs):
> + """See _BaseProgressBar.note()."""
> +
> +
> class DotsProgressBar(_BaseProgressBar):
> def __init__(self, **kwargs):
> _BaseProgressBar.__init__(self, **kwargs)
> @@ -255,11 +264,9 @@
> self.to_file.write('\r' + m.ljust(self.width - 1))
> #self.to_file.flush()
>
> -
> def clear(self):
> self.to_file.write('\r%s\r' % (' ' * (self.width - 1)))
> #self.to_file.flush()
> -
>
>
> def str_tdelta(delt):
>
> === modified file 'bzrlib/repository.py'
> --- bzrlib/repository.py
> +++ bzrlib/repository.py
> @@ -83,18 +83,16 @@
> """Construct the current default format repository in a_bzrdir."""
> return RepositoryFormat.get_default_format().initialize(a_bzrdir)
>
> - def __init__(self, transport, branch_format, _format=None, a_bzrdir=None):
> + def __init__(self, _format, a_bzrdir):
> object.__init__(self)
> - if transport is not None:
> - warn("Repository.__init__(..., transport=XXX): The transport parameter is "
> - "deprecated and was never in a supported release. Please use "
> - "bzrdir.open_repository() or bzrdir.open_branch().repository.",
> - DeprecationWarning,
> - stacklevel=2)
> - self.control_files = LockableFiles(transport.clone(bzrlib.BZRDIR), 'README')
> - else:
> - # TODO: clone into repository if needed
> - self.control_files = LockableFiles(a_bzrdir.get_repository_transport(None), 'README')
> + if isinstance(_format, (RepositoryFormat4,
> + RepositoryFormat5,
> + RepositoryFormat6)):
> + # legacy: use a common control files.
> + self.control_files = a_bzrdir._control_files
> + else:
> + self.control_files = LockableFiles(a_bzrdir.get_repository_transport(None),
> + 'lock')
>
> dir_mode = self.control_files._dir_mode
> file_mode = self.control_files._file_mode
> @@ -114,7 +112,6 @@
> if self.control_files._transport.should_cache():
> ws.enable_cache = True
> return ws
> -
>
> def get_store(name, compressed=True, prefixed=False):
> # FIXME: This approach of assuming stores are all entirely compressed
> @@ -135,20 +132,6 @@
> # os.mkdir(cache_path)
> # store = bzrlib.store.CachedStore(store, cache_path)
> return store
> -
> - if branch_format is not None:
> - # circular dependencies:
> - from bzrlib.branch import (BzrBranchFormat4,
> - BzrBranchFormat5,
> - BzrBranchFormat6,
> - )
> - if isinstance(branch_format, BzrBranchFormat4):
> - self._format = RepositoryFormat4()
> - elif isinstance(branch_format, BzrBranchFormat5):
> - self._format = RepositoryFormat5()
> - elif isinstance(branch_format, BzrBranchFormat6):
> - self._format = RepositoryFormat6()
> -
>
> if isinstance(self._format, RepositoryFormat4):
> self.inventory_store = get_store('inventory-store')
> @@ -702,7 +685,7 @@
> if not _found:
> # we are being called directly and must probe.
> raise NotImplementedError
> - return Repository(None, branch_format=None, _format=self, a_bzrdir=a_bzrdir)
> + return Repository(_format=self, a_bzrdir=a_bzrdir)
>
> @classmethod
> def register_format(klass, format):
> @@ -735,7 +718,7 @@
>
> if not _internal:
> # always initialized when the bzrdir is.
> - return Repository(None, branch_format=None, _format=self, a_bzrdir=a_bzrdir)
> + return Repository(_format=self, a_bzrdir=a_bzrdir)
>
> # Create an empty weave
> sio = StringIO()
> @@ -759,7 +742,7 @@
> control_files.put(file, content)
> finally:
> control_files.unlock()
> - return Repository(None, branch_format=None, _format=self, a_bzrdir=a_bzrdir)
> + return Repository(_format=self, a_bzrdir=a_bzrdir)
>
>
> class RepositoryFormat4(PreSplitOutRepositoryFormat):
> @@ -873,7 +856,7 @@
> control_files.put_utf8('shared-storage', '')
> finally:
> control_files.unlock()
> - return Repository(None, branch_format=None, _format=self, a_bzrdir=a_bzrdir)
> + return Repository(_format=self, a_bzrdir=a_bzrdir)
>
> def __init__(self):
> super(RepositoryFormat7, self).__init__()
> @@ -889,10 +872,6 @@
> RepositoryFormat5(),
> RepositoryFormat6()]
>
> -
> -# TODO: jam 20060108 Create a new branch format, and as part of upgrade
> -# make sure that ancestry.weave is deleted (it is never used, but
> -# used to be created)
>
> class RepositoryTestProviderAdapter(object):
> """A tool to generate a suite testing multiple repository formats at once.
>
> === modified file 'bzrlib/tests/blackbox/__init__.py'
> --- bzrlib/tests/blackbox/__init__.py
> +++ bzrlib/tests/blackbox/__init__.py
> @@ -40,6 +40,7 @@
> 'bzrlib.tests.blackbox.test_diff',
> 'bzrlib.tests.blackbox.test_export',
> 'bzrlib.tests.blackbox.test_find_merge_base',
> + 'bzrlib.tests.blackbox.test_info',
> 'bzrlib.tests.blackbox.test_log',
> 'bzrlib.tests.blackbox.test_missing',
> 'bzrlib.tests.blackbox.test_outside_wt',
>
> === modified file 'bzrlib/tests/blackbox/test_update.py'
> --- bzrlib/tests/blackbox/test_update.py
> +++ bzrlib/tests/blackbox/test_update.py
> @@ -34,7 +34,7 @@
> def test_update_up_to_date_checkout(self):
> self.make_branch_and_tree('branch')
> self.runbzr('checkout branch checkout')
> - out, err = self.runbzr('update branch')
> + out, err = self.runbzr('update checkout')
> self.assertEqual('Tree is up to date.\n', err)
> self.assertEqual('', out)
>
>
> === modified file 'bzrlib/tests/blackbox/test_upgrade.py'
> --- bzrlib/tests/blackbox/test_upgrade.py
> +++ bzrlib/tests/blackbox/test_upgrade.py
> @@ -23,20 +23,109 @@
> import bzrlib.bzrdir as bzrdir
> from bzrlib.tests import TestCaseWithTransport
> from bzrlib.transport import get_transport
> +import bzrlib.ui as ui
>
>
> -class TestUpgrade(TestCaseWithTransport):
> +class TestUIFactory(ui.UIFactory):
> + """A UI Factory which never captures its output.
> + """
> +
> + def note(self, fmt_string, *args, **kwargs):
> + """See progress.ProgressBar.note()."""
> + print fmt_string % args
> +
> + def progress_bar(self):
> + return self
> +
> + def update(self, message, count, total):
> + """See progress.ProgressBar.update()."""
> +
> +
This looks more like debugging code, rather than something we want to
stay. Is that true?
...
> === modified file 'bzrlib/tests/test_ui.py'
> --- bzrlib/tests/test_ui.py
> +++ bzrlib/tests/test_ui.py
> @@ -18,8 +18,10 @@
> """
>
> import os
> +from StringIO import StringIO
> import sys
>
> +from bzrlib.progress import TTYProgressBar
> from bzrlib.tests import TestCase
> from bzrlib.ui import SilentUIFactory
> from bzrlib.ui.text import TextUIFactory
> @@ -50,3 +52,13 @@
> # user=u'some\u1234')
> # , 'bogus')
>
> +
> + def test_progress_note(self):
> + stderr = StringIO()
> + stdout = StringIO()
> + ui = TextUIFactory()
> + pb = TTYProgressBar(to_file=stderr, to_messages_file=stdout)
> + result = pb.note('t')
> + self.assertEqual(None, result)
> + self.assertEqual("t\n", stdout.getvalue())
> + self.assertEqual("\r \r", stderr.getvalue())
>
This looks like a syntax error. Which means the code is not being run.
> === modified file 'bzrlib/tests/test_upgrade.py'
...
> + def __init__(self, url, format):
> + self.format = format
> + self.bzrdir = BzrDir.open_unsupported(url)
> + if self.bzrdir.root_transport.is_readonly():
> + raise errors.UpgradeReadonly
> + self.transport = self.bzrdir.root_transport
> + self.convert()
I generally feel that having an action occur in a constructor is not
desirable.
Overall, it looks good. I'd like a few things addressed. But it looks
very close to mergeable.
John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060216/1666b135/attachment.pgp
More information about the bazaar
mailing list