Includes PYTHON specific environment variables

[wingkinl]

fix #1629, environment variables that begin with PYTHON should also go to subprocess

[davidhalter]

@mrclary You did this in #1619. What do you think about this change?

[mrclary]

Well, I have a few concerns.

1. Grabbing environment variables from os.environ defeats the purpose of a default clean environment for subprocesses. Indeed, one of the motivations for passing clean environments is that PYTHON* system variables can cause unexpected behavior, especially when the host and client environments are different. For windows platforms, SYSTEMROOT must be present, so there is an exception for that. The purpose of the env_vars keyword argument of CompiledSubprocess is to provide the option to explicitly pass environment variables which may be desired for any particular application. This PR would also unwittingly override an explicitly passed PYTHON* variable with that which is in os.environ.
2. This PR would only affect Windows platforms, so there would be different behavior for *nix platforms. I don't know how much of an issue this would be.

My recommendation is capture your PYTHON* variables outside of CompiledSubprocess, rather than impose the behavior on all Windows users.

my_env_vars = {}
for k, v in os.environ.items():
    if k.upper().startswith('PYTHON'):
        my_env_vars.update({k: v})

CompiledSubprocess(executable, env_vars=my_env_vars)

All that being said, if Windows environment variables are not case sensitive, then I gladly appreciate the simplification in that regard.

[mrclary]

Well, I'm rethinking my comment. Perhaps the best thing to do is to adhere to the behavior of Popen as much as possible. Popen inherits os.environ by default, but accepts env keyword argument for explicit environments. Any user desiring a clean environment should explicitly pass env={} and Windows users should then shoulder the responsibility of including SYSTEMROOT (since Popen makes no such accommodation either)

Let me modify this PR with my updated recommendation

[mrclary]

@wingkinl @davidhalter , take a look at #1633. Will this do what you need?

[wingkinl]

@wingkinl @davidhalter , take a look at #1633. Will this do what you need?

It works great! Thanks.

[davidhalter]

@mrclary How about having the default of env=None instead and inherit the environment variables in that case and if somebody sets env={} it will be "clean"?

I don't want to provide full access to all popen arguments.

It could also be interesting to only give access to a few selected variables by default (SYSTEMROOT, PYTHONPATH and maybe one or two more).

[mrclary]

@mrclary How about having the default of env=None instead and inherit the environment variables in that case and if somebody sets env={} it will be "clean"?

I don't want to provide full access to all popen arguments.

No problem.

It could also be interesting to only give access to a few selected variables by default (SYSTEMROOT, PYTHONPATH and maybe one or two more).

Hmm. Users may expect default Popen behavior if env=None, i.e. inherit all os.environ variables.

[davidhalter]

@wingkinl I resolved this with the help of @mrclary in #1633. Now all environment variables are again available by default. If someone provides a dictionary the environment variables will be overwritten.

Thanks for looking into this and trying to find a solution!