[MERGE][bug #111702] bzr init <remote_branch> connects multiple times

[John Arbash Meinel]

John Arbash Meinel has voted +1 (conditional).
Status is now: Conditionally approved
Comment:
I wish there were a better way than having to pass possible transports 
around through all the different api layers, but the only obvious one is 
to use a cache, and that seems to be very frowned upon.


This looks incorrectly indented:
+    * ``bzr init`` should only connect to the remote location one time.
+      We have been connecting several times because we forget to pass
+      around the Transport object. This modifies
+      ``BzrDir.create_branch_convenience``, so that we can pass in the
+      Transport that we already have.
+     (John Arbash Meinel, Vincent Ladeuil, #111702)
+


I realize this isn't your code but this is incorrect:
-            t = get_transport(safe_unicode(base))
+            t = get_transport(safe_unicode(base), possible_transports)

get_transport() isn't defined anymore as strictly requiring a Unicode 
path. It is actually specifically defined as taking a URL or trying to 
be smart if it gets a unicode path. (If it thinks it is a URL it tries 
to encode it, if it is a local path it treats it appropriately).

So I would just remove the "safe_unicode()" calls. There are other 
places as well.

This loop could be clearer:
+    transport = None
+    if possible_transports:
+        for t in possible_transports:
+            if t.base == base:
+                transport = t
+                break
+    if transport is None:
...
+    if transport is None:
...
+    return transport

Instead, you could just do:
+    if possible_transports:
+        for t in possible_transports:
+            if t.base == base:
+                return t
+                break

Then you don't need the 'if transport is None' checks.


Also, the way you have written this, the transport will only be re-used 
if it is identically the same. I thought we were trying to also handle 
cases when it is a parent/child. Or is that something you are still 
working on?

And I don't think this works for local relative paths. Because T.base is 
expanded to the absolute path, rather than the relative path that was 
passed in. (unless I misread it and the path has already been converted 
to a full url).


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3Cm2wszoq317.fsf%40free.fr%3E