sheredom / subprocess.h

🐜 single header process launching solution for C and C++
The Unlicense
1.1k stars 97 forks source link

Should subprocess_option_search_user_path use the PATH from any environment to subprocess_create_ex? #63

Closed sheredom closed 2 years ago

sheredom commented 2 years ago

@nburrus curious on your take on this - your test case custom_search_path sets the PATH in a custom enivornment, and then calls subprocess_create_ex with this PATH and subprocess_option_search_user_path - which only works if you use the PATH provided in environment. I'm not sure if this is the best approach or not, trying to get my head around it at present. I don't think this would work on windows because we don't and won't modify the PATH, and it is affecting me trying to move over to posix_spawn.

Any thoughts? At present my thinking is I'll rework the test so that you cannot use a PATH specified in a custom environment.

nburrus commented 2 years ago

Mmmm so the way I actually use it in my app is with the subprocess_option_inherit_environment, as in that case it just leaves the global PATH variable untouched (typically set from the shell and not even from the app) and it works as expected. I guess this would be the most common use case for subprocess_option_search_user_path.

But yes otherwise if the user does not use subprocess_option_inherit_environment, then the PATH has to be set in the custom environment, since execvp will only look in the PATH of the current environ variable. I used that approach in the unit test to avoid any uncertainty due to the calling environment.

I'm not sure how it works on Windows actually, the documentation of CreateProcessA is a bit unclear. Will it always search in the PATH variable of the parent process, or in the PATH variable of the environment provided to CreateProcessA? I would expect it to behave like execvp? If not I guess we could make the Unix one behave similarly by always inheriting the PATH variable when subprocess_option_search_user_path is set, and forbidding a PATH in the custom environment, as you suggested.

(Btw about posix_spawn, the documentation says that posix_spawnp behaves like execvp regarding the path search, so I'm curious about the kind of issue you're running into?)

sheredom commented 2 years ago

Ah so the issue is a fun one - basically the way it works now:

Whereas with posix_spawnp:

nburrus commented 2 years ago

ah interesting! I realize that I actually got confused in my previous answer and forgot that execvp does NOT take an environment as a parameter, so I had to do that environ overwriting hack. Initially I actually wanted to call execvpe which would have avoided the environ variable overwrite and has the behavior of always searching in the PATH variable of the parent process, regardless of the custom environment provided to execvpe. So basically like posix_spawnp and I now consider this the "right" behavior.

So now I see that the way I emulated execvpe by overwriting environ is not great and I agree that specifying a PATH variable in the custom environment should not change the way the program is searched in the parent process path. If someone wants to change the PATH then it needs to adjust the parent environ variable instead, and this has nothing to do with subprocess.h.

So based on this I see two solutions:

1) Implement a version of execvpe ourselves since the GNU version of the libc is not standard. I actually gave this a try by importing the code from musl https://github.com/esmil/musl/blob/master/src/process/execvp.c . You can see it in this branch: https://github.com/nburrus/subprocess.h/commit/545580666a160445e3de534aba34653208319997

2) Just switch to posix_spawnp and forget about that issue.