-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: automatically check and use vendored packages #33848
Comments
This comment contains my original proposal. An updated version can be found below (in #33848 (comment)). ProposalI propose to enable In order to detect when the Concretely, the changes are:
The proposed approach allows the following commands to work from a
Other commands may fail without an explicit
Note that the Example workflow
CaveatsIntentional modificationsThe above checks do not detect intentional modifications to the vendored sources. Users who want to ensure that no intentional modifications are present will need to re-run Local filesystem changesThe above checks also do not detect skew due to changes in local files within a directory specified in a If we believe that changes in the local filesystem will be a significant source of skew in practice, we could expand the Dependency analysisBecause the approach proposed here does not retain That seems like an acceptable tradeoff: adding those |
One point I'm a bit uncertain on: above I proposed to trigger the automatic Would it be better to instead trigger it based on a |
That seems like it would be simpler to understand, including because I think it has been previously stated that the format of the modules.txt file is an internal and undocumented implementation detail? Could you even take it a step further to have it trigger off of the presence of the vendor directory itself plus It would then be triggering off of things that are both more visible and actually documented, and hence would likely be easier to understand. |
When I build in a container, I will be copying the entire projects source tree of code into that container manually. This will cause things to be out of sync. So now, I will have to make sure that I don't copy the This also means that anyone else on my team working with this project, will need to run throught the proxy server to validate their vendor folder is "correct". I am concerned that in the end, the vendor folder is not really acting as a vendor folder. If the module system doesn't like what it sees in vendor, the build breaks. The deps needs to be cached anyway. So might as well not even use vendor. |
I think I'm missing something. How would copying the entire source tree into the container cause the |
How are you going to verify that the vendor folder and the |
@ardan-bkennedy, that's all covered in the text of the proposal above (#33848 (comment)). |
I read that several more times and I think it’s starting to make sense. What the above proposal is saying is, given a vendor folder with a modules.txt file, just verify that this file is in sync with the go.mod file? If they are, build against the vendor folder. If they are not, issue a build error? Therefore no calls to the proxy server are required. The original workflow is maintained? |
Yep. Being able to do that without hitting the network at all requires that we include some extra information in the |
That's brilliant and allows vendoring to be an option again. Which is much appreciated. So the next question is, will this make a point release for 1.13 or do we need to wait for 1.14? My concern is that projects may just give up on vendoring because of the current breakage in workflows. Which is a shame because I think vendoring has strong advantages when it's reasonable to use it. |
This would not be backported to 1.13. (Per the minor release policy, we only backport “security issues, serious problems with no workaround, and documentation fixes”.) In the interim, users who rely on vendoring workflows can enable vendoring for all build commands by setting |
It sounds like the general consensus here is that we should accept this proposal for Go 1.14. Am I reading this wrong? Does anyone object to accepting this proposal? Thanks. |
An interesting wrinkle: if a This implies that we need to duplicate (and verify) all If we're not loading transitive dependencies, we don't know whether a replaced module actually exists in the build list, so we can't add an entry of the form Fortunately, the old
|
Well that's funny. The current go/src/cmd/go/internal/modcmd/vendor.go Line 72 in bdf0fe5
I think that means that |
Change https://golang.org/cl/198319 mentions this issue: |
I have updated the proposal below, based on information learned during prototyping. This version supersedes the one in #33848 (comment). ProposalI propose to enable In order to detect when the Concretely, the changes are:
The proposed approach allows the following commands to work from a
Explicitly build-list related commands will continue to download missing modules to the cache:
Other commands may fail without an explicit
Note that the Example workflow
CaveatsIntentional modificationsThe above checks do not detect intentional modifications to the vendored sources. Users who want to ensure that no intentional modifications are present will need to re-run Local filesystem changesThe above checks also do not detect skew due to changes in local files within a directory specified in a If we believe that changes in the local filesystem will be a significant source of skew in practice, we could expand the Dependency analysisBecause the approach proposed here does not retain That seems like an acceptable tradeoff: adding those |
Change https://golang.org/cl/198318 mentions this issue: |
In #33848, we propose to use 'go 1.14' in the go.mod file to enable new default behavior. That means that 'go mod init' needs to start generating that directive by default, which requires the presence of the updated version tag in the build environment. Updates #33848 Change-Id: I9f3b8845fdfd843fd76de32f4b55d8f765d691de Reviewed-on: https://go-review.googlesource.com/c/go/+/198318 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
The |
Updated #33848 (comment) and the prototype CL to omit the |
Change https://golang.org/cl/207397 mentions this issue: |
Updates #33848 Change-Id: I505490906be7cd6fbcdc6a05c1017c779dbf7bba Reviewed-on: https://go-review.googlesource.com/c/go/+/207397 Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Change https://golang.org/cl/210341 mentions this issue: |
This location was missed in CL 204521. Updates #33326 Updates #33848 Change-Id: I0ece6d9b37548d8abb54f79c69be5548a0428c76 Reviewed-on: https://go-review.googlesource.com/c/go/+/210341 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Change https://golang.org/cl/214081 mentions this issue: |
…endor' is set The information requested by these flags is not available from the vendor directory. Noticed while diagnosing #36478. Updates #33848 Change-Id: I2b181ba5c27f01fdd6277d8d0ab1003c05774ff7 Reviewed-on: https://go-review.googlesource.com/c/go/+/214081 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Change https://golang.org/cl/222538 mentions this issue: |
The rsc.io/pdf package was added to the vendor directory 5 years ago in CL 13968, back when the vendor directory was a part of the Go 1.5 vendor experiment. The vendor.json file was written by hand in a format compatible with the govendor tool. By now, that tool has been deprecated in favor of the go command in module mode. A go.mod file requiring a newer version of rsc.io/pdf was added in CL 167137. Modify the vendor directory to use the newer rsc.io/pdf version by recreating vendor directory using go command in Go 1.14: rm -rf vendor go mod vendor It's possible the vendor directory isn't needed anymore and can be safely deleted in a future change. The scope of this CL is only to migrate from the deprecated govendor tool to the supported go tool; deleting the vendor directory can still be done in a future change. Remove the explicit mention of Go 1.5 vendor experiment from README since it's old, and not relevant by now. Add a contributing section. Updates golang/go#30228 Updates golang/go#33848 Change-Id: I95de95a3b2d81faf7235c675e5b8d425141f8d7a Reviewed-on: https://go-review.googlesource.com/c/arch/+/222538 Reviewed-by: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
Change https://golang.org/cl/251159 mentions this issue: |
forceStdVendor was a special-case mechanism to allow Go contributors to use vendored dependencies by default when working in GOROOT/src. As of Go 1.14,¹ the 'go' command uses vendored dependencies by default within all modules, so the 'std' and 'cmd' modules no longer need to be special cases, and we can remove this special-case code. ¹ https://golang.org/doc/go1.14#vendor Updates #33848 Updates #30241 Change-Id: Ib2fb5841c253113b17fa86a086ce85a22ac3d121 Reviewed-on: https://go-review.googlesource.com/c/go/+/251159 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Abstract
This is a proposal to enable the use of the main module's
vendor
directory, by default, in module mode. This proposal replaces #30240 and #29058, and subsumes #27227.Background
The Go 1.5 release included support for a
vendor
subdirectory for each package subtree withinGOPATH
. Avendor
directory, if present, implicitly changed the interpretation ofimport
statements within that subtree to always refer to packages within thevendor
directory if present.In module mode, the meaning of each import is instead determined by the dependencies of the main module used for the build. However, from the first release of module mode (in Go 1.11), the
go
command has included ago mod vendor
subcommand, which populates avendor
directory at the root of the main module with source code from those dependencies; and a-mod=vendor
flag, which instructs thego
command to use that source code for building.In module mode, unlike
GOPATH
mode, thevendor
subdirectories of other packages and modules are ignored — only thevendor
directory at the root of the main module is used. This gives the author of the main module complete control over their dependencies viarequire
andreplace
directives.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.
However, vendoring in module mode today has a few rough edges:
A maintainer who introduces a new
import
statement may easily and accidentally introduce skew between the versions in thego.mod
file and those in thevendor
directory (cmd/go: the vendor/ folder should be kept up to date if present #29058).std
andcmd
modules in the standard library, but it relies on some assumptions about the dependencies ofstd
andcmd
that do not hold in general.The maintainers of projects that use vendoring expect the
vendor
directory to be used by default (cmd/go: should default to use mod=vendor flag if the vendor folder exists #27227), but today in module mode that requires users to set-mod=vendor
, either explicitly on the command line or in aGOFLAGS
variable. For users working on multiple modules with different vendoring strategies, setting the variable may require repeated intervention over the course of the day.This proposal attempts to address those rough edges.
Proposal
The concrete proposal is posted in a comment below. (Any updates will be linked from here.)
Latest update: #33848 (comment)
(CC @jayconrod @ianthehat)
The text was updated successfully, but these errors were encountered: