hack: replace go-mod-prepare.sh with wrapper script#44497
hack: replace go-mod-prepare.sh with wrapper script#44497thaJeztah merged 2 commits intomoby:masterfrom
Conversation
fda3346 to
c77cc24
Compare
|
Pushed up a new version that adds a new validate step to go with the removal of go.mod from |
c77cc24 to
9710ee2
Compare
|
@neersighted needs a rebase |
|
This is stacked on #44459; do you want to land that and I'll rebase this on top of the merged branch? |
Oh! My bad; I'll have a look at that one. |
|
The other one was merged; so this one can be rebased 👍 |
9710ee2 to
3082be9
Compare
|
Rebased 👍 |
|
Looks like shfmt isn't happy 🙈 |
To make the local build environment more correct and consistent, we should never leave an uncommitted go.mod in the tree; however, we need a go.mod for certain commands to work properly. Use a wrapper script to create and destroy the go.mod as needed instead of potentially changing tooling behavior by leaving it. If a go.mod already exists, this script will warn and call the wrapped command with GO111MODULE=on. Signed-off-by: Bjorn Neergaard <[email protected]>
Moby is not a Go module; to prevent anyone from mistakenly trying to convert it to one before we are ready, introduce a check (usable in CI and locally) for a go.mod file. This is preferable to trying to .gitignore the file as we can ensure that a mistakenly created go.mod is surfaced by Git-based tooling and is less likely to surprise a contributor. Signed-off-by: Bjorn Neergaard <[email protected]>
3082be9 to
25c3421
Compare
| # top-level go.mod is not meant to be checked in | ||
| /go.mod |
There was a problem hiding this comment.
Still slightly on the fence if this could cause accidentally adding the go.mod (if the script failed at some point etc), but I'll guess we'll see how it goes 🙈
There was a problem hiding this comment.
hack/validate/no-module added in this PR would catch those cases. Though it will need to be added to hack/validate/default to be run in CI?
There was a problem hiding this comment.
I excluded it from default because some users may want to be able to have the tooling function with a go.mod present but not committed. It's run in CI as there is a file-based glob. However, we could add it to all (the same step where vendoring is run).
There was a problem hiding this comment.
It's run in CI as there is a file-based glob.
@neersighted I don't see it (neither in the current git HEAD, nor in the source tree during the state when this PR got merged.
|
@neersighted could you prepare a 🍒 ⛏️ for this one (and the previous PR)? |
- What I did
To make the local build environment more correct and consistent, we should never leave an uncommitted go.mod in the tree; however, we need a go.mod for certain commands to work properly. Use a wrapper script to create and destroy the go.mod as needed instead of potentially changing tooling behavior by leaving it.
Also add a script to prevent go.mod from sneaking back in; as we are not a Go module, and cannot currently become one, we want to prevent new contributors from mistakenly trying to turn Moby into one.
- How to verify it
Manually, more or less.
- A picture of a cute animal (not mandatory but encouraged)
