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/vet: warn if cmd.Dir is set after calling cmd.Environ #63782

Open
cherrymui opened this issue Oct 27, 2023 · 1 comment
Open

proposal: cmd/vet: warn if cmd.Dir is set after calling cmd.Environ #63782

cherrymui opened this issue Oct 27, 2023 · 1 comment
Labels
Milestone

Comments

@cherrymui
Copy link
Member

Suppose cmd is a *os/exec.Command. On platforms that uses the PWD environment variable (non-Windows non-Plan 9 platforms), cmd.Environ sets PWD if cmd.Dir is set and cmd.Env is not already set. cmd.Environ has an example suggests that cmd.Dir should be set before calling cmd.Environ. But this is not enforced.

cmd.Environ is usually used to set cmd.Env. If that is done before cmd.Dir is set, the PWD environment variable will not be set correctly. Many programs probably don't care about PWD. But for ones that do care, this may lead to subtle and surprising behavior, e.g. how symlinks are handled.

It would be nice if we can have a way to check cmd.Dir is set after calling cmd.Environ. Ideally that check only fires if cmd.Environ is called for setting cmd.Env (probably most of them are).

It would be nice if we can also handle functions wrapping cmd.Environ, like internal/testenv.CleanCmdEnv. Not sure how hard the implementation will be.

One downside for this is that many programs probably don't care about PWD, so setting cmd.Dir after would not lead to incorrect behavior. On the other hand, cmd.Environ is relatively new (added in Go 1.19), so maybe only programs that this matters use cmd.Environ?

This is from a discussion on CL 535915. cc @bcmills

@gopherbot gopherbot added this to the Proposal milestone Oct 27, 2023
@timothy-king
Copy link
Contributor

  1. "It would be nice if we can have a way to check cmd.Dir is set after calling cmd.Environ."
  2. "Ideally that check only fires if cmd.Environ is called for setting cmd.Env (probably most of them are)."
  3. "It would be nice if we can also handle functions wrapping cmd.Environ, like internal/testenv.CleanCmdEnv."

My hunch is that adding all of the above requirements together would require an inter-procedural type state analysis. It is not impossible, but it does sound tricky.

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

No branches or pull requests

3 participants