[MERGE] migrate switch command into the core

John Arbash Meinel john at arbash-meinel.com
Fri Nov 16 17:12:46 GMT 2007


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

Aaron Bentley wrote:
> bb:resubmit
> 
> Ian Clatworthy wrote:
>> I've added their
>> names to the NEWS item but removed their names from the file headers for
>> the sake of standardising the copyright header.
> 
> I think you should be a bit less cavalier about this.  Socially, it's
> just polite to explicitly ask someone before you reassign their
> copyright.  Legally, I think you're required to.  Of course, I'm happy
> to reassign my copyright.
> 
> I think you should also retain the 2006 copyright date, i.e. it should
> read: "Copyright (C) 2006, 2007 Canonical Ltd."
> 
>> +def _update(tree, source_repository, to_branch):
>> +    tree.lock_tree_write()
>> +    try:
>> +        if tree.last_revision() == tree.branch.last_revision():
>> +            note("Tree is up to date.")
>> +            return
> 
> ^^^ something wacky here; if to_branch == tree.branch, why do we need both?

I agree. Looking closely at the code, to_branch is not needed. Specifically the
code is:
    set_branch_location(control_dir, to_branch)
    tree = control_dir.open_workingtree()
    _update(tree, source_repository, to_branch)

set_branch_location() sneaks in and directly writes to '.bzr/branch/location'.
So that when we call control_dir.open_workingtree() it is now pointing at the
new branch location. So we shouldn't need to_branch at all.

What I would like to see is a way to do:

base_tree = tree.basis_tree()

Which is valid for DirState trees even after having the branch pointer moved
(and should be faster than extracting it from the repository). We would want to
be careful, because pre-dirstate goes to the repository, and it may not be
available in the new branch.

But it would be really nice if we could make 'bzr switch' super fast. I also
wonder about the new "update_basis_by_delta". Merge3Merger has already had to
do all the work to figure out the difference between the old basis_tree() and
the new basis_tree(). And I know that 'commit' performance got a lot better
using update_basis_by_delta. (Mostly because it doesn't have to loop over the
whole tree to figure out set_parent_trees()).

I'm also a little concerned about what 'bzr switch' will do in a tree with a
pending merge. It looks safe when there are uncommitted changes (since it does
a merge with the working tree rather than just overwriting it.) But do we leave
the pending merge? Do we fail if there is a pending merge?

It would be nice if switch could handle a pending merge, so that after you do
the merge you can realize... oh, I need to be on a different branch. But I'm
always a bit leery of merging into an existing merge. So I would be willing to
be conservative and have them revert the pending merge first.

Whichever way it goes, we should at least have a test case for it, so that our
behavior isn't random.

> 
>> +def set_branch_location(control, to_branch):
>> +    """Set location value of a branch reference.
>> +
>> +    :param control: BzrDir containing the branch reference
>> +    :param location: value to write to the branch reference location.
>> +    """
>> +    _check_switch_branch_format(control)
>> +    branch_format = BranchFormat.find_format(control)
> 
> ^^^ branch_format appears unused.


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

iD8DBQFHPc+OJdeBCYSNAAMRAvPRAJ9VMy09q1Rhg4X+SQHQfsU/vDzKUwCfTWiw
9Vqag7/gysbiW2k+AHIPU78=
=XL8r
-----END PGP SIGNATURE-----



More information about the bazaar mailing list