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: document behaviour of vgo vendor (and why it adds transitive (test) dependencies) #25672

Closed
flibustenet opened this issue May 31, 2018 · 13 comments

Comments

@flibustenet
Copy link

commented May 31, 2018

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

go version go1.10.1 linux/amd64 vgo:2018-02-20.1
vgo updated just now

What did you do?

empty go.mod
vgo build
vgo list -m

MODULE                           VERSION
gotodo                           -
github.com/gorilla/mux           v1.6.2
github.com/gorilla/schema        v1.0.2
github.com/gorilla/securecookie  v1.1.1
github.com/jmoiron/sqlx          v0.0.0-20180406164412-2aeb6a910c2b
github.com/jordan-wright/email   v0.0.0-20180115032944-94ae17dedda2
github.com/lib/pq                v0.0.0-20180523175426-90697d60dd84
github.com/pelletier/go-toml     v1.1.0
github.com/pkg/errors            v0.8.0
github.com/satori/go.uuid        v1.2.0
golang.org/x/crypto              v0.0.0-20180531191117-5ba7f6308246

They are really the dependencies that I use

vgo vendor
vgo build
vgo list -m

MODULE                           VERSION
gotodo                           -
github.com/BurntSushi/toml       v0.3.0
github.com/davecgh/go-spew       v1.1.0
github.com/go-sql-driver/mysql   v1.3.0
github.com/gorilla/mux           v1.6.2
github.com/gorilla/schema        v1.0.2
github.com/gorilla/securecookie  v1.1.1
github.com/jmoiron/sqlx          v0.0.0-20180406164412-2aeb6a910c2b
github.com/jordan-wright/email   v0.0.0-20180115032944-94ae17dedda2
github.com/lib/pq                v0.0.0-20180523175426-90697d60dd84
github.com/mattn/go-sqlite3      v1.7.0
github.com/pelletier/go-toml     v1.1.0
github.com/pkg/errors            v0.8.0
github.com/satori/go.uuid        v1.2.0
golang.org/x/crypto              v0.0.0-20180531191117-5ba7f6308246
gopkg.in/check.v1                v1.0.0-20161208181325-20d25e280405
gopkg.in/yaml.v2                 v2.2.1
$ cat go.mod
module gotodo

require (
	github.com/BurntSushi/toml v0.3.0
	github.com/davecgh/go-spew v1.1.0
	github.com/go-sql-driver/mysql v1.3.0
	github.com/gorilla/mux v1.6.2
	github.com/gorilla/schema v1.0.2
	github.com/gorilla/securecookie v1.1.1
	github.com/jmoiron/sqlx v0.0.0-20180406164412-2aeb6a910c2b
	github.com/jordan-wright/email v0.0.0-20180115032944-94ae17dedda2
	github.com/lib/pq v0.0.0-20180523175426-90697d60dd84
	github.com/mattn/go-sqlite3 v1.7.0
	github.com/pelletier/go-toml v1.1.0
	github.com/pkg/errors v0.8.0
	github.com/satori/go.uuid v1.2.0
	golang.org/x/crypto v0.0.0-20180531191117-5ba7f6308246
	gopkg.in/check.v1 v1.0.0-20161208181325-20d25e280405
	gopkg.in/yaml.v2 v2.2.1
)

I don't use all of them

What did you expect to see?

The same dependencies at first.

I believe it's because of tests, it's ok to vendor them.

But maybe go.mod should not be updated with dependencies that my module don't use.

@gopherbot gopherbot added this to the vgo milestone May 31, 2018

@ianlancetaylor ianlancetaylor changed the title x/vgo vgo vendor add dependencies at next build x/vgo: vgo vendor add dependencies at next build May 31, 2018

@flibustenet

This comment has been minimized.

Copy link
Author

commented Jun 1, 2018

A smaller app to show this

package main

import _ "github.com/pelletier/go-toml"

func main() {
}
$ vgo build
vgo: resolving import "github.com/pelletier/go-toml"
vgo: finding github.com/pelletier/go-toml (latest)
vgo: adding github.com/pelletier/go-toml v1.1.0

$ vgo vendor
vgo: resolving import "github.com/BurntSushi/toml"
vgo: finding github.com/BurntSushi/toml (latest)
vgo: adding github.com/BurntSushi/toml v0.3.0
vgo: resolving import "github.com/davecgh/go-spew/spew"
vgo: finding github.com/davecgh/go-spew (latest)
vgo: adding github.com/davecgh/go-spew v1.1.0
vgo: resolving import "gopkg.in/yaml.v2"
vgo: finding gopkg.in/yaml.v2 (latest)
vgo: adding gopkg.in/yaml.v2 v2.2.1

$ cat go.mod

module test

require (
	github.com/BurntSushi/toml v0.3.0
	github.com/davecgh/go-spew v1.1.0
	github.com/pelletier/go-toml v1.1.0
	gopkg.in/yaml.v2 v2.2.1
)

go.mod is updated at vgo vendor, no need to go build to see this.

@oiooj

This comment has been minimized.

Copy link
Member

commented Jun 1, 2018

The docs says:

Vendor resets the module's vendor directory to include all
packages needed to builds and test all packages in the module
and their dependencies.

