[MERGE][Bug #64783] Fixing check to understand the split-up .bzr format

[John Arbash Meinel]

John Arbash Meinel has voted tweak.
Status is now: Semi-approved
Comment:
v- You need 2 blank spaces here:
+        tree.unlock()
+
+def _get_elements(path):
+    try:
+        tree = WorkingTree.open(path)
+    except (errors.NoWorkingTree, errors.NotLocalUrl):
+        tree = None
+    except errors.NotBranchError:
+        raise errors.NotVersionedError(path)
+
+    try:
+        repo = Repository.open(path)
+    except errors.NoRepositoryPresent:
+        repo = None
+    except errors.NotBranchError:
+        raise errors.NotVersionedError(path)
+
+    try:
+        branch = Branch.open_containing(path)[0]
+    except errors.NotBranchError:
+        branch = None
+
+    return tree, repo, branch

^- There are some oddities in this function. To start with, we almost 
always try Tree then Branch then Repository. And we would typically do 
something like:

try:
   tree = workingtree.WorkingTree.open(path)
except ...:
   pass
else:
   return tree, tree.branch, tree.branch.repository

try:
   b = branch.Branch.open(path)
except ...:
   pass
else:
   return None, b, b.repository

...

etc.

Also, you are trying WT.open() but Branch.open_containing().
If anything, what you want is WT.open_containing() and Branch.open().
Personally, I think you only want open_containing() if path == '.'. But 
maybe that is just me.

There is also a BzrDir.open_tree_or_branch or 
open_containing_tree_or_branch. Though it doesn't fall back to 
repository if the others aren't available.


+def _check_working_tree(tree):

This should be rewritten to be WT.check(). The default implementation 
can probably be what you have written.

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C20080226155240.1f2f0145%40lapbert.oxbridgetech%3E