[RFC] Bug 440952: "be more helpful when attempting to branch a shared repo"

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
>>>>>> "Brian" == Brian de Alwis <bsd at cs.ubc.ca> writes:
> 
> <snip/>
> 
>     Brian> I was recently hit a similar situation, and was sufficiently
>     Brian> annoyed  that I thought I'd try to hack out a solution
>     Brian> (lp:~slyguy/bzr/ bug-440952).  This solution modifies
>     Brian> BranchFormat.find_format() to try  opening the provided location
>     Brian> as a repository having determined that  the location is not a
>     Brian> branch.  If it is a repository, the resulting  NotBranchError's
>     Brian> extra field (now exposed from the superclass) is set  to
>     Brian> "location is a repository"; this string is only shown to the
>     Brian> user,  and thus preserves the calling requirements of
>     Brian> find_format().
> 
> That sounds like a very clever way to address the bug.
> 
> Did you try to run the test suite to check ? 
> 
> My only concern would be that foreign branches use the same code
> and so would fail to find the file too, but they may be doing so
> in different ways.
> 
> I don't think that's enough to block your patch though.
> 
> 
>     Brian> My approach does mean that two transport probes will
>     Brian> now be made when the location is not a branch. 
> 
> Yes, but that's an error case, so I think it's fine to spend a
> bit more time to give a far better error message.
> 
>     Brian> My initial approach was to modify the relevant command
>     Brian> definitions in builtin.py, but this requires modifying
>     Brian> every possible command beyond the obvious branch' and
>     Brian> 'checkout', and is somewhat error-prone (when I
>     Brian> realized I had forgotten about 'missing').  I'm not
>     Brian> sure what impact of changing find_format(), but I
>     Brian> guessed find_format() is only called a couple of times
>     Brian> per invocation.
> 
>     Brian> Comments or suggestions?
> 
> Do 'bzr lp-open lp:~slyguy/bzr/bug-440952' and propose for
> merging into trunk. You'll get the review from there and other
> may comment on top of my remarks above.
> 
>     Vincent

I suppose my primary question is whether Branch.find_format() should be
doing this, or whether BzrDir.open_branch() should.

Namely, if we have a BzrDir object, then we know there *is* a bzrdir
present at that location, which may have a repository or branch.

Anyway, the layering "feels" better if BzrDir is the one probing if this
branch location is actually a repository, rather than putting that logic
into Branch.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrPR+AACgkQJdeBCYSNAAMmOwCgwnxIOPPeobjPvjnViPGyjGjY
qIsAnAsVuF9ItRo7T99x14YKqsQdOgWp
=6wY9
-----END PGP SIGNATURE-----