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

x/vgo: rewrite v2+.x.x pointing at non-module code to v0.0.0 pseudoversion #24056

Closed
docmerlin opened this issue Feb 23, 2018 · 6 comments

Comments

@docmerlin
Copy link

commented Feb 23, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

vgo

Does this issue reproduce with the latest release?

no just with vgo

What did you do?

vgo get breaks the build of anything that has a dependency that is using tags for semantic versioning. The assumption on imports without an explicit dependency meaning v0-v1 breaks things.

If your package already use semantic versioning in git tags, anything that imports your app and has been keeping up to date with master (or with the latest tag) will suddenly break.

A possible workaround to minimize the breaking is to make import "github.com/foo/bar" assume a fallback that is less likely to break. Assuming it means master (if there are no semvar tags), or latest required semvar if it is being fetched as part of a build, or something similar is far less likely to break packages. This would also require v0-v1 being required to be explicit, if there are any semvar tags with major version greater than v1 in the git repo.

What did you expect to see?

No breaking changes in go1.

What did you see instead?

A majorly breaking change in go1. Lots of things will become un-vgo-getable.

@gopherbot gopherbot added this to the vgo milestone Feb 23, 2018

@fd0

This comment has been minimized.

Copy link

commented Feb 24, 2018

Here's an example using https://github.com/dgrijalva/jwt-go, which has semver tags starting at v1.0.0 until v3.1.0, which is the latest version. When this is used in a small sample program, e.g. https://github.com/fd0/vgotest-issue-24056, vgo will select the (ancient) tag v1.0.2:

package main

import (
        "fmt"
        jwt "github.com/dgrijalva/jwt-go"
)

func main() {
        fmt.Printf("constant from jwt-go: %v\n", jwt.ValidationErrorMalformed)
}
$ vgo version
go version go1.10 linux/amd64 vgo:2018-02-20.1

$ cat go.mod
module "github.com/fd0/vgotest-issue-24056"

$ vgo build 
vgo: resolving import "github.com/dgrijalva/jwt-go"
vgo: finding github.com/dgrijalva/jwt-go (latest)
vgo: adding github.com/dgrijalva/jwt-go v1.0.2

$ cat go.mod
module "github.com/fd0/vgotest-issue-24056"

require "github.com/dgrijalva/jwt-go" v1.0.2

Manually adding the newest tagged version in go.mod leads to the described error that the /v3 path component in the import path is not there:

$ cat go.mod
module "github.com/fd0/vgotest-issue-24056"

require "github.com/dgrijalva/jwt-go" v3.1.0

$ vgo build
vgo: errors parsing go.mod:
/home/fd0/shared/work/go/src/github.com/fd0/vgotest-issue-24056/go.mod:3: invalid module: github.com/dgrijalva/jwt-go should be v1, not v3 (v3.1.0)

From the article https://research.swtch.com/vgo-module I got the impression that repositories which do not yet have a go.mod are treated the same way by vgo as Go does today, but in this case it seems to be different. Is it maybe a good idea to not require a semantic import path component when no go.mod file is there? I think this is what @docmerlin refers to in this issue (please correct me if I'm wrong).

Is my impression wrong, did I overlook something?

Thank you very much @rsc for getting this discussion started! I'm excited about the upcoming changes!

@docmerlin

This comment has been minimized.

Copy link
Author

commented Mar 19, 2018

@fd0

This comment has been minimized.

Copy link

commented Mar 25, 2018

For the record, for direct dependencies this can be resolved by manually pinning the version of jwt-go to a specific hash in go.mod:

require "github.com/dgrijalva/jwt-go" v0.0.0-20171019215719-dbeaa9332f19a944acb5736b4456cfcc02140e29

I've found the timestamp by checking out the tag, and then using git as follows:

$ TZ=UTC git log --date='format-local:%Y%m%d%H%M%S' | egrep '(commit|Date)' | head -n 2
commit dbeaa9332f19a944acb5736b4456cfcc02140e29
Date:   20171019215719

Is there a way to do this automatically?

How can I do this with indirect dependencies?

I've added a second sample repository, https://github.com/fd0/vgotest-issue-24056-indirect The program there imports github.com/Azure/go-autorest, which in turn imports github.com/dgrijalva/jwt-go. Although I've added the jwt-go manually into go.mod, I cannot build it with vgo:

$ cat go.mod
module "github.com/fd0/vgotest-issue-24056"

require (
	"github.com/Azure/go-autorest" v0.0.0-20180321200920-7909b98056dd6f6a9fc9b7745af1810c93c15939
	"github.com/dgrijalva/jwt-go" v0.0.0-20171019215719-dbeaa9332f19a944acb5736b4456cfcc02140e29
)

$ vgo build
vgo: finding github.com/Azure/go-autorest v0.0.0-20180321200920-7909b98056dd6f6a9fc9b7745af1810c93c15939
vgo: finding github.com/Azure/go-autorest v0.0.0-20180321200920-7909b98056dd6f6a9fc9b7745af1810c93c15939
vgo: parsing downloaded go.mod: go.mod:7: invalid module: github.com/dgrijalva/jwt-go should be v1, not v3 (v3.1.0)
[1]    9250 exit 1     vgo build
@fd0

This comment has been minimized.

Copy link

commented Mar 30, 2018

For the record: It's not necessary to find out the timestamp of the git commit by hand, we can just insert the commit ID into the go.mod, and vgo will figure out the right format.

@rsc rsc changed the title x/vgo: breaking builds x/vgo: rewrite v2+.x.x pointing at non-module code to v0.0.0 pseudoversion Mar 30, 2018

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2018

If go.mod says require "github.com/dgrijalva/jwt-go" v3.1.0,
and if there is a v3.1.0 tag of that repo,
and if the tree there has no go.mod nor v3/go.mod,
then vgo should assume this is a pointer to pre-module code,
and it should rewrite v3.1.0 to the appropriate v0.0.0 pseudo-version,
the same as it would if the require line had listed the commit hash of v3.1.0 instead.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 18, 2018

Change https://golang.org/cl/107660 mentions this issue: cmd/go/internal/modfetch: fix conversion of legacy v2 versions

@golang golang locked and limited conversation to collaborators Apr 25, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.