Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: running get on a custom repo no longer works when upgrading to go 1.8.3 #20731

Closed
joegrasse opened this issue Jun 19, 2017 · 11 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@joegrasse
Copy link

I ran the following with custom urls.

go get -u -d customurl.com/lib/platform/genlog
get -u -d customurl.com/ext/daemon-example

This worked with go 1.7.4 and fails with go 1.8.3.

With go 1.8.3, the first command works, but the second command fails with the following error.

Error: Get Failed: package customurl.com/lib/platform/genlog: customurl.com/lib/platform/genlog is a custom import path for https://mysvnrepo.com/PIM/golang/customurl.com/lib/platform/genlog/trunk, but /gopath/src/customurl.com/lib/platform/genlog is checked out from https://mysvnrepo.com/PIM

The package is actually downloaded though.

When doing a svn info https://mysvnrepo.com/PIM/golang/customurl.com/lib/platform/genlog/trunk

I get the following.

Path: trunk
URL: https://mysvnrepo.com/PIM/golang/customurl.com/lib/platform/genlog/trunk
Repository Root: https://mysvnrepo.com/PIM
Repository UUID: 71ba88c3-99bc-4787-b7e4-f6bc34d7c3e8
Revision: 34403
Node Kind: directory
Last Changed Author: username
Last Changed Rev: 33154
Last Changed Date: 2017-01-27 09:05:49 -0600 (Fri, 27 Jan 2017)

I am guessing that the commit 17ad60b broke it.

/cc @rsc

@bradfitz bradfitz added this to the Go1.10 milestone Jun 19, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 19, 2017
@bradfitz
Copy link
Contributor

We're currently frozen and wrapping up Go 1.9. If this was broken through the whole Go 1.8 cycle, it can probably wait until Go 1.10.

/cc @rsc though in case he has time to investigate.

@joegrasse
Copy link
Author

joegrasse commented Jun 20, 2017

I'm thinking line

if remote != repo && rr.isCustom {
should be changed from

if remote != repo && rr.isCustom {

to

if !strings.HasPrefix(repo, remote) && rr.isCustom {

/cc @rsc

@rsc
Copy link
Contributor

rsc commented Jun 20, 2017

What does curl https://customurl.com/lib/platform/genlog show as far as meta tags?

@joegrasse
Copy link
Author

I am going to assume you mean curl https://customurl.com/lib/platform/genlog?go-get=1.

<html>
<head>
<meta name="go-import" content="customurl.com/lib/platform/genlog svn https://mysvnrepo.com/PIM/golang/customurl.com/lib/platform/genlog/trunk">
</head>
<body>
go get customurl.com/lib/platform/genlog
</body>
</html>

@joegrasse
Copy link
Author

@rsc Is there anything else you need from me? It would be really awesome if a fix didn't have to wait till go 1.10.

@rsc
Copy link
Contributor

rsc commented Jun 21, 2017

The relevant parts of the go command docs read:

The meta tag has the form:

<meta name="go-import" content="import-prefix vcs repo-root">

The import-prefix is the import path corresponding to the repository root. It must be a prefix or an exact match of the package being fetched with "go get". If it's not an exact match, another http request is made at the prefix to verify the <meta> tags match.

The vcs is one of "git", "hg", "svn", etc.

The repo-root is the root of the version control system containing a scheme and not containing a .vcs qualifier.

For example,

import "example.org/pkg/foo"

will result in the following requests:

https://example.org/pkg/foo?go-get=1 (preferred)
http://example.org/pkg/foo?go-get=1  (fallback, only with -insecure)

If that page contains the meta tag

<meta name="go-import" content="example.org git https://code.org/r/p/exproj">

the go tool will verify that https://example.org/?go-get=1 contains the same meta tag and then git clone https://code.org/r/p/exproj into GOPATH/src/example.org.

The problem here is that the <meta> tag being served does not name a Subversion repository root. Subversion is different from Git and Mercurial (and like CVS) in that you can check out a subtree, and it appears that that's what is being directed here. This is not really supported, at least according to the docs.

Whether we should support it, since it mostly works, is a different question. It looks like perhaps we should change the go/src/cmd/go/internal/get/vcs.go from looking for "\nRepository Root: " to look instead for "\nURL: ".
@joegrasse, can you try that and see if it fixes your use case?

I'm a bit reluctant to make a change at this point in the Go 1.9 cycle, but maybe it's OK if we can establish that (1) all recent subversions do print URL lines, and (2) if you do check out the repository root instead of a subdirectory, the URL and Repository Root lines say the same thing.

@joegrasse
Copy link
Author

@rsc Making changes to vcs.go worked. Here is what i did.

$ git clone https://go.googlesource.com/go
$ cd go
$ git checkout go1.8.3
$ patch src/cmd/go/vcs.go ../issue-20731.patch
$ cd src
$ GOROOT_BOOTSTRAP=/usr/local/go ./all.bash

(1) I have svn version 1.7.21 and 1.8.14 and they both print out the URL.
(2) On my test of checking out the repo root, the URL and Repository Root lines said the same thing.

Contents of issue-20731.patch

--- vcs.go.old  2017-06-21 14:06:15.166391000 -0500
+++ vcs.go.new  2017-06-21 14:22:56.566029000 -0500
@@ -299,14 +299,14 @@

        // Expect:
        // ...
-       // Repository Root: <URL>
+       // URL: <URL>
        // ...

-       i := strings.Index(out, "\nRepository Root: ")
+       i := strings.Index(out, "\nURL: ")
        if i < 0 {
                return "", fmt.Errorf("unable to parse output of svn info")
        }
-       out = out[i+len("\nRepository Root: "):]
+       out = out[i+len("\nURL: "):]
        i = strings.Index(out, "\n")
        if i < 0 {
                return "", fmt.Errorf("unable to parse output of svn info")

@gopherbot
Copy link

CL https://golang.org/cl/46417 mentions this issue.

@joegrasse
Copy link
Author

@rsc I see that a patch has been committed. Will this end up making go 1.9?

@rsc
Copy link
Contributor

rsc commented Jun 22, 2017

Yes (unless a problem is discovered with it and it's rolled back).

@joegrasse
Copy link
Author

Ok, thanks.

@golang golang locked and limited conversation to collaborators Jun 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants