[MERGE] checkout
John A Meinel
john at arbash-meinel.com
Mon Feb 13 00:19:41 GMT 2006
Robert Collins wrote:
> On Sun, 2006-02-12 at 17:18 -0600, John A Meinel wrote:
>> My mail client sucks, so I lost the text of your message.
>> But I checked out your branch. And it seems to depend on your BzrDir
>> work. So can we wait for that to land before I evaluate checkouts?
>> Otherwise it is difficult to find the checkout changes.
>
> It does indeed depend on that work.... I'm attaching a diff of the
> checkout work - if you are happy with that I'll merge it into the
> bzr-dir branch, and they can come in together.
>
Reviewing... (Do you want to merge in the BoundBranch work as well? The
only thing it was waiting on was a new branch format. I can add it once
BzrDir lands, though)
> bzr diff -r ancestor:../bzr-dir | wc -l
> 1632
> robertc at lifelesslap:~/source/baz/checkout$ bzr diff -r
> ancestor:../bzr-dir | diffstat
> add.py | 2
> branch.py | 6
> builtins.py | 84 ++++++++-
> bzrdir.py | 31 ++-
> commit.py | 7
> diff.py | 7
> errors.py | 5
> info.py | 2
> status.py | 5
> tests/blackbox/__init__.py | 3
> tests/blackbox/test_checkout.py | 55 ++++++
> tests/blackbox/test_commit.py | 84 +++++++++
> tests/blackbox/test_too_much.py | 36 ---
> tests/blackbox/test_update.py | 97 ++++++++++
> tests/blackbox/test_versioning.py | 10 -
> tests/bzrdir_implementations/test_bzrdir.py | 21 +-
> tests/test_inv.py | 67 +++----
> tests/test_merge.py | 2
> tests/test_merge_core.py | 37 ++--
> tests/test_permissions.py | 8
> tests/test_revision.py | 4
> tests/test_source.py | 2
> tests/test_workingtree.py | 10 -
> tests/workingtree_implementations/test_workingtree.py | 129
> ++++++++++++--
> workingtree.py | 164
> ++++++++++++++----
> 25 files changed, 685 insertions(+), 193 deletions(-)
>
>
> Rob
>
>
>
> ------------------------------------------------------------------------
>
> === added file 'bzrlib/tests/blackbox/test_checkout.py'
> --- /dev/null
> +++ bzrlib/tests/blackbox/test_checkout.py
> @@ -0,0 +1,55 @@
> +# Copyright (C) 2005 by Canonical Ltd
> +# -*- coding: utf-8 -*-
> +
> +# 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +"""Tests for the 'checkout' CLI command."""
> +
> +from cStringIO import StringIO
> +import os
> +import re
> +import shutil
> +import sys
> +
> +import bzrlib.bzrdir as bzrdir
> +from bzrlib.tests.blackbox import ExternalBase
> +
> +
> +class TestCheckout(ExternalBase):
> +
> + def setUp(self):
> + super(TestCheckout, self).setUp()
> + tree = bzrdir.BzrDir.create_standalone_workingtree('branch')
> + tree.commit('1', rev_id='1', allow_pointless=True)
> + self.build_tree(['branch/added_in_2'])
> + tree.add('added_in_2')
> + tree.commit('2', rev_id='2')
> +
> + def test_checkout_makes_checkout(self):
> + self.runbzr('checkout branch checkout')
> + # if we have a checkout, the branch base should be 'branch'
> + source = bzrdir.BzrDir.open('branch')
> + result = bzrdir.BzrDir.open('checkout')
> + self.assertEqual(source.open_branch().bzrdir.root_transport.base,
> + result.open_branch().bzrdir.root_transport.base)
> +
> + def test_checkout_dash_r(self):
> + self.runbzr('checkout -r -2 branch checkout')
> + # the working tree should now be at revision '1' with the content
> + # from 1.
> + result = bzrdir.BzrDir.open('checkout')
> + self.assertEqual('1', result.open_workingtree().last_revision())
> + self.failIfExists('checkout/added_in_2')
> +
>
I would tend to write this as '-r 1' rather than '-r -2'. Not a big
deal, though.
...
...
> + 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')
> + self.assertEqual('Tree is up to date.\n', err)
> + self.assertEqual('', out)
Don't you need to 'chdir("checkout")' here?
What is your concept for update. Does it take a local path to update, or
a remote path to update from. (Obviously you have it written as a local
path to update).
> +
> + def test_update_out_of_date_standalone_tree(self):
> + # FIXME the default format has to change for this to pass
> + # because it currently uses the branch last-revision marker.
> + raise TestSkipped('default format too old')
> + self.make_branch_and_tree('branch')
> + # make a checkout
> + self.runbzr('checkout branch checkout')
> + self.build_tree(['checkout/file'])
> + self.runbzr('add checkout/file')
> + self.runbzr('commit -m add-file checkout')
> + # now branch should be out of date
> + out,err = self.runbzr('update branch')
> + self.assertEqual('Updated to revision 1.\n', out)
> + self.assertEqual('', err)
> + self.failUnlessExists('branch/file')
I thought your BzrDir code implemented last_revision for working trees.
> +
> + def test_update_out_of_date_checkout(self):
> + self.make_branch_and_tree('branch')
> + # make two checkouts
> + self.runbzr('checkout branch checkout')
> + self.runbzr('checkout branch checkout2')
> + self.build_tree(['checkout/file'])
> + self.runbzr('add checkout/file')
> + self.runbzr('commit -m add-file checkout')
> + # now checkout2 should be out of date
> + out,err = self.runbzr('update checkout2')
> + self.assertEqual('All changes applied successfully.\n'
> + 'Updated to revision 1.\n',
> + err)
> + self.assertEqual('', out)
Shouldn't this also check that 'file' exists in checkout2?
> +
> + def test_update_conflicts_returns_2(self):
> + self.make_branch_and_tree('branch')
> + # make two checkouts
> + self.runbzr('checkout branch checkout')
> + self.build_tree(['checkout/file'])
> + self.runbzr('add checkout/file')
> + self.runbzr('commit -m add-file checkout')
> + self.runbzr('checkout branch checkout2')
> + # now alter file in checkout
> + a_file = file('checkout/file', 'wt')
> + a_file.write('Foo')
> + a_file.close()
> + self.runbzr('commit -m checnge-file checkout')
> + # now checkout2 should be out of date
> + # make a local change to file
> + a_file = file('checkout2/file', 'wt')
> + a_file.write('Bar')
> + a_file.close()
> + out,err = self.runbzr('update checkout2', retcode=1)
> + self.assertEqual(['1 conflicts encountered.',
> + 'Updated to revision 2.'],
> + err.split('\n')[1:3])
> + self.assertContainsRe(err, 'Diff3 conflict encountered in.*file\n')
> + self.assertEqual('', out)
> + self.failUnlessExists('checkout2/file')
> +
I think this failUnless is in the wrong function. Since you want to
check that file is conflicted here. Not that it just exists.
>
> === modified file 'bzrlib/add.py'
> --- bzrlib/add.py
> +++ bzrlib/add.py
> @@ -130,7 +130,7 @@
>
> if kind == 'directory':
> try:
> - sub_branch = WorkingTree(af)
> + sub_branch = WorkingTree.open(af)
> sub_tree = True
> except NotBranchError:
> sub_tree = False
>
> === modified file 'bzrlib/branch.py'
> --- bzrlib/branch.py
> +++ bzrlib/branch.py
> @@ -234,10 +234,10 @@
> >>> bzrlib.trace.silent = True
> >>> d1 = bzrdir.ScratchDir()
> >>> br1 = d1.open_branch()
> - >>> wt1 = WorkingTree(br1.base, br1)
> + >>> wt1 = d1.open_workingtree()
> >>> d2 = bzrdir.ScratchDir()
> >>> br2 = d2.open_branch()
> - >>> wt2 = WorkingTree(br2.base, br2)
> + >>> wt2 = d2.open_workingtree()
> >>> br1.missing_revisions(br2)
> []
> >>> wt2.commit("lala!", rev_id="REVISION-ID-1")
> @@ -1006,7 +1006,7 @@
> if (self.base.find('://') != -1 or
> not isinstance(self._transport, LocalTransport)):
> raise NoWorkingTree(self.base)
> - return WorkingTree(self.base, branch=self)
> + return self.bzrdir.open_workingtree()
>
> @needs_write_lock
> def pull(self, source, overwrite=False):
>
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py
> +++ bzrlib/builtins.py
> @@ -17,10 +17,13 @@
> """builtin bzr commands"""
>
>
> +import errno
> import os
> +from shutil import rmtree
> import sys
>
> import bzrlib
> +import bzrlib.branch as branch
This seems very dangerous, since we frequently name our variables "branch".
> import bzrlib.bzrdir as bzrdir
> from bzrlib._merge_core import ApplyMerge3
> from bzrlib.commands import Command, display_command
> @@ -396,8 +399,6 @@
> takes_args = ['location?']
>
> def run(self, location=None, remember=False, overwrite=False, verbose=False):
> - from shutil import rmtree
> - import errno
> # FIXME: too much stuff is in the command class
> tree_to = WorkingTree.open_containing(u'.')[0]
> stored_loc = tree_to.branch.get_parent()
> @@ -459,8 +460,6 @@
> create_prefix=False, verbose=False):
> # FIXME: Way too big! Put this into a function called from the
> # command.
> - import errno
> - from shutil import rmtree
> from bzrlib.transport import get_transport
>
> tree_from = WorkingTree.open_containing(u'.')[0]
> @@ -547,8 +546,6 @@
> aliases = ['get', 'clone']
>
> def run(self, from_location, to_location=None, revision=None, basis=None):
> - import errno
> - from shutil import rmtree
> if revision is None:
> revision = [None]
> elif len(revision) > 1:
> @@ -602,13 +599,59 @@
> rmtree(to_location)
> msg = "The branch %s cannot be used as a --basis"
> raise BzrCommandError(msg)
> - WorkingTree.create(branch, to_location)
> if name:
> branch.control_files.put_utf8('branch-name', name)
>
> note('Branched %d revision(s).' % branch.revno())
> finally:
> br_from.unlock()
> +
> +
> +class cmd_checkout(Command):
> + """Create a new checkout of an existing branch.
> +
> + If the TO_LOCATION is omitted, the last component of the BRANCH_LOCATION will
> + be used. In other words, "checkout ../foo/bar" will attempt to create ./bar.
> +
> + To retrieve the branch as of a particular revision, supply the --revision
> + parameter, as in "checkout foo/bar -r 5". Note that this will be immediately
> + out of date [so you cannot commit] but it may be useful (i.e. to examine old
> + code.)
> +
> + --basis is to speed up checking out from remote branches. When specified, it
> + uses the inventory and file contents from the basis branch in preference to the
> + branch being checked out. [Not implemented yet.]
> + """
> + takes_args = ['branch_location', 'to_location?']
> + takes_options = ['revision'] # , 'basis']
> +
> + def run(self, branch_location, to_location=None, revision=None, basis=None):
> + if revision is None:
> + revision = [None]
> + elif len(revision) > 1:
> + raise BzrCommandError(
> + 'bzr checkout --revision takes exactly 1 revision value')
> + source = Branch.open(branch_location)
> + if len(revision) == 1 and revision[0] is not None:
> + revision_id = revision[0].in_history(source)[1]
> + else:
> + revision_id = None
> + if to_location is None:
> + to_location = os.path.basename(branch_location.rstrip("/\\"))
> + try:
> + os.mkdir(to_location)
> + except OSError, e:
> + if e.errno == errno.EEXIST:
> + raise BzrCommandError('Target directory "%s" already'
> + ' exists.' % to_location)
> + if e.errno == errno.ENOENT:
> + raise BzrCommandError('Parent of "%s" does not exist.' %
> + to_location)
> + else:
> + raise
> + checkout = bzrdir.BzrDirMetaFormat1().initialize(to_location)
> + branch.BranchReferenceFormat().initialize(checkout, source)
> + checkout.create_workingtree(revision_id)
>
I think this would be clearer if we just used
bzrlib.branch.BranchReference...
>
> class cmd_renames(Command):
> @@ -629,6 +672,33 @@
> renames.sort()
> for old_name, new_name in renames:
> print "%s => %s" % (old_name, new_name)
> +
> +
> +class cmd_update(Command):
> + """Update a tree to have the latest code committed to its branch.
> +
> + This will perform a merge into the working tree, and may generate
> + conflicts. If you have any uncommitted changes, you will still
> + need to commit them after the update.
> + """
> + takes_args = ['dir?']
> +
> + def run(self, dir='.'):
> + tree = WorkingTree.open_containing(dir)[0]
> + tree.lock_write()
> + try:
> + if tree.last_revision() == tree.branch.last_revision():
> + note("Tree is up to date.")
> + return
> + conflicts = tree.update()
> + note('Updated to revision %d.' %
> + (tree.branch.revision_id_to_revno(tree.last_revision()),))
> + if conflicts != 0:
> + return 1
> + else:
> + return 0
> + finally:
> + tree.unlock()
Shouldn't this be using 'BzrDir.open_containing()' or something like
that? I thought BzrDir was replacing all of the other Branch/WorkingTree
factories.
>
>
> class cmd_info(Command):
>
> === modified file 'bzrlib/bzrdir.py'
> --- bzrlib/bzrdir.py
> +++ bzrlib/bzrdir.py
> @@ -76,7 +76,7 @@
> pass
> try:
> self.open_workingtree().clone(result, basis=basis_tree)
> - except (errors.NotBranchError, errors.NotLocalUrl):
> + except (errors.NoWorkingTree, errors.NotLocalUrl):
> pass
> return result
>
> @@ -187,8 +187,11 @@
> bzrdir = BzrDir.create_branch_and_repo(safe_unicode(base)).bzrdir
> return bzrdir.create_workingtree()
>
> - def create_workingtree(self):
> - """Create a working tree at this BzrDir"""
> + def create_workingtree(self, revision_id=None):
> + """Create a working tree at this BzrDir.
> +
> + revision_id: create it as of this revision id.
> + """
> raise NotImplementedError(self.create_workingtree)
>
> def get_branch_transport(self, branch_format):
> @@ -364,7 +367,7 @@
> self.open_workingtree().clone(result,
> revision_id=revision_id,
> basis=basis_tree)
> - except (errors.NotBranchError, errors.NotLocalUrl):
> + except (errors.NoWorkingTree, errors.NotLocalUrl):
> result.create_workingtree()
> return result
>
> @@ -395,9 +398,19 @@
> """See BzrDir.create_repository."""
> return self.open_repository()
>
> - def create_workingtree(self):
> + def create_workingtree(self, revision_id=None):
> """See BzrDir.create_workingtree."""
> - return self.open_workingtree()
> + # this looks buggy but is not -really-
> + # clone and sprout will have set the revision_id
> + # and that will have set it for us, its only
> + # specific uses of create_workingtree in isolation
> + # that can do wonky stuff here, and that only
> + # happens for creating checkouts, which cannot be
> + # done on this format anyway. So - acceptable wart.
> + result = self.open_workingtree()
> + if revision_id is not None:
> + result.set_last_revision(revision_id)
> + return result
Shouldn't clone & sprout take a last_revision parameter, rather than
sprouting and then setting the revision?
...
> +
> +
> +class OutOfDateTree(BzrNewError):
> + """Working tree is out of date, please run 'bzr update'."""
This should probably take a path defining what is out of date. (It may
not print it, but it should have it internally for people catching this
error).
>
> === modified file 'bzrlib/info.py'
> --- bzrlib/info.py
> +++ bzrlib/info.py
> @@ -47,7 +47,7 @@
>
> count_version_dirs = 0
>
> - working = WorkingTree(b.base, b)
> + working = b.bzrdir.open_workingtree()
> basis = working.basis_tree()
> work_inv = working.inventory
> delta = diff.compare_trees(basis, working, want_unchanged=True)
Definitely b.bzrdir.open_workingtree() is better than WorkingTree(b.base).
...
A lot of these changes look like cleanups that should happen anyway.
(WorkingTree() => WorkingTree.open())
...
>
> === modified file 'bzrlib/tests/test_source.py'
> --- bzrlib/tests/test_source.py
> +++ bzrlib/tests/test_source.py
> @@ -72,4 +72,4 @@
> # written. Note that this is an exact equality so that when the number
> # drops, it is not given a buffer but rather this test updated
> # immediately.
> - self.assertEqual(9, occurences)
> + self.assertEqual(6, occurences)
Nice to see the number go down. :)
...
>
> import bzrlib
> +import bzrlib.branch as branch
> from bzrlib.branch import Branch
> import bzrlib.bzrdir as bzrdir
> from bzrlib.bzrdir import BzrDir
Again, I would strongly discourange "import bzrlib.branch as branch"
.
...
> + def test_update_sets_last_revision(self):
> + # working tree formats from the meta-dir format and newer support
> + # setting the last revision on a tree independently of that on the
> + # branch. Its concievable that some future formats may want to
> + # couple them again (i.e. because its really a smart server and
> + # the working tree will always match the branch). So we test
> + # that formats where initialising a branch does not initialise a
> + # tree - and thus have separable entities - support skewing the
> + # two things.
Since we disabled using the doc string instead of the test path when
using --verbose, we can use docstrings instead of comments. (There are
other tests you added which can be changed as well)
...
> === modified file 'bzrlib/workingtree.py'
> --- bzrlib/workingtree.py
> +++ bzrlib/workingtree.py
> @@ -79,6 +79,7 @@
> realpath,
> relpath,
> rename)
> +from bzrlib.revision import NULL_REVISION
> from bzrlib.symbol_versioning import *
> from bzrlib.textui import show_status
> import bzrlib.tree
> @@ -188,7 +189,7 @@
> """
>
> def __init__(self, basedir='.',
> - branch=None,
> + branch=DEPRECATED_PARAMETER,
> _inventory=None,
> _control_files=None,
> _internal=False,
> @@ -204,7 +205,11 @@
> self._format = _format
> self.bzrdir = _bzrdir
> if not _internal:
> - # created via open etc.
> + # not created via open etc.
> + warn("WorkingTree() is deprecated ass of bzr version 0.8. "
> + "Please use bzrdir.open_workingtree or WorkingTree.open().",
> + DeprecationWarning,
> + stacklevel=2)
Is this a Freudian slip? I think you want *as*.
> wt = WorkingTree.open(basedir)
> self.branch = wt.branch
> self.basedir = wt.basedir
> @@ -219,11 +224,18 @@
> "base directory %r is not a string" % basedir
> basedir = safe_unicode(basedir)
> mutter("openeing working tree %r", basedir)
> - if branch is None:
> - branch = Branch.open(basedir)
> - assert isinstance(branch, Branch), \
> - "branch %r is not a Branch" % branch
> - self.branch = branch
> + if deprecated_passed(branch):
> + if not _internal:
> + warn("WorkingTree(..., branch=XXX) is deprecated as of bzr 0.8."
> + " Please use bzrdir.open_workingtree().",
> + DeprecationWarning,
> + stacklevel=2
> + )
> + self.branch = branch
> + else:
> + self.branch = self.bzrdir.open_branch()
> + assert isinstance(self.branch, Branch), \
> + "branch %r is not a Branch" % self.branch
This error supports my earlier comment that you should use BzrDir
instead of WorkingTree.open()
> self.basedir = realpath(basedir)
> # if branch is at our basedir and is a format 6 or less
> if isinstance(self._format, WorkingTreeFormat2):
> @@ -779,7 +791,7 @@
> >>> from bzrlib.bzrdir import ScratchDir
> >>> d = ScratchDir(files=['foo', 'foo~'])
> >>> b = d.open_branch()
> - >>> tree = WorkingTree(b.base, b)
> + >>> tree = d.open_workingtree()
> >>> map(str, tree.unknowns())
> ['foo']
> >>> tree.add('foo')
> @@ -919,6 +931,7 @@
> def kind(self, file_id):
> return file_kind(self.id2abspath(file_id))
>
> + @needs_read_lock
> def last_revision(self):
> """Return the last revision id of this working tree.
>
> @@ -949,7 +962,38 @@
> def _basis_inventory_name(self, revision_id):
> return 'basis-inventory.%s' % revision_id
>
> + @needs_write_lock
> def set_last_revision(self, new_revision, old_revision=None):
> + """Change the last revision in the working tree."""
> + self._remove_old_basis(old_revision)
> + if self._change_last_revision(new_revision):
> + self._cache_basis_inventory(new_revision)
> +
> + def _change_last_revision(self, new_revision):
> + """Template method part of set_last_revision to perform the change."""
> + if new_revision is None:
> + self.branch.set_revision_history([])
> + return False
> + # current format is locked in with the branch
> + revision_history = self.branch.revision_history()
> + try:
> + position = revision_history.index(new_revision)
> + except ValueError:
> + raise errors.NoSuchRevision(self.branch, new_revision)
> + self.branch.set_revision_history(revision_history[:position + 1])
> + return True
> +
> + def _cache_basis_inventory(self, new_revision):
> + """Cache new_revision as the basis inventory."""
> + try:
> + xml = self.branch.repository.get_inventory_xml(new_revision)
> + path = self._basis_inventory_name(new_revision)
> + self._control_files.put_utf8(path, xml)
> + except WeaveRevisionNotPresent:
> + pass
> +
> + def _remove_old_basis(self, old_revision):
> + """Remove the old basis inventory 'old_revision'."""
Can we merge my basis-inventory changes so that we don't have this
_basis_inventory_name stuff?
(They were also waiting on a BranchFormat which would delete
ancestry.weave and any basis-inventory.* files).
...
> +
> +
> +class WorkingTree3(WorkingTree):
> + """This is the Format 3 working tree.
> +
> + This differs from the base WorkingTree by:
> + - having its own file lock
> + - having its own last-revision property.
> + """
> +
> + @needs_read_lock
> + def last_revision(self):
> + """See WorkingTree.last_revision."""
> + try:
> + return self._control_files.get_utf8('last-revision').read()
> + except NoSuchFile:
> + return None
> +
> + def _change_last_revision(self, revision_id):
> + """See WorkingTree._change_last_revision."""
> + if revision_id is None or revision_id == NULL_REVISION:
> + try:
> + self._control_files._transport.delete('last-revision')
> + except errors.NoSuchFile:
> + pass
> + return False
> + else:
> + try:
> + self.branch.revision_history().index(revision_id)
> + except ValueError:
> + raise errors.NoSuchRevision(self.branch, revision_id)
> + self._control_files.put_utf8('last-revision', revision_id)
> + return True
> +
It would be nice to at least mark with a comment when WorkingTree2
format will be expired. (It also will help us remember to remove it when
the time comes).
...
So, I'll give you a +1 to merge this into BzrDir as long as you address
my comments. (Though please wait to do so. So we don't have to review it
again to allow BzrDir to land).
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/20060212/df63d2eb/attachment.pgp
More information about the bazaar
mailing list