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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by bradfitz
Modified:
7 years, 12 months 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/
8 years 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. ...
8 years 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 ...
8 years ago (2012-02-14 05:40:58 UTC) #3
r2
i appreciate the end but not the means. -rob
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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, ...
8 years 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 ...
8 years 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, ...
8 years 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. ...
8 years 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 ...
8 years 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: > ...
8 years 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, ...
8 years 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 ...
8 years 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 ...
8 years 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, ...
8 years ago (2012-02-14 06:39:17 UTC) #18
gburd
> we'd want to send a certain Accept header from the go tool, > so ...
8 years 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 ...
8 years 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 ...
8 years ago (2012-02-14 17:36:55 UTC) #21
gburd
> The Accept header is probably overkill; Gary says it is hard to handle, The ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years ago (2012-02-14 19:29:44 UTC) #28
gburd
> How many people run web sites directly out of S3 Amazon added the features ...
8 years 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 ...
8 years 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 ...
8 years 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, ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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, > > ...
8 years 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 ...
8 years 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 ...
8 years 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.
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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, ...
8 years 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 ...
8 years 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 ...
8 years 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 ...
8 years 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. ...
8 years 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, ...
8 years 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 ...
8 years 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: ...
8 years 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, ...
8 years 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 ...
7 years, 12 months 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. ...
7 years, 12 months 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.
7 years, 12 months 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: > ...
7 years, 12 months 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.
7 years, 12 months 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: > ...
7 years, 12 months 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.
7 years, 12 months 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 ...
7 years, 12 months 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, ...
7 years, 12 months 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 ...
7 years, 12 months 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.
7 years, 12 months 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 ...
7 years, 12 months 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 ...
7 years, 12 months 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 ...
7 years, 12 months 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 ...
7 years, 12 months 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 ...
7 years, 12 months 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 ...
7 years, 12 months ago (2012-03-06 11:31:08 UTC) #69
rsc1
7 years, 12 months 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