Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1347)

Issue 5660051: code review 5660051: cmd/go: allow go get with arbitrary URLs (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by bradfitz
Modified:
12 years ago
Reviewers:
rog
CC:
r2, rsc, gburd, eikeon, mc, niemeyer, rsc1, golang-dev
Visibility:
Public.

Description

cmd/go: allow go get with arbitrary URLs This CL permits using arbitrary, non-VCS-qualified URLs as aliases for fully VCS-qualified and/or well-known code hosting sites. Example 1) A VCS-qualified URL can now be shorter. Before: $ go get camlistore.org/r/p/camlistore.git/pkg/blobref After: $ go get camlistore.org/pkg/blobref Example 2) A custom domain can be used as the import, referencing a well-known code hosting site. Before: $ go get github.com/bradfitz/sonden After: $ go get bradfitz.com/pkg/sonden The mechanism used is a <meta> tag in the HTML document retrieved from fetching: https://<import>?go-get=1 (preferred) http://<import>?go-get=1 (fallback) The meta tag should look like: <meta name="go-import" content="import-alias-prefix vcs full-repo-root"> The full-repo-root must be a full URL root to a repository containing a scheme and *not* containing a ".vcs" qualifier. The vcs is one of "git", "hg", "svn", etc. The import-alias-prefix must be a prefix or exact match of the package being fetched with "go get". If there are multiple meta tags, only the one with a prefix matching the import path is used. It is an error if multiple go-import values match the import prefix. If the import-alias-prefix is not an exact match for the import, another HTTP fetch is performed, at the declared root (which does *not* need to be the domain's root). For example, assuming that "camlistore.org/pkg/blobref" declares in its HTML head: <meta name="go-import" content="camlistore.org git https://camlistore.org/r/p/camlistore" /> ... then: $ go get camlistore.org/pkg/blobref ... looks at the following URLs: https://camlistore.org/pkg/blobref?go-get=1 http://camlistore.org/pkg/blobref?go-get=1 https://camlistore.org/?go-get=1 http://camlistore.org/?go-get=1 Ultimately it finds, at the root (camlistore.org/), the same go-import: <meta name="go-import" content="camlistore.org git https://camlistore.org/r/p/camlistore" /> ... and proceeds to trust it, checking out git //camlistore.org/r/p/camlistore at the import path of "camlistore.org" on disk. Fixes issue 3099

Patch Set 1 #

Patch Set 2 : diff -r 1cd9ea014a9e https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 1cd9ea014a9e https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r 08cd55fc9a7d https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r 116b2ccf6c88 https://go.googlecode.com/hg/ #

Total comments: 14

Patch Set 6 : diff -r 116b2ccf6c88 https://go.googlecode.com/hg/ #

Total comments: 24

Patch Set 7 : diff -r 086a1ee9175b https://go.googlecode.com/hg #

Patch Set 8 : diff -r e219970cdf9f https://go.googlecode.com/hg/ #

Patch Set 9 : diff -r e219970cdf9f https://go.googlecode.com/hg/ #

Total comments: 26

Patch Set 10 : diff -r ec44ee506a8e https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 11 : diff -r f46217216512 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+282 lines, -20 lines) Patch
M src/cmd/go/bootstrap.go View 1 2 3 4 5 6 7 8 9 1 chunk +15 lines, -2 lines 0 comments Download
A src/cmd/go/discovery.go View 1 2 3 4 5 6 1 chunk +63 lines, -0 lines 0 comments Download
M src/cmd/go/get.go View 1 2 3 4 5 6 1 chunk +5 lines, -4 lines 0 comments Download
M src/cmd/go/http.go View 1 2 3 4 5 6 7 8 9 2 chunks +51 lines, -0 lines 0 comments Download
M src/cmd/go/vcs.go View 1 2 3 4 5 6 7 8 9 10 4 chunks +148 lines, -14 lines 0 comments Download

Messages

Total messages: 70
bradfitz
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
12 years, 1 month ago (2012-02-14 05:34:30 UTC) #1
dsymonds
I'm not a fan. It's adding more magic to something that is already fairly magical. ...
12 years, 1 month ago (2012-02-14 05:36:56 UTC) #2
bradfitz
I'm not a fan of, 1) ugly import paths 2) making github, bitbucket et al ...
12 years, 1 month ago (2012-02-14 05:40:58 UTC) #3
r2
i appreciate the end but not the means. -rob
12 years, 1 month ago (2012-02-14 05:44:08 UTC) #4
dsymonds
Can't you set up a symlink or something on the server-side so that camlistore.org/testlib looks ...
12 years, 1 month ago (2012-02-14 05:44:24 UTC) #5
bradfitz
On Tue, Feb 14, 2012 at 4:44 PM, David Symonds <dsymonds@golang.org> wrote: > Can't you ...
12 years, 1 month ago (2012-02-14 05:46:27 UTC) #6
dsymonds
On Tue, Feb 14, 2012 at 4:46 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > because it ...
12 years, 1 month ago (2012-02-14 05:50:56 UTC) #7
rsc
On Tue, Feb 14, 2012 at 00:50, David Symonds <dsymonds@golang.org> wrote: > I thought the ...
12 years, 1 month ago (2012-02-14 05:52:21 UTC) #8
bradfitz
On Tue, Feb 14, 2012 at 4:50 PM, David Symonds <dsymonds@golang.org> wrote: > On Tue, ...
12 years, 1 month ago (2012-02-14 05:52:29 UTC) #9
rsc
i like the general redirect idea but not so much the implementation. i'll sleep on ...
12 years, 1 month ago (2012-02-14 05:53:36 UTC) #10
bradfitz
Cool. I'm not wed to specific implementations. On Tue, Feb 14, 2012 at 4:53 PM, ...
12 years, 1 month ago (2012-02-14 05:58:03 UTC) #11
gburd
Suggestion: If the URL is not recognized, then send a HEAD request for the URL. ...
12 years, 1 month ago (2012-02-14 06:21:49 UTC) #12
rsc
On Tue, Feb 14, 2012 at 01:21, Gary Burd <gary.burd@gmail.com> wrote: > If the URL ...
12 years, 1 month ago (2012-02-14 06:24:40 UTC) #13
bradfitz
On Tue, Feb 14, 2012 at 5:21 PM, Gary Burd <gary.burd@gmail.com> wrote: > Suggestion: > ...
12 years, 1 month ago (2012-02-14 06:25:54 UTC) #14
dsymonds
On Tue, Feb 14, 2012 at 5:24 PM, Russ Cox <rsc@golang.org> wrote: > On Tue, ...
12 years, 1 month ago (2012-02-14 06:26:54 UTC) #15
rsc
On Tue, Feb 14, 2012 at 01:26, David Symonds <dsymonds@golang.org> wrote: > I know it's ...
12 years, 1 month ago (2012-02-14 06:30:00 UTC) #16
gburd
The entire tree should be redirected to parallel paths. This is easy on many web ...
12 years, 1 month ago (2012-02-14 06:30:54 UTC) #17
dsymonds
On Tue, Feb 14, 2012 at 5:30 PM, Russ Cox <rsc@golang.org> wrote: > On Tue, ...
12 years, 1 month ago (2012-02-14 06:39:17 UTC) #18
gburd
> we'd want to send a certain Accept header from the go tool, > so ...
12 years, 1 month ago (2012-02-14 07:43:08 UTC) #19
rsc
I think it is important that this work across servers. I like the idea that ...
12 years, 1 month ago (2012-02-14 16:53:29 UTC) #20
gustavo_niemeyer.net
> RewriteEngine On > RewriteCond %{QUERY_STRING} ^go-get-vcs=1$ > RewriteRule ^/my/package/dir$ https://github.com/me/package/dir [R=302,L] The ability to ...
12 years, 1 month ago (2012-02-14 17:36:55 UTC) #21
gburd
> The Accept header is probably overkill; Gary says it is hard to handle, The ...
12 years, 1 month ago (2012-02-14 17:58:54 UTC) #22
gustavo_niemeyer.net
On Tue, Feb 14, 2012 at 15:58, Gary Burd <gary.burd@gmail.com> wrote: >> The Accept header ...
12 years, 1 month ago (2012-02-14 18:04:56 UTC) #23
rsc
On Tue, Feb 14, 2012 at 12:58, Gary Burd <gary.burd@gmail.com> wrote: > The Accept header ...
12 years, 1 month ago (2012-02-14 18:07:46 UTC) #24
gburd
A problem with redirects is that sites hosted on Amazon S3 and similar services cannot ...
12 years, 1 month ago (2012-02-14 18:29:22 UTC) #25
rsc
On Tue, Feb 14, 2012 at 13:29, Gary Burd <gary.burd@gmail.com> wrote: > A problem with ...
12 years, 1 month ago (2012-02-14 18:49:34 UTC) #26
gburd
Amazon S3 does not have the feature. There's no way to specify redirect rules or ...
12 years, 1 month ago (2012-02-14 19:25:14 UTC) #27
rsc
On Tue, Feb 14, 2012 at 14:25, Gary Burd <gary.burd@gmail.com> wrote: > Amazon S3 does ...
12 years, 1 month ago (2012-02-14 19:29:44 UTC) #28
gburd
> How many people run web sites directly out of S3 Amazon added the features ...
12 years, 1 month ago (2012-02-14 19:45:44 UTC) #29
rsc
Okay, let's suppose we're not going to use the redirect. It has this S3 problem ...
12 years, 1 month ago (2012-02-14 20:01:58 UTC) #30
gustavo_niemeyer.net
On Tue, Feb 14, 2012 at 18:01, Russ Cox <rsc@golang.org> wrote: > What if instead ...
12 years, 1 month ago (2012-02-14 20:39:39 UTC) #31
rsc
On Tue, Feb 14, 2012 at 15:39, Gustavo Niemeyer <gustavo@niemeyer.net> wrote: > That looks nice, ...
12 years, 1 month ago (2012-02-14 20:41:40 UTC) #32
eikeon
On Feb 14, 2012, at 3:01 PM, Russ Cox wrote: > What if instead we ...
12 years, 1 month ago (2012-02-14 21:05:13 UTC) #33
rsc
Recognized paths (github, bitbucket, things with .git in the import path) would not follow this ...
12 years, 1 month ago (2012-02-14 21:28:24 UTC) #34
mc
On 2012/02/14 20:01:58, rsc wrote: > Okay, let's suppose we're not going to use the ...
12 years, 1 month ago (2012-02-16 06:47:25 UTC) #35
rsc
On Thu, Feb 16, 2012 at 01:47, <untheoretic@googlemail.com> wrote: > Isn't this worse than the ...
12 years, 1 month ago (2012-02-16 19:27:18 UTC) #36
bradfitz
On Mon, Feb 13, 2012 at 9:34 PM, <bradfitz@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > ...
12 years, 1 month ago (2012-02-21 09:59:53 UTC) #37
rsc
Are you planning to update this CL to implement the scheme we converged on? I ...
12 years, 1 month ago (2012-02-21 19:09:01 UTC) #38
bradfitz
On Wed, Feb 22, 2012 at 6:09 AM, Russ Cox <rsc@golang.org> wrote: > Are you ...
12 years, 1 month ago (2012-02-21 22:03:35 UTC) #39
bradfitz
Hello golang-dev@googlegroups.com, dsymonds@golang.org, r@google.com, rsc@golang.org, gary.burd@gmail.com, gustavo@niemeyer.net, eikeon@eikeon.com, untheoretic@googlemail.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years, 1 month ago (2012-02-23 07:48:57 UTC) #40
bradfitz
Missing from this email is the new CL description: cmd/go: allow go get with arbitrary ...
12 years, 1 month ago (2012-02-23 07:49:50 UTC) #41
niemeyer
Very nice, thanks Brad. A few comments: http://codereview.appspot.com/5660051/diff/25001/src/cmd/go/http.go File src/cmd/go/http.go (right): http://codereview.appspot.com/5660051/diff/25001/src/cmd/go/http.go#newcode82 src/cmd/go/http.go:82: // serve ...
12 years, 1 month ago (2012-02-23 13:40:35 UTC) #42
rsc
On Thu, Feb 23, 2012 at 02:49, Brad Fitzpatrick <bradfitz@golang.org> wrote: > If the import-alias-prefix ...
12 years, 1 month ago (2012-02-23 17:49:32 UTC) #43
bradfitz
On Thu, Feb 23, 2012 at 9:49 AM, Russ Cox <rsc@golang.org> wrote: > On Thu, ...
12 years, 1 month ago (2012-02-23 19:39:41 UTC) #44
rsc
On Thu, Feb 23, 2012 at 14:39, Brad Fitzpatrick <bradfitz@golang.org> wrote: > No, I don't ...
12 years, 1 month ago (2012-02-23 19:43:20 UTC) #45
bradfitz
http://codereview.appspot.com/5660051/diff/25001/src/cmd/go/vcs.go File src/cmd/go/vcs.go (right): http://codereview.appspot.com/5660051/diff/25001/src/cmd/go/vcs.go#newcode285 src/cmd/go/vcs.go:285: // (thus root is a prefix of importPath) On ...
12 years, 1 month ago (2012-02-23 22:26:58 UTC) #46
bradfitz
Hello golang-dev@googlegroups.com, dsymonds@golang.org, r@google.com, rsc@golang.org, gary.burd@gmail.com, gustavo@niemeyer.net, eikeon@eikeon.com, untheoretic@googlemail.com, n13m3y3r@gmail.com (cc: golang-dev@googlegroups.com), Please take another ...
12 years, 1 month ago (2012-02-23 22:29:02 UTC) #47
niemeyer
http://codereview.appspot.com/5660051/diff/25001/src/cmd/go/http.go File src/cmd/go/http.go (right): http://codereview.appspot.com/5660051/diff/25001/src/cmd/go/http.go#newcode82 src/cmd/go/http.go:82: // serve a meta import in their 404 page. ...
12 years, 1 month ago (2012-02-23 23:51:39 UTC) #48
bradfitz
ping On Thu, Feb 23, 2012 at 2:29 PM, <bradfitz@golang.org> wrote: > Hello golang-dev@googlegroups.com, dsymonds@golang.org, ...
12 years, 1 month ago (2012-02-29 20:16:51 UTC) #49
rsc
Sorry, I'm trying to wrap up other work on the go tool to send out ...
12 years, 1 month ago (2012-02-29 20:20:39 UTC) #50
niemeyer
For reference, moving pending comments to the current diff. http://codereview.appspot.com/5660051/diff/26008/src/cmd/go/http.go File src/cmd/go/http.go (right): http://codereview.appspot.com/5660051/diff/26008/src/cmd/go/http.go#newcode82 src/cmd/go/http.go:82: ...
12 years, 1 month ago (2012-02-29 20:24:56 UTC) #51
bradfitz
I'm not going to update code until we agree on desired behavior. On Feb 29, ...
12 years, 1 month ago (2012-02-29 20:27:58 UTC) #52
rsc
http://codereview.appspot.com/5660051/diff/26008/src/cmd/dist/build.c File src/cmd/dist/build.c (right): http://codereview.appspot.com/5660051/diff/26008/src/cmd/dist/build.c#newcode1130 src/cmd/dist/build.c:1130: "pkg/encoding/xml", Can we move the xml stuff into the ...
12 years ago (2012-03-05 20:53:10 UTC) #53
bradfitz
http://codereview.appspot.com/5660051/diff/26008/src/cmd/go/http.go File src/cmd/go/http.go (right): http://codereview.appspot.com/5660051/diff/26008/src/cmd/go/http.go#newcode44 src/cmd/go/http.go:44: // https resource or, if unavailable, the http resource. ...
12 years ago (2012-03-05 23:56:59 UTC) #54
bradfitz
Hello r@google.com, rsc@golang.org, gary.burd@gmail.com, eikeon@eikeon.com, untheoretic@googlemail.com, n13m3y3r@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years ago (2012-03-05 23:57:11 UTC) #55
rsc1
http://codereview.appspot.com/5660051/diff/26008/src/cmd/go/http.go File src/cmd/go/http.go (right): http://codereview.appspot.com/5660051/diff/26008/src/cmd/go/http.go#newcode64 src/cmd/go/http.go:64: re := <-httpsCh On 2012/03/05 23:56:59, bradfitz wrote: > ...
12 years ago (2012-03-06 04:12:09 UTC) #56
bradfitz
Hello r@google.com, rsc@golang.org, gary.burd@gmail.com, eikeon@eikeon.com, untheoretic@googlemail.com, n13m3y3r@gmail.com, rsc@google.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years ago (2012-03-06 04:27:46 UTC) #57
bradfitz
k, both fixed. PTAL. On Mon, Mar 5, 2012 at 8:12 PM, <rsc@google.com> wrote: > ...
12 years ago (2012-03-06 04:27:55 UTC) #58
bradfitz
Hello r@google.com, rsc@golang.org, gary.burd@gmail.com, eikeon@eikeon.com, untheoretic@googlemail.com, n13m3y3r@gmail.com, rsc@google.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years ago (2012-03-06 04:44:55 UTC) #59
bradfitz
And now with more -v logging, to help people debug. On Mon, Mar 5, 2012 ...
12 years ago (2012-03-06 04:45:17 UTC) #60
rsc1
Very close; just fine-tuning errors and logging. http://codereview.appspot.com/5660051/diff/44001/src/cmd/go/vcs.go File src/cmd/go/vcs.go (right): http://codereview.appspot.com/5660051/diff/44001/src/cmd/go/vcs.go#newcode340 src/cmd/go/vcs.go:340: return nil, ...
12 years ago (2012-03-06 05:13:22 UTC) #61
bradfitz
I think I got most of them, at least in spirit. PTAL? http://codereview.appspot.com/5660051/diff/44001/src/cmd/go/vcs.go File src/cmd/go/vcs.go ...
12 years ago (2012-03-06 05:33:10 UTC) #62
bradfitz
Hello r@google.com, rsc@golang.org, gary.burd@gmail.com, eikeon@eikeon.com, untheoretic@googlemail.com, n13m3y3r@gmail.com, rsc@google.com (cc: golang-dev@googlegroups.com), Please take another look.
12 years ago (2012-03-06 05:33:19 UTC) #63
rsc1
On Tue, Mar 6, 2012 at 00:33, <bradfitz@golang.org> wrote: > no, it doesn't. (get.go just ...
12 years ago (2012-03-06 05:38:37 UTC) #64
rsc1
LGTM http://codereview.appspot.com/5660051/diff/43007/src/cmd/go/vcs.go File src/cmd/go/vcs.go (right): http://codereview.appspot.com/5660051/diff/43007/src/cmd/go/vcs.go#newcode412 src/cmd/go/vcs.go:412: return nil, fmt.Errorf("http or https fetch for import ...
12 years ago (2012-03-06 05:41:24 UTC) #65
bradfitz
On Mar 5, 2012 9:38 PM, "Russ Cox" <rsc@google.com> wrote: > > On Tue, Mar ...
12 years ago (2012-03-06 05:47:34 UTC) #66
rsc1
On Tue, Mar 6, 2012 at 00:47, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Can't say I ...
12 years ago (2012-03-06 05:49:11 UTC) #67
bradfitz
*** Submitted as http://code.google.com/p/go/source/detail?r=0af5bcfdb4c1 *** cmd/go: allow go get with arbitrary URLs This CL permits ...
12 years ago (2012-03-06 06:36:20 UTC) #68
rog
On 6 March 2012 05:33, <bradfitz@golang.org> wrote: > plus, I've always been told to include ...
12 years ago (2012-03-06 11:31:08 UTC) #69
rsc1
12 years ago (2012-03-06 14:27:22 UTC) #70
On Tue, Mar 6, 2012 at 06:31, roger peppe <rogpeppe@gmail.com> wrote:
> i wish this was the case.
>
> the current situation (some functions add context to their errors,
> some don't) is not ideal for producing predictably good diagnostics.

It is definitely true that exported functions should include context,
like os.Open does.  If they don't, then after Go 1 send us CLs
and we can fix them.  (It is too late for this kind of minor tweaking now.)

Inside a package or a binary, the line gets fuzzier: often when calling
a helper function you can generate the most consistent messages if
the caller adds the context, so that it only gets handled in one place.

Russ
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b