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

Jump to bottom

x/vgo: vendor writes misleading vgo.list file #25624

Closed
iand opened this issue May 29, 2018 · 6 comments
Closed

x/vgo: vendor writes misleading vgo.list file #25624

iand opened this issue May 29, 2018 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
vgo

Comments

@iand
Copy link
Contributor

iand commented May 29, 2018

Please answer these questions before submitting your issue. Thanks!

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

go 1.10

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/iand/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/iand"
GORACE=""
GOROOT="/opt/go"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build364187917=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Added a dependency on a package not referred to in an import statement. In this sample it's a tool required for go generate but in the codebase where I first encountered this issue it is several tools used as part of the continuous build process that we have previously vendored in the past using govendor and dep.

Example that reproduces it.

What did you expect to see?

The source for golang.org/x/tools to be copied into vendor directory

What did you see instead?

vendor/vgo.list updated to

golang.org/x/tools          v0.0.0-20180525024113-a5b4c53f6e8b

but golang.org/x/tools is not copied to the vendor directory.

I note from a quick look at the vgo source that the vendor command uses the import list to build the copy list but uses a different list when writing vgo.list
@gopherbot gopherbot added this to the vgo milestone May 29, 2018
@oiooj oiooj added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 2, 2018
@Piszmog
Copy link

Piszmog commented Jun 4, 2018

I am experience a similar thing when using go-micro in my project. vgo.list has go-micro listed but will not be copied to the vendor directory.

Even stranger if I delete the vendor directory and run vgo vendor, the directory will not be created as long as my project is using go-micro (if I remove all references to go-micro, then the vendor directory will be created and populated)

@rsc rsc changed the title x/vgo: vendor updates vgo.list but doesn't copy package x/vgo: vendor writes misleading vgo.list file Jun 6, 2018
@gopherbot
Copy link

Change https://golang.org/cl/116759 mentions this issue: cmd/go/internal/vgo: fix handling of vendor dir in package patterns

@rsc
Copy link
Contributor

rsc commented Jun 6, 2018

Probably the vgo.list file should be refined to make clear that certain packages from certain modules are copied. For the specific case of iand/vgo-vendor, there's no path from the packages in that repo to anything that imports golang.org/x/tools, so x/tools is (correctly) not copied. It should also not appear in vgo.list.

@Piszmog, please file a separate issue about the problem you are seeing, with a bit more detail. I sent a CL 116759 that might help or might not - I don't fully understand what you were reporting.

@iand
Copy link
Contributor Author

iand commented Jun 6, 2018

It was my expectation that adding require golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b to go.mod would declare golang.org/x/tools as a dependency that needs to be vendored by vgo -vendor.

Enabling this behaviour would address many use cases where tools supporting development or build are to be versioned along with direct dependencies. Examples include tools referenced by go:generate, asset packing utilities (go-bindata etc.), protobuf compilers and linter/validators.

gopherbot pushed a commit to golang/vgo that referenced this issue Jun 7, 2018
@rsc
cmd/go/internal/vgo: fix handling of vendor dir in package patterns
203abfb 
A directory x/vendor containing code is just a package called vendor.
A directory x/vendor/y is vendored code.
When a package pattern scans the current module's file tree
it should include a package called vendor but should not
include the directories containing vendored code.

Maybe related to a comment on golang/go#25624.

Change-Id: I083a98a9c70c2121cff7a2f394ff985a54bed37a
Reviewed-on: https://go-review.googlesource.com/116759
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@rsc
Copy link
Contributor

rsc commented Jun 7, 2018 •
edited

@iand that's not what vendor is defined to do. Vendor only copies the specific packages needed by the module, not everything recursively reachable from go.mod (a far larger set). To force vendoring of some set of additional packages, I would suggest putting a file tools.go into the root of the module that says:

// Imports of tools we want "vgo vendor" to include.

// +build tools

package tools

import (
    _ "rsc.io/2fa"
)

You wouldn't put _ "golang.org/x/tools" though, since that's not a command. You want to specify exactly the pieces that you need to be able to build/run.

It's important that we have some indication that these are not just unused go.mod entries, since we also want to be able to diagnose unused go.mod entries. :-)

@gopherbot
Copy link

Change https://golang.org/cl/117258 mentions this issue: cmd/go/internal/vgo: process packages for all build tags in vendor

@myitcv myitcv mentioned this issue Jun 16, 2018
Closed
@rodrigc rodrigc mentioned this issue Apr 6, 2019
Closed
@golang golang locked and limited conversation to collaborators Jun 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
vgo
Development

No branches or pull requests
5 participants
@iand @rsc @oiooj @gopherbot @Piszmog