[MERGE] Switch nick too

[Marius Kruger]

Hi peoples,
I think/hope I've now implemented what you wanted.
Sjo, what a workout!

2008/9/29 Marius Kruger <amanic at gmail.com>
...

> I thought about it a little more and read Robert comments on the bug (
> https://bugs.launchpad.net/bugs/230903).
> And I think I finally understand what you have been saying:
>
> Robert Collins wrote:
> Some notes -
>  - implicit nicks should be drawing from the branch anyway
>
> Done: checkouts without explicit nicks, use the nick of their master

>
>  - the implicit nick of a bound branch should draw from the url of the
> master
>
> This seems contradictory to the first note: I'm using the nick of the
master, and if the master does not have an explicit nick, then it uses its
folder name. I might be misunderstanding what you are trying to say.


>  - if there is an explicit nick of a bound branch, it should be changed
> to the nick of the new master when switching
>
> Done

> (and probably done at
> bind() time for consistency)
>
> Done, at no extra charge.

>
>  - switching should not create an explicit nick where there was none
> before
>
> Done

>
> So if and only if there is an explicit nick already set, it should it set
> the nick explicitly again.
> Is there a way to unset the nick?
>
>  this question remains ^^^. Maybe we can add `nick --unset`, but I'll
leave that for another
bundle, this one is getting bloated already.



When running the test suite the following failed since when committing on
bound branches we
now need to hit the master to obtain the nick first:
self.assertEquals(1, len(self.connections))

Vincent Ladeuil's comment on a previous review, helped me to understand why
this happens, and why to fix it:

...
The purpose of the test is to check that a single
connection to the server is used, for performance reasons but
also because the user will be prompted for its password again
otherwise.
..


I would not have had to jump through so many hoops if we were memoising the
master
as this comment suggests:
    def get_master_branch(self, possible_transports=None):
        """Return the branch we are bound to.

        :return: Either a Branch, or None

        This could memoise the branch, but if thats done
        it must be revalidated on each new lock.
        So for now we just don't memoise it.
        # RBC 20060304 review this decision.
        """



With that sorted, I had a bigger problem:
Because for checkouts we now always need to hit the master
to obtain the nick, we couldn't do truly local commits any more,
which was fortunately caught by the following test:

    def test_local_commit_ignores_master(self):
        # a --local commit does not require access to the master branch
        # at all, or even for it to exist.
        # we test this by setting up a bound branch and then corrupting
        # the master.

I solved this by silently falling back to the normal implicit nick if we
can't connect to the master. I think this is a good solution given
the alternatives. I wanted to only catch BoundBranchConnectionFailure,
but our test suite forces a UnknownFormatError, which made me
think that there might be other cases where we might want to
be able to recover for nick's sake.


-- 
regards
Marius
<><
<http://amanica.blogspot.com/>Its 03:27 already, its becoming a bad habbit.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20081004/f12de78f/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: switch_nick4.patch
Type: text/x-patch
Size: 22103 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20081004/f12de78f/attachment-0001.bin