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

[Vincent Ladeuil]

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

P.S.: The whole test suite passes with your patch, which is a
      good sign, we may want to add more tests though :)