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

cmd/go: go mod vendor preserves go.mod and go.sum files unpredictably #42970

Closed
thaJeztah opened this issue Dec 3, 2020 · 14 comments
Closed
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@thaJeztah
Copy link
Contributor

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

$ go version
go version go1.15.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/sebastiaan/Library/Caches/go-build"
GOENV="/Users/sebastiaan/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/sebastiaan/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/sebastiaan/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/sebastiaan/go/src/github.com/containerd/containerd/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/c_/vjh56sc12fd2b_q2n02_lt140000gn/T/go-build441796130=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

containerd recently switched to using go modules (with vendoring), and I noticed that go.mod for dependencies is only preserved if the main module is used, for example;
https://github.com/containerd/containerd/tree/59a0667cff783b9e4e61ea2a138db220b4fbdca2/vendor/github.com/containerd/cgroups

However, a dependency for which only some packages are vendored, but not the main module, go.mod, and go.sum are omitted; for example: https://github.com/containerd/containerd/tree/59a0667cff783b9e4e61ea2a138db220b4fbdca2/vendor/github.com/opencontainers/runc

What did you expect to see?

I expected both go.mod and go.sum to be preserved for any dependency, irregardless if the main module is consumed or not (similar to how LICENSE and NOTICE of the main module are preserved.

What did you see instead?

I saw that go.mod and go.sum are only preserved if the main module is consumed.

@mvdan
Copy link
Member

mvdan commented Dec 3, 2020

I saw that go.mod and go.sum are only preserved if the main module is consumed.

To clarify, you mean if the package at the root of the third-party module is used and vendored, right?

@mvdan mvdan added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 3, 2020
@jayconrod
Copy link
Contributor

This is probably feasible, but it may be misleading when vendoring a package for which there are multiple modules whose paths are prefixes in the build list.

For example, suppose package example.com/a/b from module example.com/a is vendored. Module example.com/a/b is also in the build list, and it does not contain a package in its root directory. Should go mod vendor copy go.mod and go.sum from module example.com/a/b into vendor/example.com/a/b, even though the contents of that directory come from module example.com/a?

I'm not sure what we do here for license files. I didn't actually know we copied them into the vendor directory when the root package of a module is not vendored.

cc @bcmills @matloob

@mvdan
Copy link
Member

mvdan commented Dec 3, 2020

I didn't actually know we copied them into the vendor directory when the root package of a module is not vendored.

I assume that might be a requirement, since many licenses like GPL require distributing the license text when distributing the code.

If we include go.mod/go.sum, which I think can be reasonable, that perhaps opens the door to other "special" top-level files like READMEs too, which pkgsite also treats in a special way.

@bcmills
Copy link
Contributor

bcmills commented Dec 3, 2020

I think we should actually strip out go.mod files — and perhaps go.sum files — unconditionally. I'm a bit surprised that we don't already, and I consider that a bug.

Otherwise, if you cd into a vendor subdirectory and invoke a go command, it will think that it is in the actual module instead of walking up to the root of the actual main module as it ought to.

@mvdan
Copy link
Member

mvdan commented Dec 3, 2020

If someone wants to obtain or inspect those go.mod files, I assume they can always inspect go list -m -json all? That makes sense to me at least, because vendoring works at the package level, not at the module level.

@bcmills
Copy link
Contributor

bcmills commented Dec 3, 2020

The general rule for go mod vendor is that it must preserve everything needed in order to go build (but not go test) the vendored packages, and should make a best effort to preserve everything needed to comply with licensing requirements (although the latter is ultimately the responsibility of the author of the main module).

The go.mod file falls into neither of those cases.

@bcmills
Copy link
Contributor

bcmills commented Dec 3, 2020

@mvdan, go list -m -json all explicitly fails in vendor mode, precisely because we don't preserve all of the go.mod files that would be needed for it. But go list -mod=readonly -m -json all should work.

@bcmills
Copy link
Contributor

bcmills commented Dec 3, 2020

(And go mod download -json does show the go.mod location, since go mod subcommands don't have the -mod flag and therefore cannot default to -mod=vendor. And go mod graph and go mod why can also query those dependencies without any extra flags.)

@bcmills bcmills changed the title cmd/go: module: go mod vendor does not preserve go.mod and go.sum if main module is not used cmd/go: module: go mod vendor preserves go.mod and go.sum files unpredictably Dec 3, 2020
@thaJeztah
Copy link
Contributor Author

Just out of a meeting; catching up now

To clarify, you mean if the package at the root of the third-party module is used and vendored, right?

Correct, yes

For example, suppose package example.com/a/b from module example.com/a is vendored. Module example.com/a/b is also in the build list, and it does not contain a package in its root directory. Should go mod vendor copy go.mod and go.sum from module example.com/a/b into vendor/example.com/a/b, even though the contents of that directory come from module example.com/a?

You mean if example.com/a/b is a submodule (so has its own go.mod ?)

I assume that might be a requirement, since many licenses like GPL require distributing the license text when distributing the code.

Yes, some licenses require that (and at least the NOTICE to be preserved. README's are a bit of a grey area; technically the LICENSE itself is not providing the "license", but the NOTICE (and depending on the license, the presence of headers in each file), that said; many project apply the license by putting the license/copyright in the README (which is why some other vendoring tools preserve those files).

Otherwise, if you cd into a vendor subdirectory and invoke a go command, it will think that it is in the actual module instead of walking up to the root of the actual main module as it ought to.

Tempted to say: "don't do that", but 🤷‍♂️ that said; I know use-cases where the vendor directory is used to build binaries, in which case (I think?) that may actually be the behavior someone is looking for?)

If someone wants to obtain or inspect those go.mod files, I assume they can always inspect go list -m -json all? That makes sense to me at least, because vendoring works at the package level, not at the module level.

I do think the go.mod (at least) and go.sum are useful information to have; to an extend to "visually" get informed of updates in dependencies, also in an offline situation.

@jayconrod jayconrod changed the title cmd/go: module: go mod vendor preserves go.mod and go.sum files unpredictably cmd/go:go mod vendor preserves go.mod and go.sum files unpredictably Dec 3, 2020
@jayconrod
Copy link
Contributor

You mean if example.com/a/b is a submodule (so has its own go.mod ?)

Yes.

Tempted to say: "don't do that", but 🤷‍♂️ that said; I know use-cases where the vendor directory is used to build binaries, in which case (I think?) that may actually be the behavior someone is looking for?)

I think this is a somewhat common use case. When we were talking about removing vendoring early on, we got feedback that a lot of people preferred vendoring because they could easily poke around the vendor to directory to view and modify their dependencies' code.

I do think the go.mod (at least) and go.sum are useful information to have; to an extend to "visually" get informed of updates in dependencies, also in an offline situation.

Could you expand on this a bit more? Generally go list -m -u all is the best way to list available updates. It doesn't work offline, but I don't think anything would.

I'm worried that adding go.mod files will introduce ambiguity; they can even be actively misleading. In my example above, example.com/a/b might not provide any vendored packages at all; it might be in the build list because it's a test dependency of something else.

@bcmills
Copy link
Contributor

bcmills commented Dec 3, 2020

I do think the go.mod (at least) and go.sum are useful information to have; to an extend to "visually" get informed of updates in dependencies, also in an offline situation.

The diffs of the vendor/modules.txt file and the main module's go.mod file, and to a lesser extent the main module's go.sum file, should pretty much always be sufficient to inform the developer of changes in relevant dependencies. (And I don't see a strong use-case for being notified of changes in irrelevant dependencies — that is, modules for which a nontrivial selected version is known, but from which no packages are imported, even indirectly, by the main module.)

In an offline situation, you can't edit dependency versions without a populated module cache anyway. (And if you do have a populated module cache, the relevant go.mod and go.sum files are already there.)

@bcmills bcmills added this to the Backlog milestone Dec 3, 2020
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 8, 2020
@bcmills bcmills modified the milestones: Backlog, Go1.17 Dec 8, 2020
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 8, 2020
@jayconrod jayconrod changed the title cmd/go:go mod vendor preserves go.mod and go.sum files unpredictably cmd/go: go mod vendor preserves go.mod and go.sum files unpredictably Jan 6, 2021
@bcmills bcmills self-assigned this Apr 30, 2021
@gopherbot
Copy link

Change https://golang.org/cl/315410 mentions this issue: cmd/go: prune go.mod and go.sum files from vendored dependencies

@thaJeztah
Copy link
Contributor Author

Thanks!

@gopherbot
Copy link

Change https://golang.org/cl/335050 mentions this issue: _content/ref/mod: mention other Go 1.17 behaviors that depend on the go version

gopherbot pushed a commit to golang/website that referenced this issue Jul 16, 2021
…go version

Updates golang/go#36876
Updates golang/go#42970
Updates golang/go#45965

Change-Id: I542e9ece986806f9b4a062f242387b1ca47f5814
Reviewed-on: https://go-review.googlesource.com/c/website/+/335050
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@rsc rsc unassigned bcmills Jun 23, 2022
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
…go version

Updates golang/go#36876
Updates golang/go#42970
Updates golang/go#45965

Change-Id: I542e9ece986806f9b4a062f242387b1ca47f5814
Reviewed-on: https://go-review.googlesource.com/c/website/+/335050
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants