[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