code review 98790043: cmd/go: detect which VCS to use with Bitbucket wh...

cmd/go: detect which VCS to use with Bitbucket when the API call fails.

      The API call will fail when Bitbucket repositories are private. In
that case, probe for the repository using vcsCmd.ping.

      Fixes issue 5375

[bhcleek, 2014-04-26 14:18:36]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[rh, 2014-04-26 18:26:07]

Do you also happen to have a couple sample private repositories of git, hg types
this can be tested on?  If not, no big deal, I can make some.

https://codereview.appspot.com/98790043/diff/10002/src/cmd/go/vcs.go
File src/cmd/go/vcs.go (right):

https://codereview.appspot.com/98790043/diff/10002/src/cmd/go/vcs.go#newcode690
src/cmd/go/vcs.go:690: if err != nil {
Is there a specific type of error returned for private repositories and while
probing for the repo type that can be checked, now that this logic is handling
that specific case?
If I disconnect from the internet, this would previously return a more detailed
message like: "package bitbucket.org/rhencke/cantcode: Get
https://api.bitbucket.org/1.0/repositories/rhencke/cantcode: dial tcp: lookup
api.bitbucket.org: no such host".  It now returns the more generic "unable to
detect version control system..." message.

https://codereview.appspot.com/98790043/diff/10002/src/cmd/go/vcs.go#newcode691
src/cmd/go/vcs.go:691: root := expand(match, "{root}")
root := match["root"]?

https://codereview.appspot.com/98790043/diff/10002/src/cmd/go/vcs.go#newcode694
src/cmd/go/vcs.go:694: return fmt.Errorf("unable to detect version control
system for bitbucket.org/ path")
To reduce duplication, can this be changed into a loop over {"git", "hg"}?

https://codereview.appspot.com/98790043/diff/10002/src/cmd/go/vcs.go#newcode709
src/cmd/go/vcs.go:709: if vcsByCmd(resp.SCM) != nil {
Some of this logic is being done here already.  Can you re-use that logic by
setting resp.SCM in your new code?

[rh, 2014-04-26 18:40:01]

Thank you!  I'll make a private hg one for testing, and share the URL.

(If you keep reply@codereview-hr.appspotmail.com on the cc list, it will 
ensure your replies make it back to the code review)

On Saturday, April 26, 2014 2:27:43 PM UTC-4, Billie H Cleek wrote:
>
> I have a private git repo. bitbucket.org/bhcleek/sqldecoder. I can make a 
> private hg one in a few hours...
> On Apr 26, 2014 12:26 PM, <robert...@gmail.com <javascript:>> wrote:
>
>> Do you also happen to have a couple sample private repositories of git,
>> hg types this can be tested on?  If not, no big deal, I can make some.
>>
>>
>> https://codereview.appspot.com/98790043/diff/10002/src/cmd/go/vcs.go
>> File src/cmd/go/vcs.go (right):
>>
>> https://codereview.appspot.com/98790043/diff/10002/src/
>> cmd/go/vcs.go#newcode690
>> src/cmd/go/vcs.go:690: if err != nil {
>> Is there a specific type of error returned for private repositories and
>> while probing for the repo type that can be checked, now that this logic
>> is handling that specific case?
>> If I disconnect from the internet, this would previously return a more
>> detailed message like: "package bitbucket.org/rhencke/cantcode: Get
>> https://api.bitbucket.org/1.0/repositories/rhencke/cantcode: dial tcp:
>> lookup api.bitbucket.org: no such host".  It now returns the more
>> generic "unable to detect version control system..." message.
>>
>> https://codereview.appspot.com/98790043/diff/10002/src/
>> cmd/go/vcs.go#newcode691
>> src/cmd/go/vcs.go:691: root := expand(match, "{root}")
>> root := match["root"]?
>>
>> https://codereview.appspot.com/98790043/diff/10002/src/
>> cmd/go/vcs.go#newcode694
>> src/cmd/go/vcs.go:694: return fmt.Errorf("unable to detect version
>> control system for bitbucket.org/ path")
>> To reduce duplication, can this be changed into a loop over {"git",
>> "hg"}?
>>
>> https://codereview.appspot.com/98790043/diff/10002/src/
>> cmd/go/vcs.go#newcode709
>> src/cmd/go/vcs.go:709: if vcsByCmd(resp.SCM) != nil {
>> Some of this logic is being done here already.  Can you re-use that
>> logic by setting resp.SCM in your new code?
>>
>> https://codereview.appspot.com/98790043/
>>
>

[rh, 2014-04-26 18:53:02]

bitbucket.org/rhencke/cantcode for hg private repo

[bhcleek, 2014-04-27 23:06:38]

Hello golang-codereviews@googlegroups.com, robert.hencke@gmail.com (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[rh, 2014-04-28 00:24:20]

Thanks for making these changes.  It's more code, but errors are much clearer
now when something goes wrong.  I tested this with a private Mercurial repo and
it works to solve the issue.

I'm not sure if this is worth documenting somewhere that private bitbucket repos
do work now, as long as they are first cloned to the appropriate location in
GOPATH, and their credentials are set up correctly, nor where such documentation
would belong.

I'll defer to rsc on this, as he wrote the original function.  Thanks!

R=rsc

https://codereview.appspot.com/98790043/diff/50001/src/cmd/go/vcs.go
File src/cmd/go/vcs.go (right):

https://codereview.appspot.com/98790043/diff/50001/src/cmd/go/vcs.go#newcode691
src/cmd/go/vcs.go:691: if httpErr, ok := err.(*httpError); ok &&
httpErr.statusCode == 403 {
This may be worth a simple comment explaining its purpose.
// This may be a private repository. If so, attempt to determine which VCS it
uses. See issue 5375.

[bhcleek, 2014-04-28 00:31:23]

I'll put in a comment explaining the fallback to discovery when there's a 403
StatusCode.

As for documenting the private repository issue, it's been discussed on
golang-nuts several times. If this CL gets submitted, it will become a moot
point anyway.

[gobot, 2014-04-28 00:36:31]

R=rsc@golang.org (assigned by robert.hencke@gmail.com)

[bhcleek, 2014-04-28 00:38:45]

Hello golang-codereviews@googlegroups.com, robert.hencke@gmail.com,
gobot@golang.org, rsc@golang.org (cc: golang-codereviews@googlegroups.com),

Please take another look.

[rh, 2014-04-28 00:41:26]

On 2014/04/28 00:38:45, bhcleek wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
mailto:robert.hencke@gmail.com,
> mailto:gobot@golang.org, mailto:rsc@golang.org (cc:
mailto:golang-codereviews@googlegroups.com),
> 
> Please take another look.

LGTM, pending rsc's review.  Thanks. :)

[adg, 2014-04-29 14:19:12]

Please change the CL description to begin with "cmd/go: ..."

[bhcleek, 2014-04-29 14:27:54]

Hello golang-codereviews@googlegroups.com, robert.hencke@gmail.com,
gobot@golang.org, rsc@golang.org, adg@golang.org (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[adg, 2014-04-29 14:29:05]

How'd you change it? You need to write "hg change 98790043".


On 29 April 2014 08:27, <bhcleek@gmail.com> wrote:

> Hello golang-codereviews@googlegroups.com, robert.hencke@gmail.com,
> gobot@golang.org, rsc@golang.org, adg@golang.org (cc:
>
> golang-codereviews@googlegroups.com),
>
> Please take another look.
>
>
> https://codereview.appspot.com/98790043/
>

[bhcleek, 2014-04-29 14:54:11]

Sorry about that. I spelled it wrong first, realized it, and corrected it; but
you must have seen it before my correction. 

PTAL

[bhcleek, 2014-09-09 16:46:10]

Hello golang-codereviews@googlegroups.com, robert.hencke@gmail.com,
gobot@golang.org, rsc@golang.org, adg@golang.org (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[gobot, 2014-12-19 05:19:39]

R=close

To the author of this CL:

The Go project has moved to Gerrit Code Review.

If this CL should be continued, please see the latest version of
https://golang.org/doc/contribute.html for instructions on
how to set up Git and the Go project's Gerrit codereview plugin,
and then create a new change with your current code.

If there has been discussion on this CL, please give a link to it
(golang.org/cl/98790043 is best) in the description in your
new CL.

Thanks very much.