Skip to content
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

proposal: cmd/go: GOWORK env var should be off by default #51967

Open
MatthewJamesBoyle opened this issue Mar 26, 2022 · 4 comments
Open

proposal: cmd/go: GOWORK env var should be off by default #51967

MatthewJamesBoyle opened this issue Mar 26, 2022 · 4 comments
Labels
Projects
Milestone

Comments

@MatthewJamesBoyle
Copy link

@MatthewJamesBoyle MatthewJamesBoyle commented Mar 26, 2022

As of go 1.18, if you have a go.work file present at the time of running go run, it will use the module referenced in the go.work file. This means, in my opinion, it is more likely that people will take code intended for local development and deploy it into a production environment.

For example, you might use a go.work file to override your complex auth module to always return true locally. if you forget to set GOWORK=off when running go run ..., you will use this module and there is no warning in the console that it has happened. For those less familiar with the go ecosystem , perhaps who are cloning and running someone else's project for the first time, it may not even be entirely clear that this is happening.

Good practise dictates you should use a binary in production environments and probably not commit your go.work file, but that's all it is, good practise. There is nothing stopping someone simply cloning a repo and running go run on a server and I have done this for side projects in the past.

I therefore propose GOWORK should alway be off by default, and if you want to use it, you should explicitly flag it on when executing the go run command such as:
GOWORK=on go run ...,

@gopherbot gopherbot added this to the Proposal milestone Mar 26, 2022
@MatthewJamesBoyle MatthewJamesBoyle changed the title proposal: --workfile flag should default to off and have to be explicitly turned on. proposal: GOWORK env var should be off by default. Mar 26, 2022
@ianlancetaylor ianlancetaylor changed the title proposal: GOWORK env var should be off by default. proposal: cmd/go: GOWORK env var should be off by default Mar 26, 2022
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 26, 2022

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Mar 26, 2022
@ianthehat
Copy link

@ianthehat ianthehat commented Mar 27, 2022

I don't think a committed go.work adds any extra danger over the things you can already do in your go.mod. You could already do this with a bad replace. If anything go.work reduces the danger, as you say you should not be committing it so it allows you to have a development replace that will not make it to production. Even that is a bad practice though, that kind of functionality switch should be done with a build tag or configuration that is off by default and you are certain will not make it to production or other users machines.
I don't think bad practice producing bad results is something we can reasonably defend against, and it is not worth making the tools less ergonomic for people applying good practices even if it did.

@MatthewJamesBoyle
Copy link
Author

@MatthewJamesBoyle MatthewJamesBoyle commented Mar 27, 2022

Thanks for sharing your thoughts. I hear you on making the tool less ergonomic for those who use it well and is something I thought quite a bit before raising this proposal. The reason I still raised it as if I view the go tooling through the eyes of a beginner, I can see confusion and bad things happening with the current setup.

If you know nothing about go or just the basics if you try and run an application it's going to spit out some error message about go modules or vendoring. This will lead you to read up on go.mod and likely take a look at it. Before go 1.18, everything module related would be in go.mod. You could have a replace directive in here (and you still might, for other reasons than go.work) but at least it was all in one place. If there does happen to be a go.work file you do not receive any error messaging (or messaging at all) when you run go run ... so it would lead you to a confusing google search as you try and figure out what is going on.

Perhaps instead of making the go tooling less ergonomic for users who use it well, the same problem could be solved by providing a more verbose output by default when a go.work file is used. For example, the go tool could log "running in workspace mode. For more information please see https://go.dev/doc/tutorial/workspaces for more info"

Do you think this could be a better solution?

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Mar 28, 2022

I also don't think defending against bad practice is reasonable (if anything, it should be made more painful so people do it less). "everything module related would be in go.mod" is already not true, there is global config in the environment and env file that can affect the build process, and we've seen people get confused if they've previously copy-pasted random commands off the internet and forgot about them.

More logging is also unergonomic: I now have unexpected output that needs to filtered out if I'm passing the output anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Proposals
Incoming
Development

No branches or pull requests

5 participants