[MERGE] Fix for bug 285055
Andrew Bennetts
andrew.bennetts at canonical.com
Tue Oct 21 00:10:00 BST 2008
Justyn Butler wrote:
> This is a fix for bug #285055
> http://bugs.launchpad.net/bzr/+bug/285055
>
> It simply uses
>
> if (not force) | (control_dir.get_branch_reference().startswith('file://'))
>
> to check whether the force option has been used and whether the branch
> reference is on the local file system.
> If the force option *has* been used and the branch reference is *not*
> on the local file system it will skip checks for uncommitted changes.
bb:resubmit
This needs a test in bzrlib/tests/test_switch.py, I think. In fact, this patch
breaks several of the tests that are already there. The errors all look like
this:
ERROR: test_switch_after_branch_moved (bzrlib.tests.test_switch.TestSwitchHeavyweight)
----------------------------------------------------------------------
Traceback (most recent call last):
File "bzrlib/tests/test_switch.py", line 75, in test_switch_after_branch_moved
switch.switch, checkout.bzrdir, to_branch)
File "bzrlib/tests/__init__.py", line 966, in assertRaises
callableObj(*args, **kwargs)
File "bzrlib/switch.py", line 32, in switch
if (not force) | (control_dir.get_branch_reference().startswith('file://')):
AttributeError: 'NoneType' object has no attribute 'startswith'
======================================================================
You can run just the switch tests yourself with “bzr selftest -s
bt.test_switch”. You may find
<http://doc.bazaar-vcs.org/latest/developers/testing.html> to be helpful if you
haven't already seen it.
The tests I'd like to see added are:
* that switch with force=True works when the branch reference is absent (but
the revision_id/revision_ids of the checkout's parent trees are all still
accessible, i.e. they are present in the to_branch.repository)
* what happens when switch with force=True is attempted in a lightweight
checkout, but not all the revisions listed in checkout.get_parent_ids() are
present in the to_branch.repository. I guess the best we can do here is give
an error?
> === modified file 'bzrlib/switch.py'
> --- bzrlib/switch.py 2007-12-07 05:31:54 +0000
> +++ bzrlib/switch.py 2008-10-20 19:02:59 +0000
> @@ -29,10 +29,15 @@
> :param to_branch: branch that the checkout is to reference
> :param force: skip the check for local commits in a heavy checkout
> """
> - _check_pending_merges(control_dir, force)
> - try:
> - source_repository = control_dir.open_branch().repository
> - except errors.NotBranchError:
> + if (not force) | (control_dir.get_branch_reference().startswith('file://')):
> + # If force is not used, or associated branch is on local file system, perform checks.
“|” is actually the bit-wise OR operator, not boolean OR. Also, our coding
standard requires lines to be no more than 80 columns wide. And I don't think
the redundant parentheses add much here. So these two lines should be:
if not force or control_dir.get_branch_reference().startswith('file://'):
# If force is not used, or associated branch is on local file system,
# perform checks.
Thanks for taking the time to submit this patch!
-Andrew.
More information about the bazaar
mailing list