[MERGE] Fix for bug 285055

[Andrew Bennetts]

Justyn Butler wrote:
> Hi,
> 
> 2008/10/22 Andrew Bennetts <andrew.bennetts at canonical.com>:
> > The first test is to make sure that this patch actually works — i.e. that when
> > the force flag is used that switch in a lightweight checkout will work anyway,
> > normally.  So, the test would do these steps, roughly:
> >
> >  * make a branch (with at least one commit in it)
> >  * make a checkout of the branch
> >  * rename the branch
> >  * switch with force=True to the branch's new location
> >
> > That test should fail without your patch, but pass with it.
> 
> I'm not sure about this - because the test will have to checkout from
> a branch on the local machine and will therefore not be affected by
> the bug, or the patch.

Oh, right, I forgot about that.  You can run a server in a test; for example
bzrlib.smart.server has a SmartTCPServer_for_testing, so a test can do:

    smart_server = SmartTCPServer_for_testing()
    smart_server.setUp(self.get_server())
    addCleanup(self.smart_server.tearDown)
    url = smart_server.get_url()

I've checked and a TCP bzr:// server such as the one that creates does reproduce
your problem.

[...]
> > All that said, looking closer, I'm inclined to this that this ought to Just Work
> > without --force.  Two ideas spring to mind:
> >
> >  1. The switch code should be catching and treating connection errors the same
> >    way it treats NotBranchError.
> 
> In this case presumably it would still fail without --force, but with
> a sensible error, just as if it could not find the branch on the local
> file system.

Right.

Although I'm tempted to say it should go further and that it should just work
without --force, at least if the target's repository has all the necessary
revisions.  In theory it'd be possible to check for pending merges in a working
tree (i.e. get the tree's parent IDs) without needing to open the corresponding
branch/repository, it's just that we lack an API to do so.  But it's not a big
deal to require --force for users to skip this.

> >  2. The switch code arguably shouldn't even be trying to use the source
> >    location if the target location already has all the required data.
> 
> Personally I feel that --force should simply skip any "uncommitted
> change" checks (as appears to be incorrectly suggested in the doc
> string) regardless of the location. But this would change existing
> behaviour of course.

Hmm, me too.  If the target repository has all the necessary revisions, then I
don't know why bzr should disallow the switch.  I guess there's scope for
confusion if sometimes pending merges prevent a switch and sometimes don't, but
then sometimes even without a pending merge the tree's parent might not be in
the target.

So rather than simply bailing if there are pending merges, I think it should
determine what the get_parent_ids() are and then check that they are present in
the to_branch's repo.

> My worry about connecting to remote branches is that it can take a
> long time to fail if there is a problem with the server/connection.
> Occasionally the login for a remote branch can take an absurd amount
> of time to complete due to non-bzr problems. There has to be a way to
> avoid any connection being attempted when you are trying to switch the
> branch away from the problem.

I agree.  Well, you could always change the .bzr/branch/location file
directly... but it would be better if bzr was more helpful.

> > Neither of these trivial to implement, so in the meantime I think your patch is
> > still a good idea! :)
> 
> I'll hold off trying to write tests until we've decided what needs to be done.

Fair enough.

-Andrew.