-
Notifications
You must be signed in to change notification settings - Fork 58
feat: automatically configure vendor mode #74
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#!/bin/sh | ||
# go.sh is a wrapper for the Go command that will | ||
# automatically set the required module related flags | ||
# when a vendor folder is detected. | ||
# | ||
# Currently, in Go 1.18, Go Workspaces are incompatible | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I am kinda thinking how all sh scripts can be avoided but I thought vendor files were compatible with workspaces, it is intriguing. I think http templates are super simple compared to past sh scripts before 1.18 and this amount seems OK, I will chase this incompatibility issue. Looks good to me in this way tho, it is more clear than old shell scripts I will be kinda confused again but I thought -mod=vendor was the default flag when Go detects vendor folder since 1.13, when did this actually change so we had to specify it explicitly? I am very bad vendor user(not using at all since 1.13) If you have a look at the footnotes, it says -mod=vendor is the default here https://www.ardanlabs.com/blog/2020/04/modules-06-vendoring.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually -mod readonly also reads the vendor as far as I understand from this thread golang/go#30404, so I think shell script may no longer needed? the end user needs to do -mod=readonly as you pointed out here Lucas https://github.com/LucasRoesler/openfaas-golang-template-workspace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mrwormhole i don't 100% follow. Were you able to get a workspaces project to build with the vendor by using The main reason for the script, is that when using workspaces, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -mod=readonly should use vendor if vendor exists, I also don't know why this config needs to specify -mod=vendor to get vendor files, it is automatic when not specified, default is -mod=readonly, -mod=mod is the one who only looks up for go modules and ignores vendor completely There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand what you are saying, but
Perhaps you can share a working example repo with the changes you would like to see? When I tried to use vendor with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so I actually created the same repository but without -mod vendor flag, I vendored my dependencies (which is the user responsibility) and deleted -mod=vendor and made go modules ON, it actually detected and built with vendor folder but the key here was I deleted the repository of my module, I do think -mod vendor is completely unneeded and shouldn't be used. https://github.com/MrWormHole/openfaas-golang-template-workspace the end-user should be able to know as a Go dev, what could be the use case for enabling go modules and doing -mod vendor. Vendor folder should exist only for disaster recovery, else it will download all deps with go.mod and not use vendor folder. Also go mod vendor should be called explicitly by developers and stored in the repository, else it is a problem There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we can move forward without using additional bash and if statements, I'd be interested to see that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Lucas is right @LucasRoesler depending on your Go version it gets set to default to -mod=readonly, I would keep it and resolve these comments. It will enhance the dev usage of these templates even though it sets it internally. I am on version 1.17.11 and -mod=mod always runs when I don't specify -mod=vendor with go modules "OFF" or "ON", sorry for the confusion I had earlier. One theory from my side is that alpine images(Dockerfile this uses) of 1.18 sets it to -mod=mod, I didn't confirm yet today but if you fork it, you will see this one builds https://github.com/MrWormHole/openfaas-golang-template-workspace without on or off specification because -mod flag figures it out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mrwormhole can you clarify your recommendations in a few bullets or steps, or are you retracting what you said? |
||
# with vendored modules. As a result, we must disable | ||
# Go modules and use GOPATH mode. | ||
|
||
# We use this bash script to wrap Go commands because | ||
# there is no clear way to set an env variable from the | ||
# a script in a Dockerfile. | ||
# It is possible to use `go env -w` but, but env varaibles | ||
# have precedence and if it is set as an arg/env variable, | ||
# then it will be ignored by the `go env -w`. | ||
|
||
# if the function/vendor folder exists | ||
# then we set the env variables for | ||
# GO111MODULE=off | ||
if [ -d "/go/src/handler/function/vendor" ]; then | ||
echo "Setting vendor mode env variables" | ||
export GO111MODULE=off | ||
fi | ||
|
||
# if DEBUG env is 1, print the go env | ||
if [ "${DEBUG:-0}" = "1" ]; then | ||
go env | ||
fi | ||
|
||
go "$@" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#!/bin/sh | ||
# go.sh is a wrapper for the Go command that will | ||
# automatically set the required module related flags | ||
# when a vendor folder is detected. | ||
# | ||
# Currently, in Go 1.18, Go Workspaces are incompatible | ||
# with vendored modules. As a result, we must disable | ||
# Go modules and use GOPATH mode. | ||
|
||
# We use this bash script to wrap Go commands because | ||
# there is no clear way to set an env variable from the | ||
# a script in a Dockerfile. | ||
# It is possible to use `go env -w` but, but env varaibles | ||
# have precedence and if it is set as an arg/env variable, | ||
# then it will be ignored by the `go env -w`. | ||
|
||
# if the function/vendor folder exists | ||
# then we set the env variables for | ||
# GO111MODULE=off | ||
if [ -d "/go/src/handler/function/vendor" ]; then | ||
echo "Setting vendor mode env variables" | ||
export GO111MODULE=off | ||
fi | ||
|
||
# if DEBUG env is 1, print the go env | ||
if [ "${DEBUG:-0}" = "1" ]; then | ||
go env | ||
fi | ||
|
||
go "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should still mention that
GO111MODULE
will be forced tooff
.auto-magic can end up being more confusing than setting explicit values.