Update/verify dep vendoring#132
Conversation
yes please! |
Tim Hockin (thockin)
force-pushed
the
add-vendoring
branch
from
78740f8 to
2d2c34a
Compare
|
Now vendors all go.mod directories |
Tim Hockin (thockin)
force-pushed
the
add-vendoring
branch
from
2d2c34a to
4785e85
Compare
|
I broke up the large commits into script-changes and run-the-script commits. the "run" commits are just the output of running the script. |
Davanum Srinivas (dims)
left a comment
Davanum Srinivas (dims)
left a comment
There was a problem hiding this comment.
LGTM
the individual vendor/ for each tool ... we'll get used to it.
|
rebased |
Tim Hockin (thockin)
force-pushed
the
add-vendoring
branch
from
4785e85 to
f9e50be
Compare
I'm not convinced. That adds a LOT more bloat, and reviewing tools-module vendor is not nearly as actionable. We're using go mod as an implementation detail of obtaining those tools, we could just as well fetch binaries. |
|
Also, we shouldn't be dropping anything from LICENSES here, what's going on there? |
| @@ -1,27 +0,0 @@ | |||
| // +build windows solaris | |||
There was a problem hiding this comment.
The code in LICENSES was here because the library is MPL and requires redistributing the sources. I think this commit broke something.
There was a problem hiding this comment.
Running go mod vendor causes go-licenses to NOT create these files. No logs, no warnings. Adding --include_tests does not bring it back (though we should probably add that).
Given that these are tests, they are not linked into our project, so I think the need for them is dubious.
|
Github won't show me the full diff, even commit by commit. |
|
Benjamin Elder (@BenTheElder) i switched to |
|
Also, the tools modules are supposed to be updated by updating the version of the tool and letting the other deps be auto selected to the versions specified by the to upstream. We have a script for that. Which avoids breakage caused by patching those tools in unexpected ways. So updating the tools will create a huge churn each time as ~all transitive deps update at once. I would rather work out binary fetch for each than pretend to review their vendor. If the concern is those tools stop building because the module proxy is missing a dependency, we should use binaries. If those are a concern, well we have bottom turtles we need to download for our images too. |
|
One of the motivations for vendoring is "I am at the airport. I quickly clone the repo and get on the plane. I can do work." If I can't compile, that's a problem? |
|
We don't vendor the base images though. |
This is needed when go-licenses fails to recognize standard library packages as part of Go's core. This breaks when using a versioned or managed Go toolchain (e.g., via Go 1.21+ automatic toolchain downloads) that places standard library files outside your main GOROOT path.
|
I moved the commits to vendor tools to the end of this series. Other than that, I think it's good. |
Tim Hockin (thockin)
force-pushed
the
add-vendoring
branch
2 times, most recently
from
79177e4 to
8e0c849
Compare
|
In fact, I pushed without the tools now. Let's debate that in a followup. |
This removed go code from within LICENSES, but it was not caused by this commit. It would get removed as soon as we ran `go mod vendor` - I guess go-licenses decided internally (no logs or anything) that vendoring is good enough. But these are tests - they are not linked in anyway.
Tim Hockin (thockin)
force-pushed
the
add-vendoring
branch
from
8e0c849 to
c47776b
Compare
3 participants
This adds an update/verify script pair to check
go mod vendorin the main module. Should we do this in sub-modules too?Fixes #19