So this looks right.

@myitcv

This comment has been minimized.

Copy link
Member

commented Jun 1, 2018

Agree with your conclusion @oiooj

@flibustenet - I'm going to close this for now because it's working as intended/expected, but if you have any concerns with those semantics please feel free to comment/question here.

Thanks

@myitcv myitcv closed this Jun 1, 2018

@myitcv myitcv changed the title x/vgo: vgo vendor add dependencies at next build x/vgo: question on why vgo vendor adds transitive (test) dependencies Jun 1, 2018

@flibustenet

This comment has been minimized.

Copy link
Author

commented Jun 1, 2018

My concern is not so much that it vendor additional dependencies, it's ok to have a full copy of the dependencies.

My concern is that vendor it should not update go.mod with them as they are not required by the app.

For example my use case now is to vendor only because i need to use legacy tools (fresh, guru...) and there is no "universal proxy-cache". But soon i will not need to vendor, so i don't want to commit a go.mod with theses temporary dependencies.

@oiooj

This comment has been minimized.

Copy link
Member

commented Jun 1, 2018

@flibustenet Yes, your concerns is right. It's no need to change go.mod maybe.

@myitcv

This comment has been minimized.

Copy link
Member

commented Jun 1, 2018

My concern is that vendor it should not update go.mod with them as they are not required by the app.

I suspect this is happening because your app's dependencies are not Go modules (i.e. do not have go.mod files themselves), hence the transitive set of dependencies has to be captured in your app's go.mod.

Others might be able to verify/refute this.

For example my use case now is to vendor only because i need to use legacy tools (fresh, guru...) and there is no "universal proxy-cache".

That's good to know. There is work in progress to help make these tools Go module aware.

There is also the more general question of trimming/pruning go.mod files which picks up your post-vgo vendor point about how to remove what will be extraneous dependencies.

@flibustenet

This comment has been minimized.

Copy link
Author

commented Jun 4, 2018

I think it still need investigation, we should verify this and confirm that it's the behavior that we want. Maybe you can reopen it or do you see an other issue to discuss about vendoring with vgo ?

@myitcv

This comment has been minimized.

Copy link
Member

commented Jun 5, 2018

@flibustenet - I think it certainly makes sense to re-open this issue as a placeholder for documenting how and what vgo vendor does more thoroughly. I'll tweak the title too.

@myitcv myitcv reopened this Jun 5, 2018

@myitcv myitcv changed the title x/vgo: question on why vgo vendor adds transitive (test) dependencies x/vgo: document behaviour of vgo vendor (and why it adds transitive (test) dependencies) Jun 5, 2018

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2018

You would see the same things added to go.mod if you ran go test all. Any command that learns about a missing entry in go.mod will add them. Once dependencies' go.mods start being more accurate there will be less of these "incidental" additions - you'll just see what your module's packages and tests need. But for now they're part of accurately describing the world.

The quoted text, except for the typo, looks right to me. What more do you want to say?

@flibustenet

This comment has been minimized.

Copy link
Author

commented Jun 7, 2018

(like) Any command that learns about a missing entry in go.mod will add them

I think a sentence like this should go somewhere in the doc. Maybe in the vgo vendor help or more globally.

Anyway for me it's clear now, and will see in the future what's append when every dependencies will have a go.mod. To @myitcv if you can close the issue
Thanks

@flibustenet

This comment has been minimized.

Copy link
Author

commented Jun 13, 2018

There are situations where a build is ok (very few dependencies) but with transitive (test) dependencies it doesn't work. Then it's annoying to have to resolve broken test dependencies just to vendor working dependencies.

What about an option to decide if we want to vendor test or not ?

Example with github.com/pelletier/go-toml vgo build works but vgo vendor not:

import "github.com/pelletier/go-toml"

$ vgo build
$ vgo vendor
vgo: resolving import "github.com/BurntSushi/toml"
vgo: finding github.com/BurntSushi/toml (latest)
vgo: adding github.com/BurntSushi/toml v0.3.0
vgo: resolving import "github.com/davecgh/go-spew/spew"
vgo: finding github.com/davecgh/go-spew (latest)
vgo: adding github.com/davecgh/go-spew v1.1.0
vgo: resolving import "gopkg.in/yaml.v2"
vgo: finding gopkg.in/yaml.v2 (latest)
vgo: adding gopkg.in/yaml.v2 v2.2.1
vgo: import "github.com/davecgh/go-spew/spew/testdata" [/home/wilk/go/src/v/github.com/davecgh/go-spew@v1.1.0/spew/testdata]: no Go source files

edit: The issue for spew : davecgh/go-spew#86 maybe related to #25871

@gopherbot

This comment has been minimized.

Copy link

commented Jul 5, 2018

Change https://golang.org/cl/122256 mentions this issue: cmd/go/internal/modcmd: drop test sources and data from mod -vendor

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2018

The problem with go-spew/spew/testdata was just a bug.
We're going to drop tests of deps from 'vgo mod -vendor',
but they will still be analyzed during 'vgo mod -sync' or 'vgo list all'.
Will be fixed by CL 122256.

@golang golang locked and limited conversation to collaborators Jul 10, 2019

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