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

ability to pass custom environments to processes #1

Closed
graphitemaster opened this issue Dec 21, 2017 · 8 comments · Fixed by #38
Closed

ability to pass custom environments to processes #1

graphitemaster opened this issue Dec 21, 2017 · 8 comments · Fixed by #38

Comments

@graphitemaster
Copy link

would be nice if you could pass custom environments to the processes by passing a list of environment variable strings or NULL to inherit the current processes environment.

@sheredom
Copy link
Owner

Yeah its on my TODO list (in my head) - worry not!

@sheredom
Copy link
Owner

Quick question - it's not thread safe to inherit the environment of the parent and have the child process append to the environment of the parent process's environment.

Would it be ok for your usecases that the child process would not inherit the parent's environment, and instead if you wanted that as a user you'd have to pass along the environment variables yourself?

@graphitemaster
Copy link
Author

That isn't the approach you would do. The process construction methods themselves allow you to pass an environment list, when you pass NULL they inherit by default the parent's environment. On windows this would be done by creating an environment block which is just a single block of string separated by null terminator characters. The end of the block must be an empty string (so the block end has two null termination characters.). On POSIX systems there's two ways, the easy way and the correct way, the easy way would be to just pass everything through /bin/env as a string piping it to the actual process you create with execl. The correct way however is to just specify a list of strings (terminated by an empty string) and use execvpe instead. You do not have to call any of the setenv/putenv/etc functions here.

@sheredom
Copy link
Owner

Right but the issue is that if you specify any environment variables at all then any environment variables that were set in the parent are not passed down.

I'm thinking of the case where you want to spawn a process but specify MY_SPECIAL_ENV_VAR but also be able to read the PATH variable that was in the parent's context - the ways in which you do this in Wiindows/Linux are both inherently thread unsafe (you basically have to change the env of the parent process, spawn the child, then reset the env of the parent from my experience!).

@graphitemaster
Copy link
Author

That isn't a useful operation. You can always read the parents's environment and copy them when you make your list before spawning the child process. This is actually required on Windows because Window's environment block does not permit you to override an environment variable that is already in the block. If you have FOO already and then supply your own FOO, it won't override. You have to meticulously hand craft the block not to contain duplicates. So it's not a 'safe' operation to blindly inherit the parent's environment and also accept user supplied ones that could conflict with what is already there. It's all or none as far as I understand. You eiher have all the parent's environment block upon child process construction or none of it.

@sheredom
Copy link
Owner

Gotya! Glad we had some back and forth on this, I'm pretty sure we're on the same page now 😄

I think I'll add some option flags var that controls whether you inherit the parents environment or not - at present we are always inheriting which doesn't seem great!

@sheredom
Copy link
Owner

sheredom commented Jan 9, 2018

As the first part of this, I've created an MR that allows you to specify whether the parent environment should be inherited or not #6.

I'll then do a follow-up for setting individual environment variables.

@ha11owed
Copy link
Contributor

I also needed this feature on Linux and created a quick an dirty way to pass the env variables. The environment variables are ignored when the inherit option is set.

The approach that I took can be found here:
ha11owed@c95cf4e

As a note:
I was also considering creating a dedicated struct for all the parameters that need to be passed (to allow further extensibility in the future, for example should a current dir option also be added).
The MSVC part is missing, as I do not have access to a windows system right now.

sheredom added a commit that referenced this issue Feb 2, 2021
This commit adds `subprocess_create_ex` which has an additional argument
`environment` that lets you specify the environment for the spawned
process.

Fixes #1.
sheredom added a commit that referenced this issue Feb 2, 2021
This commit adds `subprocess_create_ex` which has an additional argument
`environment` that lets you specify the environment for the spawned
process.

Fixes #1.
sheredom added a commit that referenced this issue Feb 2, 2021
This commit adds `subprocess_create_ex` which has an additional argument
`environment` that lets you specify the environment for the spawned
process.

Fixes #1.
sheredom added a commit that referenced this issue Feb 2, 2021
This commit adds `subprocess_create_ex` which has an additional argument
`environment` that lets you specify the environment for the spawned
process.

Fixes #1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants