cmd/go: allow go-import <meta> tags to specify a branch #10913

Open
alanconway opened this Issue May 19, 2015 · 10 comments

Projects

None yet

5 participants

@alanconway

Allow meta tags in 'go get' to specify a branch. Suggested syntax from discussion at https://groups.google.com/d/msg/golang-dev/SW8r9ODYQf0/kqofheawGWYJ

<meta name="go-import" content="qpid.apache.org/proton git https://git-wip-us.apache.org/repos/asf/qpid-proton.git/proton-c/bindings/go branch=go1">

Rationale: there are several reasons why you might want go get to retrieve from a branch that is not "master", "trunk" or the default branch for a repository.

  • master/trunk usually used for development, a project may want direct "go get" to get the latest stable release or snapshot on a branch or tag.
  • make "experimental" work available from a branch before it is considered stable/mature enough to move to the project's main branch.
  • some repos have odd branch naming conventions for historical reasons (e.g. git repos that were converted from SVN and still use "trunk" instead of "master")
@bradfitz
Member

/cc @adg

@minux minux changed the title from Allow go-import <meta> tags to specify a branch to cmd/go: allow go-import <meta> tags to specify a branch May 19, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jun 3, 2015
@rsc
Contributor
rsc commented Nov 23, 2015

I wonder if it should be rev= instead of branch=, so that other non-branch identifiers can be used too. That said, too late for Go 1.6. Need more details. Perhaps the right next step is to write a proposal.

@rsc rsc modified the milestone: Unplanned, Go1.6 Nov 23, 2015
@alanconway

rev= makes sense to me, released labels could be useful as well as branches.

@elithrar

I'd be interested in picking this up—which includes writing the proposal—but want to hash out the design a little more and am seeking feedback:

Q. How should the revision/branch/tag support work? Ideally we want to support all three forms but we will need ways to identify which form the go-imports tag is referring to.
Proposed Answer: <meta name="go-import" content="import-prefix vcs repo-root repo-rev"> where repo-rev is of the form branch=$name, rev=$commit or tag=$tag.

Q: What does backward compatibility look like here?
Proposed Answer: Because older versions of the Go tool will not be aware of the additional tag, they will likely (TODO: check this) pull from HEAD instead of the specified rev. This implicit behaviour will be confusing to package consumers as a package author might set up the import URL as getattest.io/pkgname tag=v2, but the consumer won't get this revision and might wonder why their program is breaking or otherwise different.

Should we fail outright here? Rely on // +build go1.7 in the package itself to prevent the behaviour (has other ramifications)? This needs more thought. One solution may be to re-order the go-imports format to force this failure—e.g. import-prefix vcs repo-rev repo-root—at the risk of a confusing error message from earlier versions of go get and other tools.

There are likely other questions here as well, but I wanted to get these down to get the discussion moving.

@alanconway

Q1. For git I would say rev=xxx (or commit=xxx) is sufficient for repo-rev. The git tools that take a commit will accept a branch, tag, hash or any other string that can name a commit, so I would expect this to do the same (i.e. I can say rev=mybranch, rev=mytag, rev=xxxxcommithash and all will work) I don't know if that is true for other repo types (mercurial, bzr etc.) or if they need the extra info to interpret the string as a branch/tag/etc. Possibly this needs to be different to follow the normal syntax for different repo types?

Q2. Fail outright. Anyone who needs backwards compat will have to set things up to work the old way anyway (HEAD or the magic go1 branch), so there's no point in them using the new feature. If they use the new feature it is safe to assume they have not intentionally set things up to work the old way, and if it appears to work by accident Bad Things will probably happen.

Perhaps a safe way to ensure a clean failure would be use <meta name="go-import-rev">, so old go won't even find the new directive. That would also allow you to set up both "go-import" for old go and "go-import-rev" for new go, e.g. go-import might point to a different repo that has the proper branch cloned as master for backwards compat.

@rsc
Contributor
rsc commented Dec 29, 2015

Q1. If you say rev= then that's supposed to work for any code identifier. You don't have to say branch= sometimes or tag= other times, just like you don't have to tell, say, git checkout what kind of argument you are giving it.

Q2. No, older versions of the go command will ignore the tag entirely, resulting in not being able to resolve the reference. That's fine. If you need to specify a branch, then you can't speak to them.

@elithrar

Thanks for the responses @alanconway and @rsc.

The rev/branch/tag suggestion was primarily for compatibility across VCS', but since git, Mercurial, bazaar and Subversion all just take a revision of any kind it's unnecessary (as Russ points out).

I'll look to push a patch to support an updated go-import tag of the form <meta name="go-import" content="import-prefix vcs repo-root repo-rev"> when I have some free time (moving continents).

@alanconway

Q2: I believe there will be some set of go versions that does understand <meta name="go-import" but does not understand the new rev= Since rev= implies the author does not want you to pull master it would be bad if they simply ignored the rev= and pulled master anyway. Some kind of failure is called for in that case I think.

@elithrar

I think my original proposal to just re-order the fields: repo-rev before
the URL - would be sufficient to prevent that.

I'll see how the current tool parses the meta tag when I'm in front of a PC.
On Wed, 30 Dec 2015 at 5:16 AM, alanconway notifications@github.com wrote:

Q2: I believe there will be some set of go versions that does understand <meta
name="go-import" but does not understand the new rev= Since rev= implies
the author does not want you to pull master it would be bad if they
simply ignored the rev= and pulled master anyway. Some kind of failure is
called for in that case I think.


Reply to this email directly or view it on GitHub
#10913 (comment).

@rsc
Contributor
rsc commented Jan 4, 2016

@alanconway, @elithrar regarding Q2 (again):

Q2. No, older versions of the go command will ignore the tag entirely, resulting in not being able to resolve the reference. That's fine. If you need to specify a branch, then you can't speak to them.

They ignore the meta tag because it has the wrong number of fields. See the source code (src/cmd/go/discovery.go):

    if f := strings.Fields(attrValue(e.Attr, "content")); len(f) == 3 {
        imports = append(imports, metaImport{
            Prefix:   f[0],
            VCS:      f[1],
            RepoRoot: f[2],
        })
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment