re 0.15rc3/win32: bug report, suggestion and question

[John Arbash Meinel]

Martin Pool wrote:
> On 4/1/07, Andrew Voznytsa <andrew.voznytsa at gmail.com> wrote:
>> Hi all,
>>
>> I tried to use 0.15 with lightweight checkout (0.14) from shared
>> repository (0.14) and I got this:
...

>>   File "paramiko\sftp.pyc", line 183, in _read_packet
>> SFTPError: Garbage packet received
>>
>> bzr 0.15.0candidate3 on python 2.5.0.final.0 (win32)
>> arguments: ['bzr', 'co', 'sftp://username@host/repository/branch']
>>
>> Server is http://sshwindows.sourceforge.net, host WinXP Pro SP2,
>> repository - shared, no trees, 0.14 format. Working tree is lightweight
>> checkout (0.14 too).
>>
>> Note: ssh keys were regenerated after last checkout 0.14, before
>> upgrading to 0.15rc3.
>>
>> After that I used bzr 0.14. It said that keys mismatched and I should
>> edit here and there to make it working. I did it and bzr 0.14 worked
>> just fine. So I tried 0.15 again - it failed. Seems that SSH is a
>> problem.
> 
> I'm surprised that we're getting an error from that place inside
> paramiko if it's a bzr bug.  Could it be that your installation on
> Windows is using a different version of paramiko for the two
> installations of Bazaar?
> 

I think this is fairly likely. I would guess that he is using the
standalone installer, which means that it bundles a version of paramiko
with it. Probably the version bundled with 0.15 changed something
relative to the version bundled with 0.14. (I couldn't say what 'Garbage
Packet' is for, so I don't know if the new one is being more strict, or
just missing something).


>> OK, I decided to bzr 0.14 unbind/bind to UNC patch in hope to forget
>> about SSH - no luck. 0.14 (and 0.15) still ask for SSH password. I
>> looked in <branch>\.bzr\branch\location and there is still sftp URL so
>> it just silently exited from bind call, without any notifications that
>> bind failed or is not possible at all.
> 
> So you're trying to unbind from the ssh location and bind to a UNC
> location, and that fails because you can't reach the ssh server?

He has a lightweight checkout, so bind and unbind don't do anything. We
*could* make 'unbind' be a heavy operation which changes your
lightweight checkout into a heavy one, but I don't specifically
recommend that.


> 
>> Next, suggestion: why not to just allow rebind without connecting to
>> server? What if server is not (and won't) available anymore and mobile
>> user will have to use mirror? (checkout from server A to notebook;
>> server A crash; rebind to server B and use it. I agree that mobile user
>> won't use lightweight checkout, but anyway it would be useful)
> 
> I agree that if you're trying to unbind from a server we shouldn't
> talk to it anymore.  Is that what you're seeing?  It's more
> contentious whether we should talk to the new master when you connect,
> or only when you first try to update or commit.

That would happen for heavy checkouts, but not for lightweight ones.

It sounds like he is actually looking for 'bzr switch' which is provided
by the 'bzrtools' plugin. (Which IIRC is bundled with the windows
installer).

Right now if you have a heavy checkout, doing 'bzr bind
sftp://new/master' will effectively do 'switch', without connecting to
the existing master.

The way our internal model works, whenever you open a lightweight
checkout it always connects to the remote branch.

In internal details it is because WT.open() needs a self.branch() and
BranchReference.open() is designed to return the actual branch, rather
than proxying for the branch (possibly not actually connecting until the
first request needs it.)

I would like to see BranchReference.open() be a proxy, because it would
also help commands like 'bzr status', since they won't have to connect
to the remote branch at all.

It could be something as simple as:

def open(self, ...):
  self._location = (read .bzr/branch/location)
  self._real_branch = None
  return self

def __getattr__(self, attr):
  if self._real_branch is None:
    self._real_branch = branch.Branch.open(self._location)

  return getattr(self._real_branch, attr)

Also, internally we don't have a good way to tell if something is a
lightweight checkout, because we actually wanted to hide it from
higher-level code, and we probably hid it a bit too well.


That brings up another question, though. Would we want 'bzr bind' to
take over the 'bzr switch' ability? The biggest difference (right now)
is that 'bzr switch' automatically runs 'bzr update'. (Also, 'bzr
switch' is explicitly hard-coded to not work on heavyweight checkouts).

Thoughts?

John
=:->