[MERGE REQUEST] Couple of small fixes

[Jelmer Vernooij]

Hi John,

On Thu, Dec 15, 2005 at 08:57:57AM -0600, John Arbash Meinel wrote about 'Re: [MERGE REQUEST] Couple of small fixes':
> Jelmer Vernooij wrote:
> > I don't really see how this makes opening containing branches slower.

> > There are two approaches that can used for 'open_containing': 
> > depth-first and breadth-first search for a Branch. I've implemented
> > depth-first search because it does not make opening bzr branches
> > slower even if you have a several foreign branch classes loaded (since
> > it will try Bzr first).
> The big problem is it may mean trying to reconnect to a remote host
> multiple times.

> At the point you have, 'url' is simply a string. While a Transport
> frequently has an open connection.
I still use t.clone('..') to go up a level, just like happened in the
older code, so I don't see the problem. For example, if you run
Branch.open_containing('http://bazaar-ng.org/bzr/bzr.dev/bzrlib')
roughly the following will happen:

Branch.open_containing('http://bazaar-ng.org/bzr/bzr.dev/bzrlib')
 -> BzrBranch.open_containing('http://bazaar-ng.org/bzr/bzr.dev/bzrlib')
  -> t = get_transport('http://bazaar-ng.org/bzr/bzr.dev/bzrlib')
  -> # No .bzr subdir is found
  -> t = t.clone('..')
  -> # .bzr subdir is found -> BzrBranch object is returned

I don't see how this could be slower then what is happening at the
moment. When opening a SVN http URL, the following happens:

Branch.open_containing('http://anonsvn.ethereal.com/ethereal/trunk')
 -> BzrBranch.open_containing('http://anonsvn.ethereal.com/ethereal/trunk')
  -> t = get_transport('http://anonsvn.ethereal.com/ethereal/trunk')
  -> # No .bzr subdir is found
  -> t = t.clone('..')
  -> # No .bzr subdir is found 
  -> t = t.clone('..')
  -> # No .bzr subdir is found, root of URL is reached -> throws NotBranchError
 -> SvnBranch.open_containing('http://anonsvn.ethereal.com/ethereal/trunk')
  -> svn.connect('http://anonsvn.ethereal.com/ethereal/trunk') succeeds
  -> a SvnBranch object is returned

> > The code you are looking at here which searches for a .bzr branch by going down one level at a time was previously in Branch.open_containing. I moved it to 
> > BzrBranch since not all derived classes of Branch may be using Transport.

> Now, one thing I'm not sure about, though, is that if you are doing say
> a "svn" branch. Do you use svn:// notation, or are you just supplying a
> path, and recognizing it is an SVN branch because it has the appropriate
> .svn/_svn directory?
The latter. Branch classes throw a 'NotBranchError' if they don't support a 
particular URL so we just try to open with the available Branch
classes until we hit a success.

> 'svn://' notation would require to be parsed directly by the branch
> class, since we wouldn't register a tranport type for svn. The latter
> would work just fine with a simple Transport object.
The SvnBranch class does indeed accept 'svn://' URLs, and http URLs 
(if they contain a .svn subdirectory). Note that the SvnBranch does
not use the Transport object at all but uses the SVN standard
libraries to access these URLs.

Cheers,

Jelmer

-- 
Jelmer Vernooij <jelmer at samba.org> - http://jelmer.vernstok.nl/
 16:00:42 up 55 days,  5:05,  1 user,  load average: 0.18, 0.23, 0.16
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051215/f947fad9/attachment.pgp