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: {get|build|install} need to improve behaviors with vendor directory. #44841

Closed
james-lawrence opened this issue Mar 7, 2021 · 23 comments
Labels
FrozenDueToAge GoCommand cmd/go 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

@james-lawrence
Copy link
Contributor

james-lawrence commented Mar 7, 2021

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

go version
go version go1.16 linux/amd64

Does this issue reproduce with the latest release?

yes

this is more of an experience report vs a specific bug/proposal around things I've found frustrating with go tooling since moving to modules for the last year.

vendor setups are treated as second class citizens.

this shows itself in a number of ways.

  1. in a repository that contains vendor directory dependencies go get doesn't automatically install to the vendor directory. requiring an additional command.
  2. accesses the network to fetch dependencies when they're in the vendor directory.
  3. numerous claims by go developers that go modules will allow for removing vendor support. this is not true and will never be true. in theory yes, you can build identical programs from go.mod and related infrastructure. however it ignores things like: reliance of 3rd party infrastructure or having to spinning up your own infrastructure to do so. vendoring is essentially that infrastructure in a single folder. it use to be far easier to use, understand, maintain, audit. however it has always been treated as as second class since support was added to satisfy the community at large.

go get updates dependencies without explicit user action.

I've run into this a number of types where I'm working run so go tooling commands and when I build/run into an error where my vendor is out of sync with go.mod only to find a bunch of modules have been updated. Unfortunately I don't know exact which command is causing this, I suspect go get or go mod tidy but its just a gut feeling based on what I remember doing each time i've run into this. what I do know is this has occurred to me multiple times now where dependencies I was not interacting with in any way have been updated.

possibly related:

go modules has caused an explosion in complexity in the go environment

it has almost doubled the number of environment variable flags. its responsible for 9 out of 20 environment variables.

  • GOPROXY
  • GONOPROXY
  • GOPRIVATE
  • GONOSUMDB
  • GOSUMDB
  • GOVCS
  • GOINSECURE
  • GOMODCACHE

most of which are accessing the network and all of which could essentially be dropped from a developers consciousness with a functional vendor directory. in fact if a vendor directory is detected in the main module during get/install/build you could in fact treat it as the gomodcache unless upgrading or adding a new package (in which case you're interacting with the VCS to populate that cache).

@kevinburke1
Copy link

Basically the way I work around this is by providing an extra command that sets all of the flags appropriately, and then just asking everyone to run make vendor.

vendor:
	envdir envs/localdev go mod vendor && envdir envs/localdev go mod tidy

And then envs/localdev contains

$ ll envs/localdev | grep GO
-rw-r--r--  1 kevin staff   3 2019-08-14 16:21 GO111MODULE
-rw-r--r--  1 kevin staff  33 2019-10-04 12:12 GOFLAGS
-rw-r--r--  1 kevin staff 209 2020-01-16 21:49 GONOSUMDB
-rw-r--r--  1 kevin staff  19 2020-01-16 21:49 GOPRIVATE
-rw-r--r--  1 kevin staff   4 2020-01-16 21:49 GOSUMDB

in particular GOFLAGS is -mod=vendor -race, GO111MODULE set to on, and the other ones disable the public checksum db and cache.

@kevinburke1
Copy link

I'm not saying this is a good solution, it took a while to get there, but having it has simplified working with the vendor directory pretty well.

@james-lawrence
Copy link
Contributor Author

james-lawrence commented Mar 7, 2021

@kevinburkemeter appreciate the workaround, looking for the discussion on how we as a community can fix the tool so it can provide a great experience out of the box without twiddling bits and bobs for the basic use cases. there is also a fundamental tooling problem underlying all of these issues which i don't think has been addressed @rsc touches on it in that thread I linked.
moving to a offline/read only starting point for gomodcaches could potentially resolve a ton of this complexity.

@oiooj oiooj added the GoCommand cmd/go label Mar 8, 2021
@toothrot toothrot changed the title go {get|build|install} need to improve behaviors with vendor directory. cmd/go: {get|build|install} need to improve behaviors with vendor directory. Mar 8, 2021
@toothrot
Copy link
Contributor

toothrot commented Mar 8, 2021

/cc @bcmills @jayconrod @matloob

@toothrot toothrot added this to the Backlog milestone Mar 8, 2021
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 8, 2021
@bcmills
Copy link
Contributor

bcmills commented Mar 8, 2021

  1. in a repository that contains vendor directory dependencies go get doesn't automatically install to the vendor directory. requiring an additional command.

That was an intentional decision resulting from the discussion in #30240 (see #30240 (comment) for a summary); see also #29058 and #38222.

The final design as implemented was #33848.

@bcmills
Copy link
Contributor

bcmills commented Mar 8, 2021

go get updates dependencies without explicit user action.

The go command's default behavior was changed in Go 1.16 (see proposal #42466). The go.mod file is no longer updated implicitly by default except in go get and go mod tidy. However, it was already not updated implicitly when -mod=vendor is set.

Note that go get and go mod tidy do continue to update the go.mod file — and do not support the -mod flag at all. That is because the primary purpose of those commands is to update the go.mod file: they are “explicit user action”. (If you want to install a binary without changing your dependencies, use go install instead of go get.)

@bcmills
Copy link
Contributor

bcmills commented Mar 8, 2021

  1. accesses the network to fetch dependencies when they're in the vendor directory.

That should never be the case when -mod=vendor is set (implicitly or explicitly).

When vendoring is in use, the only times the go command should access the network are when you run an explicit go get command (which has the primary purpose of accessing the network to update your dependencies) or an explicit go mod subcommand (which has the primary purpose of either resolving or reporting information that is not present in the vendor directory).

If you find that a go command other than go get or go mod is accessing the network when -mod=vendor is set (implicitly or explicitly), please file a separate issue with steps to reproduce it.

@bcmills
Copy link
Contributor

bcmills commented Mar 8, 2021

  1. numerous claims by go developers that go modules will allow for removing vendor support.

As far as I am aware, no one on the Go project is claiming that modules will allow for removing vendor support. (If we were planning to remove it, we would not have bothered to implement #33848!)

Per the Background section on #33848:

Go users have built a variety of workflows around vendored dependencies for a variety of use-cases — including self-contained builds, language-agnostic code review, and ephemeral patches — and since the launch of module mode in Go 1.11 they have made it clear that those workflows remain important in module mode.

@bcmills
Copy link
Contributor

bcmills commented Mar 8, 2021

in fact if a vendor directory is detected in the main module during get/install/build you could in fact treat it as the gomodcache unless upgrading or adding a new package (in which case you're interacting with the VCS to populate that cache).

Sorry, but that is not feasible. The vendor directory contains individual packages, not complete modules. It also does not in general include the go.mod files needed to reconstruct the dependencies of the modules containing those packages.

However, note that with -mod=vendor today the go command already does use the vendor directory to load packages and dependency information “unless upgrading or adding a new package”. Also note that -mod=vendor has been the default behavior when a vendor directory is present since Go 1.14. (See https://golang.org/doc/go1.14#vendor.)

@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2021

Regarding the environment variables:

GOPROXY
GOSUMDB

These variables do admittedly add complexity. However, GOPROXY improves the availability of (non-vendored) dependencies, such as for new projects using existing libraries, and GOSUMDB improves security and reproducibility by ensuring that everyone agrees on the exact contents of the versions they require. We believe that those benefits substantially outweigh the cost of the added complexity.

GONOPROXY
GOPRIVATE
GONOSUMDB

GOPRIVATE sets the defaults for GONOPROXY and GONOSUMDB. The latter are finer-grained controls, but most users should only need to bother with GOPRIVATE, which allows users to trade off the security and availability benefits mentioned above against the benefit of being able to work with code that is not publicly available.

GOMODCACHE

The default value for GOMODCACHE is derived from GOPATH — this is another one of those variables that provides a finer-grained control for something that most users will probably set via a coarse-grained

GOINSECURE

GOINSECURE replaces the -insecure flag, which is deprecated (and planned for removal in Go 1.17) because it is too insecure (#37519). So this one isn't adding complexity so much as replacing existing complexity, and it comes with a substantial increase in security.

GOVCS

GOVCS is mostly unnecessary for public dependencies, because the public module proxy currently allows all of the supported VCS tools (with sandboxing to mitigate the security risks of less-well-tested tools).

(GOVCS is a bit like GOINSECURE, in that it allows for finer-grained security policies when updating private dependencies — improving security by default, but still allowing an opt-out for private dependencies on specific trusted hosts.)

@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2021

this is more of an experience report vs a specific bug/proposal

As far as I can tell, many of the points here were either addressed, or explicitly considered and rejected, in #33848.

In order to revisit the behaviors rejected for #33848, we would need to see new information (something not already considered in the design and decisions for that proposal, such as a failure mode or mode of use that we were not aware of at the time), and probably also a concrete proposal of what to do differently.

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 9, 2021
@gudvinr

This comment has been minimized.

@bcmills

This comment has been minimized.

@james-lawrence
Copy link
Contributor Author

james-lawrence commented Mar 29, 2021

@bcmills i'll get around to responding hopefully next weekend. but i think the problem with refering to #33848 is i believe the go team it making fundamental mistakes w/ respect to go modules that make it harder to work with golang tooling and defeating some of the main benefits to the tooling. the last few releases I've run into more and more problems with the tooling than at anytime in the 4 years prior.

and the problems start with how packages are resolved (and particularly the lack of a library for dealing with them), and progress into how all these commands interact with modules.

basically my intuition is telling me that the go team is taking a remote first approach to the resolution vs a local filesystem first approach. which is the source of most of the ergonomics of problems with vendoring. (-mod=vendor is a hack and unintuitive solution and dare I say it entirely unnecessary) and with the replace directive.

i'm particularly frustrated with the fact other attempts at addressing specific problems have been rejected out of hand because the goteam thinks they've considered the problem and 'solved' them. usually by saying 'we'll deal with it late if it comes up' and when it does come up 'we've already considered and rejected that its problem' despite the fact it is a problem and sometimes regressions from previous behavior.

I believe I have some ideas to addressing the issues without causing any major issues going forward. but means the go team needs to take a step back and listen vs decreeing that they've 'already considered the issue'

@james-lawrence
Copy link
Contributor Author

@bcmills sorry for the huge delay in response. I just don't have the time to hand hold this issue I wish i could but life.

i do have a repository that doesn't build cleanly anymore due to go go modules treats replace directives and vendoring dependencies. you can use as an example. unfortunately due to upstream motivations and I believe security issues its unlikely that the changes needed to support my use of said library will ever allow me to pull in upstream. i'm likely going to hard fork the dependency at some point as a result of lack of forward movement on this issue and #40053.

@james-lawrence
Copy link
Contributor Author

the one thing I forgot to mention was my idea for solving the issues presented is predicated on turning vendor directory into a go mod cache, identical to the global one so instead of golang tooling looking at the global cache it'll only consider the modules stored in vendor.

the go.sum would need to keep the quad tuple (mod name, version/go.mod, repository source, checksum) for the replace directive to work properly. as the source repository is important when replacing a module. at the moment it seems to only store the triplet mod name, version + go.mod, and a checksum.

after that I'd move a bunch of the environment variables out into configuration files that live in GOMODCACHE (or go.mod directly) allowing for tooling to pick them up directly and per module. ideally with a settings that disables all network access outside of explicitly updating and adding modules for #40053.

@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2021

It is not feasible to change the vendor directory into a module cache. They are fundamentally different data-stores with different formats and layouts.

It does not seem necessary to expand or redefine the Go module cache for this use-case: we already have the four data you mention; they're just encoded slightly differently than you suggest. Specifically:

  • Instead of the “repository source”, we have the module path from which the module was fetched. (That might not match the current repository, but the difference does not matter as long as the content remains the same.)
  • The checksum for each module is stored in a .ziphash file in the module cache. For public projects, the checksums are served from a public database at https://sum.golang.org.
  • The version is encoded in the file structure of the module cache.
  • The “module name” can be read from the module declaration in the go.mod file, if present. (If no go.mod file is present, there is no canonical module path.)

GOPROXY=off already disables network access, as was suggested to you in #40053 (comment). We are still considering a possibility like GOPROXY=modcache, but that has little to no interaction with the vendor directory.

-mod=readonly (the default since Go 1.16) disables implicit resolution of missing dependencies, which in turn prevents network lookups except for downloading module zip files and go.mod files for dependencies that are already present in the main module's dependency graph.

@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2021

You say you've “run into more and more problems with the tooling than at anytime in the 4 years prior”, but again, I haven't seen new, concrete information here. If you want us to change the go command to address certain problems, you need to be specific about what those problems are, not just how you want to see them addressed.

(#47109 has no apparent connection to the vendor directory, but is closely connected to #30791 which is still open. #44840 was explicitly considered and debated at length in #40276 and #30515. #39009 is very close to #30516, which is still open but again has little to do with vendor.)

@james-lawrence
Copy link
Contributor Author

james-lawrence commented Oct 27, 2021

@bcmills seems like we're talking past each other. my point was holistically the tooling has had a very degraded experience with things just not working. some of that is growing pains for go modules. I get that, and assume things will improve.

this particular issue is about vendoring. and the original problem is still outstanding which is that the replace directive just doesn't function properly. yes its been discussed but it hasn't been fixed. there are real world problems that ignoring it in the tools causes problems; particularly around distribution and maintenance of a tool.

my latest suggestions could potentially be a path forward to fixing said issue. but it certainly will never be fixed unless the source repository is considered. the module path simply isn't sufficient.

I've literally linked to a repository that no longer builds correctly as a result of these changes correct despite having vendored dependencies. even worse is iirc it says it builds correctly, but it builds with the wrong dependency.

I'm not sure what else you'd like me to do. I've given a real world example, the problem itself (along with others generally), and possible ideas to address it.

@bcmills
Copy link
Contributor

bcmills commented Oct 27, 2021

@james-lawrence

this particular issue is about vendoring. and the original problem is still outstanding which is that the replace directive just doesn't function properly.

Sorry, but “doesn't function properly” is not actionable. We need to be able to see the same symptoms you're seeing in order to understand what's going wrong for you. The issue template asks “What did you do?”, “What did you expect to see?”, and “What did you see instead?”. Those questions need specific answers.

You say that you've given a real example that no longer builds, but all I see in support of that for this specific issue is a link to a repository. At what commit did you have the repository checked out? What commands did you use it before, and with what environment variables? What go version did they last work with, and what happens if you run those same commands with a current version of the go command?

Probably to make progress it would be best to start by filing a new issue and filling out the issue template. The original post for this issue lists too many disparate complaints to be actionable as a single issue, and I'm having a hard time following which parts you believe are still unaddressed.

@james-lawrence
Copy link
Contributor Author

sorry given the nature of the conversation up until now it was understood how and why the replace directive didn't function. through the various iterations of the conversation. I'll happily recreate the issue focusing specifically on that aspect at some point in the future when I have time. but the particularly deficiency has been discussed repeatedly and its always ended the same with the golang team basically denying there is a real world problem with their choices.

but i'll happily try again later once i have time to hand hold the issue.

@bcmills
Copy link
Contributor

bcmills commented Oct 30, 2021

I am not asking you to recreate an issue that was previously closed for a design point that was already settled. I am asking to to create a new issue if you have new information about something that isn't working as designed, or an aspect or implication of the design that was not known at the time the design was settled.

@seankhliao
Copy link
Member

Closing as there is no actionable information here.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jun 12, 2022
@golang golang locked and limited conversation to collaborators Jun 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go 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

8 participants