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
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@docmerlin
Copy link

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
Copy link

fd0 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
Copy link
Author

docmerlin commented Mar 19, 2018 via email

@fd0
Copy link

fd0 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
Copy link

fd0 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
Copy link
Contributor

rsc 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.

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 30, 2018
@gopherbot
Copy link

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.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants