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: convert replacements in Gopkg.lock file #24087

Open
artyom opened this Issue Feb 23, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@artyom
Contributor

artyom commented Feb 23, 2018

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

go version go1.10 darwin/amd64 vgo:2018-02-20.1

What did you do?

Use vgo on an existing project which dependencies are pinned with dep tool. One dependency in Gopkg.toml has source attribute overriding source code location:

[[constraint]]
  branch = "master"
  name = "github.com/armon/go-socks5"
  source = "github.com/artyom/go-socks5"

This corresponds to the following section in Gopkg.lock file with revision being the one from the fork (with fork being ahead of original):

[[projects]]
  branch = "master"
  name = "github.com/armon/go-socks5"
  packages = ["."]
  revision = "5ab49e6379c2e1ea5f402e29430ac01f39e60acc"
  source = "github.com/artyom/go-socks5"

Example program along with Gopkg.* files and go.mod derived from them by vgo can be found here: https://gist.github.com/artyom/3f73ca859f7476902ba4e1c78da0429d

vgo tool imported metadata from Gopkg.* files, but ignored that source attribute.

What did you expect to see?

Dependency location replacement being imported:

module "me/example"

require (
	"github.com/armon/go-socks5" v0.0.0-20160902184237-e75332964ef5
	"golang.org/x/net" v0.0.0-20180218175443-cbe0f9307d01
)

replace "github.com/armon/go-socks5" v0.0.0-20160902184237-e75332964ef5 => "github.com/artyom/go-socks5" v0.0.0-20171215124554-5ab49e6379c2

What did you see instead?

Dependency location replacement ignored:

module "me/example"

require (
	"github.com/armon/go-socks5" v0.0.0-20160902184237-e75332964ef5
	"golang.org/x/net" v0.0.0-20180218175443-cbe0f9307d01
)

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

@rsc

This comment has been minimized.

Contributor

rsc commented Mar 30, 2018

That's true. The vgo importer is only trying to do a good job with the basic cases, not capture all the complexity possible in a Gopkg.lock. @sdboyer is planning to write a more advanced standalone converter for this kind of case.

@rsc rsc changed the title from x/vgo: metadata import ignores source overrides in Gopkg.* files to x/vgo: point to standalone converter for advanced Gopkg.toml files Mar 30, 2018

@rsc rsc added the NeedsFix label Mar 30, 2018

@rsc rsc modified the milestones: vgo, vgo2 Jun 6, 2018

@gopherbot

This comment has been minimized.

gopherbot commented Jun 20, 2018

Change https://golang.org/cl/120075 mentions this issue: cmd/go/internal/modconv: support conversion preserves replace and exclude statement

gopherbot pushed a commit to golang/vgo that referenced this issue Jun 22, 2018

cmd/go/internal/modconv: support conversion preserves replace and exc…
…lude statement

Now modconv is only work with the basic cases, we need support
"replace" and "exclude" from legacy config.

In followup CLs, It will preserve replacements from glide and vendor.json.

Updates golang/go#25556
Updates golang/go#24087

Change-Id: Ie5ca8df7f685177afea9cc7affcc6240b38223b5
Reviewed-on: https://go-review.googlesource.com/120075
Reviewed-by: Russ Cox <rsc@golang.org>

@rsc rsc modified the milestones: vgo2, Go1.12 Jul 12, 2018

@rsc rsc changed the title from x/vgo: point to standalone converter for advanced Gopkg.toml files to cmd/go: point to standalone converter for advanced Gopkg.toml files Jul 12, 2018

@rsc rsc added the modules label Jul 12, 2018

@rsc

This comment has been minimized.

Contributor

rsc commented Jul 17, 2018

We've dropped the plans for the standalone converter - the go command does a good enough job that we should just fix whatever is left that it can handle but doesn't. (Many things in Gopkg.toml it can't handle, and that's OK too.)

We should look into making cmd/go/internal/modconv handle the example in this report directly, though. Repurposing this issue.

@rsc rsc changed the title from cmd/go: point to standalone converter for advanced Gopkg.toml files to cmd/go: convert replacements in Gopkg.lock file Jul 17, 2018

@oiooj

This comment has been minimized.

Member

oiooj commented Jul 17, 2018

Maybe I can work on this, @rsc.

@rsc

This comment has been minimized.

Contributor

rsc commented Jul 19, 2018

@oiooj That'd be great, thanks.

@gopherbot

This comment has been minimized.

gopherbot commented Jul 31, 2018

Change https://golang.org/cl/126915 mentions this issue: cmd/go/internal/modconv: support convert replacements in Gopkg.lock

@dmitris

This comment has been minimized.

Contributor

dmitris commented Aug 8, 2018

@oiooj - thanks for picking this up! A question - would your versions of ParseGopkgLock (modconv/dep.go) be able to handle source directives such as source = "git@internal.mirror.company.com:google/go-github.git"? Here's the full snippet from our Gopkg.lock:

[[projects]]
  branch = "customfix"
  name = "github.com/google/go-github"
  packages = ["github"]
  revision = "7f787a673ce7e986ecc98b1925c90369e1a3b372"
  source = "git@internal.mirror.xyzcompany.com:google/go-github.git"

corresponding part from Gopkg.toml is:

[[constraint]]
  name = "github.com/google/go-github"
  branch = "customfix"
  source = "git@internal.mirror.xyzcompany.com:google/go-github.git"

Currently running go1.11beta3 mod init gives an error:
go: converting Gopkg.lock: stat github.com/google/go-github@7f787a673ce7e986ecc98b1925c90369e1a3b372: unknown revision7f787a673ce7e986ecc98b1925c90369e1a3b372 - possibly because the revision 7f787a673ce7e986ecc98b1925c90369e1a3b372 is the commit hash for the customfix branch in the internal fork, not present in the upstream repo. If this is something that you intend to fix, I'd be happy to test your version.

@oiooj

This comment has been minimized.

Member

oiooj commented Aug 8, 2018

@dmitris Thanks for your reminding, the change updated and this change not yet merged in the master.

@dmitris

This comment has been minimized.

Contributor

dmitris commented Oct 3, 2018

I see that @bcmills made last comments on Sep. 11 on https://go-review.googlesource.com/c/go/+/126915 - @oiooj do you plan to address them / update the patch? If you could use some help with getting the change to the merge, let me know, I may be able to spend some time on this. (I'm interested in this since we would like to convert a large number of Gopkg.lock files to go.mod - with all the "third party" imports sourced from the internal mirror - and doing it manually is very tedious and time-consuming.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment