code review 5306069: goinstall: More intelligent vcs selection for common sites

goinstall: More intelligent vcs selection for common sites

goinstall has built in support for a few common code hosting sites.  The
identification of which vcs tool should be used was based purely on a
regex match against the provided import path.  The problem with this
approach is that it requires distinct import paths for different vcs
tools on the same site.

Since bitbucket has recently starting hosting Git repositories under the
same bitbucket.org/user/project scheme as it already hosts Mercurial
repositories, now would seem a good time to take a more flexible
approach.

We still match the import path against a list of regexes, but now the
match is purely to distinguish the different hosting sites.  Once the
site is identified, the specified function is called with the repo and
path matched out of the import string.  This function is responsible for
creating the vcsMatch structure that tells us what we need to download
the code.

For github and launchpad, only one vcs tool is currently supported, so
these functions can simply return a vcsMatch structure.  For googlecode,
we retain the behaviour of determing the vcs from the import path - but
now it is done by the function instead of the regex.  For bitbucket, we
use api.bitbucket.org to find out what sort of repository the specified
import path corresponds to - and then construct the appropriate vcsMatch
structure.

[quantumfyre, 2011-10-26 22:56:13]

Hello golang-dev@googlegroups.com (cc: adg@golang.org,
golang-dev@googlegroups.com, rsc@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[quantumfyre, 2011-10-26 23:06:31]

Hello golang-dev@googlegroups.com (cc: adg@golang.org,
golang-dev@googlegroups.com, rsc@golang.org),

Please take another look.

[adg, 2011-10-27 04:58:49]

I like this.

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go
File src/cmd/goinstall/download.go (right):

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:127:
{regexp.MustCompile(`^([a-z0-9\-]+\.googlecode\.com/(svn|git|hg))(/[a-z0-9A-Z_.\-/]*)?$`),
googleVcs},
please expand these lines to be of the form:
{
   regexp.Must...,
   googleVcs,
},

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:149: panic("Unknown googlecode vcs: " + parts[1])
return nil, os.NewError("unsupported googlecode vcs: " + parts[1])

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:154: return nil, os.NewError("repository " + repo
+ path + " should not have .git suffix")
I think the repo + path context is provided upstream (in the install function),
so this should be simply
return nil, os.NewError("path must not include .git suffix")

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:159: type bitbucketInfo struct {
Put this type definition inside the bitbuckedVcs function.

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:165: return nil, os.NewError("repository " + repo
+ path + " should not have .git suffix")
ditto my earlier comment from githubVcs

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:170: // Use a GET request to api.bitbucket.org to
find out about the repository
// Ask the bitbucket API what kind of repository this is.

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:171: r, err :=
http.Get("https://api.bitbucket.org/1.0/repositories/" + parts[1])
Please put this URL in a constant (at the top of the function)

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:174: return nil, os.NewError(s)
return nil, fmt.Errorf("error querying BitBucket API: %v", err)

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:181: return nil, os.NewError(s)
ditto

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:184: var info bitbucketInfo
var response struct { Vcs string `json:"scm"` }

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:186: // Decode the JSON into a bitbucketInfo
structure - currently the only
drop this comment and the blank line above

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:189: err = d.Decode(&info)
err = json.NewDecoder(r.Body).Decode(&repsonse)

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:192: return nil, os.NewError(s)
ditto previous error changes (use the same message for each - the err string
itself will explain what went wrong)

http://codereview.appspot.com/5306069/diff/8001/src/cmd/goinstall/download.go...
src/cmd/goinstall/download.go:203: return nil, os.NewError("Unknown vcs type " +
info.Vcs + "received from api.bitbucket.org")
return nil, os.NewError("unsupported bitbucket vcs: " + info.Vcs)

[quantumfyre, 2011-10-27 07:40:26]

Hello golang-dev@googlegroups.com, adg@golang.org (cc:
golang-dev@googlegroups.com, rsc@golang.org),

Please take another look.

[adg, 2011-10-27 08:44:04]

LGTM

It would be nice to have some tests for this, but I think it's worth going ahead
and committing this one for now.

[adg, 2011-10-27 08:45:14]

*** Submitted as http://code.google.com/p/go/source/detail?r=69423942510b ***

goinstall: More intelligent vcs selection for common sites

goinstall has built in support for a few common code hosting sites.  The
identification of which vcs tool should be used was based purely on a
regex match against the provided import path.  The problem with this
approach is that it requires distinct import paths for different vcs
tools on the same site.

Since bitbucket has recently starting hosting Git repositories under the
same bitbucket.org/user/project scheme as it already hosts Mercurial
repositories, now would seem a good time to take a more flexible
approach.

We still match the import path against a list of regexes, but now the
match is purely to distinguish the different hosting sites.  Once the
site is identified, the specified function is called with the repo and
path matched out of the import string.  This function is responsible for
creating the vcsMatch structure that tells us what we need to download
the code.

For github and launchpad, only one vcs tool is currently supported, so
these functions can simply return a vcsMatch structure.  For googlecode,
we retain the behaviour of determing the vcs from the import path - but
now it is done by the function instead of the regex.  For bitbucket, we
use api.bitbucket.org to find out what sort of repository the specified
import path corresponds to - and then construct the appropriate vcsMatch
structure.

R=golang-dev, adg
CC=golang-dev, rsc
http://codereview.appspot.com/5306069

Committer: Andrew Gerrand <adg@golang.org>

[rsc, 2011-10-31 16:16:39]

Please add some tests, to make it less likely
that I will break this when moving it into "go".

Thanks.