[MERGE REQUEST] Couple of small fixes

[John Arbash Meinel]

Jelmer Vernooij wrote:
> 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).

Except for the fact that you probably need to do breadth-first search.
Because it is possible for me to have this arrangement:

base/
  .bzr/
  subproject/
    .svn/
    subsubproject/
      CVS
  otherproject/
    .git/


We may decide that we don't support the above configuration. But it is
certainly possible that I will have a master project which I am working
on in bzr, which is using a library from someone else who only wants to
use svn, and someone else who only uses CVS.

I don't think we *have* to support nested multi-project systems. But
since someone gave you a full path, you really should do breadth-first
search.

>>
>>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
> 

There are a few ways to handle the multi-connect issues.

I personally would rather see us change how we name branches, and go
with specific prefixes rather than try the same type multiple times.

To clarify: we would introduce bzr+file:// bzr+http:// bzr+sftp://, as
well as svn+http://, etc.
For compatibility (and the common use case), an plain path, http:// and
sftp:// are all treated with their bzr+ prefixes.

Then we would have a Transport list, but we would also have a Branch
registry.
So that rather than being able to say:

bzr log http://anonsvn.ethereal.com/foo
we would require
bzr log svn+http://anonsvn.ethereal.com/foo

The open* code can then do something like:

proto, * = urlparse.parse(url)
loc = proto.find('+')
if loc != -1:
  branch_type = proto[:loc]
  proto = proto[loc+1:]
else:
  branch_type = 'bzr'

if not branch_types.has_key(branch_type):
  raise UnsupportedBranchType()

Then we still can support alternative branches just by giving a prefix,
and we don't have to do a complete searching through all possible branch
types everytime we use a relative path.

At least that was how I was thinking about implementing it.

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051215/f7d7b919/attachment.pgp