validate/vendor: always tidy#44459
Conversation
neersighted
force-pushed
the
validate_always_tidy
branch
from
5f2ec0c to
d54c61b
Compare
neersighted
force-pushed
the
validate_always_tidy
branch
2 times, most recently
from
80bf5bf to
e6e2219
Compare
|
|
||
| set -x | ||
|
|
||
| tee "${ROOTDIR}/go.mod" <<EOF |
There was a problem hiding this comment.
Was there a specific reason to use tee for this? This also prints the go.mod on stdout (whereas the old one only wrote it to the file)
There was a problem hiding this comment.
The tee is intended to increase transparency (along with the set -x) for debugging/CI. It's a bit more noise, but I do think it makes the steps being run much more clear to the user.
| help() { | ||
| printf "%s:\n" "$(basename "$0")" | ||
| echo " - tidy: run go mod tidy" | ||
| echo " - vendor: run go mod vendor" | ||
| echo " - all: run tidy && vendor" | ||
| echo " - help: show this help" | ||
| } |
There was a problem hiding this comment.
We should really have a vendor (and tidy ?) target in the makefile, so that we have a canonical "help" (we already have make help).
There was a problem hiding this comment.
The current docs say to use hack/vendor.sh -- I'd rather slowly chip away at technical debt in the build system and think about what would be more maintainable in the long term without adding too much that is "new" before we expose more/depend more on these implementation details.
That all goes to say that I think the status quo here is fine and the long term intention is to replace/refine this further, as we have logic spread out among Dockerfiles, shell scripts, Makefiles, and more, and it's very hard to sustain/reason about.
neersighted
force-pushed
the
validate_always_tidy
branch
from
e6e2219 to
55a4b3b
Compare
|
Pushed up a new version I think is good as-is, let me know if you have any objections to my responses on the review comments. |
Signed-off-by: Bjorn Neergaard <[email protected]>
Signed-off-by: Bjorn Neergaard <[email protected]>
Signed-off-by: Bjorn Neergaard <[email protected]>
neersighted
force-pushed
the
validate_always_tidy
branch
from
39fecd5 to
5d0bd5c
Compare
|
Pushed a squashed version and also pushed up #44497 to encapsulate the fake go.mod. |
Signed-off-by: Bjorn Neergaard <[email protected]>
neersighted
force-pushed
the
validate_always_tidy
branch
from
5d0bd5c to
af8e955
Compare
|
Pushed up a new version that passes shfmt. |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
seems that it works better if I actually submit my reviews, and don't keep them as "pending" 😂 😂 😂
| tidy) tidy ;; | ||
| vendor) vendor ;; | ||
| ""|all) tidy && vendor ;; | ||
| *) help ;; |
There was a problem hiding this comment.
technically this is a bit of a breaking change; the hack/vendor.sh script (when we were using vndr) allowed for a specific dependency to be specified to skip doing a "full" revendor, e.g.
hack/vendor.sh github.com/containerd/containerdCalling it like the above would re-vendor containerd (and its dependencies), but not do a "full" cycle (download all dependencies and update the vendor for all of them).
I don't think that will be an issue (and don't think there's a 1:1 equivalent with modules without updating the dependency), but we can revisit that; we could either change this to;
| *) help ;; | |
| help) help ;; |
Or re-purpose that option to go get <name of passed dependency>@latest (or something along those lines)
There was a problem hiding this comment.
I don't mind introducing that in another PR; but I'm not sure how useful it is given the speed of a hot module cache (since local development is the most likely usage).
|
|
||
| set -x | ||
|
|
||
| tee "${ROOTDIR}/go.mod" << EOF |
There was a problem hiding this comment.
Don't think we should change right now, but I was wondering if we could use a regular go mod init for this (or go mod init -go "$GO_VERSION") instead of manually crafting the go.mod;
go mod init github.com/docker/docker
go: creating new go.mod: module github.com/docker/docker
go: to add module requirements and sums:
go mod tidyBut I guess that would complicate things if the go.mod was already there (well we could ignore the error, but not sure if that's good);
go mod init github.com/docker/docker
go: /Users/thajeztah/Projects/docker/go.mod already exists
echo $?
1There was a problem hiding this comment.
I think creating exactly what we need is better than depending on the go tool, in case something changes.
3 participants
- What I did
Prevent #44453 or similar by always running
go mod tidywhen checking vendoring.- How I did it
Allow running individual steps in
hack/vendor.shwhile still using it as the exclusive wrapper forgo modby introducing 'subcommands'; the default behavior is identical to the previous (which is now aliased as theallsubcommand).Make
hack/validate/vendortake advantage of these split steps, and refactor it to be more robust against whitespace/follow bash best practices.- How to verify it
Manually; revert #44453 locally to test tidy detection, and commit a modification to a vendored file to test re-vendor detection.
- Description for the changelog
N/A
- A picture of a cute animal (not mandatory but encouraged)
