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: os/exec: add LookPath equivalent where PATH can be specified #60604

Open
peterebden opened this issue Jun 5, 2023 · 2 comments
Open
Labels
Milestone

Comments

@peterebden
Copy link

os/exec.LookPath contains robust logic for looking up paths cross-platform. However, it is always based on the value of the PATH environment variable; there are cases where one might like to employ the same logic but using a PATH that has come from elsewhere. My current motivating example relates to remote build execution where it may be based on an incoming RPC, I expect others to exist in the wild as well.

In terms of prior art, this seems a little similar to os.Expand and os.ExpandEnv which offer the same option of using the environment or not. We can't match the naming convention of those though so the best suggestion I have is exec.LookPathFrom(file, path string) (string, error).

I anticipate the implementation of this being straightforward, and it shouldn't represent significant additional maintenance in future. I'm of course happy to raise the PR if this proposal is accepted.

Alternatives

In the absence of this function, one could set the PATH variable to the desired value, use LookPath, then reset it back again. This is slightly more work but more importantly is mutating global process state, and is therefore not safe to use if multiple goroutines might execute the same code concurrently.

Possibly a more likely solution is that you'd write some code to do this yourself. That isn't so hard on the face of it, but looking at the actual implementation in exec, it seems unlikely that most implementations would be diligent enough to mimic all the cross-platform nuances or the various edge/error cases that are checked. It would of course also be nice to get updates or fixes to it as well (the command path security changes from 1.19 would be a good example of that).

@gopherbot gopherbot added this to the Proposal milestone Jun 5, 2023
@seankhliao
Copy link
Member

We've declined this before in #20081 #59753.
Is there any new information that should make us reconsider this?

@peterebden
Copy link
Author

Apologies, I did not find those issues when opening this.

I guess that I have nothing new. I will simply reiterate dissatisfaction with the previous reasoning for rejecting this - the existing code is several hundred lines across 4 files (plus 4 more for tests), I think it's disingenuous to write that off as "it's easy to copy". As I tried to outline before, it seems unlikely to me that a copy would keep all the nuance (am I going to copy and maintain a file for plan9 which I'll never use and have no way of testing?) or keep up with updates, and the cost of adding this seems to me to be low.

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