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: the existence of a vendor directory kicks Go into vendor mode when using modules #38644

Closed
ydnar opened this issue Apr 24, 2020 · 11 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@ydnar
Copy link

ydnar commented Apr 24, 2020

Using Go 1.14, this has bit us a couple times. We have a polyglot monorepo with a go.mod and go.sum file at the root level, and previously had a vendor directory for non-Go code. We’ve hacked around the issue my moving what we could out of the vendor directory.

Currently, the mere existence of a vendor directory as a sibling of go.mod will break all Go commands unless explicitly run with -mod=mod.

Request

Go could better exist in a polyglot environment if the trigger for vendor mode was was the existence of vendor/modules.txt instead of just the vendor directory.

Repro Steps

$ go test ./...
# this works

$ mkdir vendor
$ go test ./...
...
run 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory
@andybons andybons changed the title modules: the existence of a vendor directory kicks Go into vendor mode cmd/go: the existence of a vendor directory kicks Go into vendor mode when using modules Apr 27, 2020
@andybons andybons added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 27, 2020
@andybons andybons added this to the Unplanned milestone Apr 27, 2020
@andybons
Copy link
Member

@jayconrod @bcmills @matloob

@bcmills
Copy link
Contributor

bcmills commented Apr 27, 2020

This was an intentional decision: a user may already have a vendor directory present from a pre-modules vendoring tool, and using the vendor directory only when it contains vendor/modules.txt would allow the existing vendor directory to silently present outdated dependencies to downstream users who are themselves using GOPATH mode (and thus unaware of vendor/modules.txt).

(See specifically #33848 (comment).)

@bcmills
Copy link
Contributor

bcmills commented Apr 27, 2020

@ydnar, I think we could revisit this decision if we have evidence that non-Go vendor directories are much more common in practice that stale Go vendor directories.

(Our experience so far suggests that they are not: see, for example, #37738.)

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. modules labels Apr 27, 2020
@ydnar
Copy link
Author

ydnar commented Apr 27, 2020

@bcmills how would you obtain that evidence? Our polyglot repo, for instance, is not open-source.

One thing of note: this worked fine until we added go 1.14 to our go.mod file, so could a behavior change to look for vendor/modules.txt be tied to go 1.15?

@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2020

@ydnar, reports from users, statistical distributions of vendor directories in open-source repos, perhaps others.

I would expect open-source libraries to have existing Go vendor directories at a higher rate than closed-source repos, just because they are more likely to have downstream consumers (and less likely to assume a uniform configuration among those users).

@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2020

so could a behavior change to look for vendor/modules.txt be tied to go 1.15?

In theory it could, but we shouldn't make that change unless we are confident that it is a net-positive. Setting GOFLAGS=-mod=mod doesn't seem that bad as a workaround for your specific repo...

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 28, 2020
@ydnar
Copy link
Author

ydnar commented Apr 28, 2020

In theory it could, but we shouldn't make that change unless we are confident that it is a net-positive. Setting GOFLAGS=-mod=mod doesn't seem that bad as a workaround for your specific repo...

That would mean updating every CI workflow, every makefile, documentation, educating every developer, and retraining our muscle memory to add additional flags to every go invocation.

This was an intentional decision: a user may already have a vendor directory present from a pre-modules vendoring tool, and using the vendor directory only when it contains vendor/modules.txt would allow the existing vendor directory to silently present outdated dependencies to downstream users who are themselves using GOPATH mode (and thus unaware of vendor/modules.txt).

(See specifically #33848 (comment).)

Re-reading this it seems like a missed opportunity to add a vendor true or mod vendor flag in the go.mod file instead of go 1.4 and detecting a vendor directory. At least then it’s explicit.

@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2020

Re-reading this it seems like a missed opportunity to add a vendor true or mod vendor flag in the go.mod file instead of [go 1.14] and detecting a vendor directory. At least then it’s explicit.

That specifically would not have helped with issues like #37738, in which the user has forgotten about the stale vendor directory and thus would presumably also have forgotten to add the explicit flag.

Based on the user feedback we received prior to Go 1.14, I still believe that those cases are vastly more common than the one you are describing (a vendor suddirectory containing exclusively non-Go code directly adjacent to the go.mod file).

@ydnar ydnar closed this as completed Apr 28, 2020
@ydnar
Copy link
Author

ydnar commented Apr 28, 2020

One more try: how about vendor never or vendor disable? We want to use modules, we know what we’re doing, and we don’t need magical hand-holding based on the implied purpose of a directory in our module.

@bcmills
Copy link
Contributor

bcmills commented Apr 28, 2020

Run go env -w GOFLAGS=-mod=mod. The GOFLAGS variable is the general way to set per-user go command flag defaults.

@ydnar
Copy link
Author

ydnar commented Apr 28, 2020

Run go env -w GOFLAGS=-mod=mod. The GOFLAGS variable is the general way to set per-user go command flag defaults.

That’s fine for per user defaults, but not per repo. I mean, if the go command wants to read a .goenv file in cwd or the module directory, great.

@golang golang locked and limited conversation to collaborators Apr 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants