Unexpected (buggy?) environment passing

[sambarluc]

Describe the bug When I do session.run("stuff", env={}), I would expect to run the command in an environment where no variable is defined, however a copy of the local environment is made. Similarly, if I do session.run("stuff", env={"CREEPY": "TRUE"}) I would expect to run in an environment where only $CREEPY is defined, while this only updates the copy of the current environment.

How to reproduce Check the environment when running with env={}.

Expected behavior If I pass env={}, I would expect to work in a clean environment. At the moment, the workaround is to do for instance session.virtualenv.env = {"SYSTEMROOT": os.getenv("SYSTEMROOT", "")} at the beginning of each session, but I find that a bit too cumbersome.

[theacodes]

Hi @sambarluc, this is intentional. On Windows, SYSTEMROOT (and iirc a few other variables) have to be defined for executing other programs to work. Can you help me understand a use case where you'd need to have complete control over the environment variables passed down?

[sambarluc]

In our tests, we often use environment variables to, e.g., point to C extensions, define authentication variables,... I would like to have full control over the environment, and thus only pass the essential stuff (e.g., the SYSTEMROOT you mention) to the test environment and eventually to subprocess, to avoid flakiness or spurious results because, for instance, I happen to run from the wrong local environment, or some other test caused a state change. I think this is the current behaviour of tox.

BTW, thanks for the good work, I really appreciate it.

[theacodes]

I see, so it's not necessarily SYSTEMROOT that's causing you trouble, it's other stuff that we let through. Nox and tox take fundamentally different approaches here - tox requires you to explicitly pass env vars where Nox allows them all by default.

Does setting an env var to None work?

[sambarluc]

Does setting an env var to None work?

I think that's not possible, I think it's rejected at the subprocess level.

[theacodes]

Hmm. So what would the ideal behavior be for you? What could we add / do to fix this use case for you?

On Wed, Oct 9, 2019 at 12:34 AM Andrea Cimatoribus [email protected] wrote:

Does setting an env var to None work?

I that's not possible, I think it's rejected at the subprocess level.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/theacodes/nox/issues/253?email_source=notifications&email_token=AAB5I42QOO5CV26LLANQTXDQNWCP5A5CNFSM4I5L3B7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAW6DLI#issuecomment-539877805, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB5I4YWC2KLOJCV7RKRGELQNWCP5ANCNFSM4I5L3B7A .

[sambarluc]

Ideally, it should be up to the user to decide if and which variables to pass, so something like:

session(passenv=None)  # The default, current behaviour, passes everything
session(passenv={"PATH": None, "MY_VAR": 42})  # Passes from the local environment `PATH` alone, defines a new `MY_VAR`
session(passenv={})  # No variable defined in the running environment

I think this would already be sufficient to easily support all my workflows, but of course the API could be different, this is just the simplest thing that comes to my mind.

[theacodes]

I'd be for that, but I'm concerned it'll break existing users. :/

On Wed, Oct 16, 2019 at 12:08 AM Andrea Cimatoribus < [email protected]> wrote:

Ideally, it should be up to the user to decide if and which variables to pass, so something like:

session(passenv=None) # The default, current behaviour, passes everything session(passenv={"PATH": None, "MY_VAR": 42}) # Passes from the local environment PATH alone, defines a new MY_VAR session(passenv={}) # No variable defined in the running environment

I think this would already be sufficient to easily support all my workflows, but of course the API could be different, this is just the simplest thing that comes to my mind.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/theacodes/nox/issues/253?email_source=notifications&email_token=AAB5I4ZWRJ4MIQIIP4BWIPLQO24YVA5CNFSM4I5L3B7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBLMAUA#issuecomment-542556240, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5I44G3ZOYUJK76ZJI433QO24YVANCNFSM4I5L3B7A .

[sambarluc]

I'd be for that, but I'm concerned it'll break existing users. :/

I see your point, that's up to you to decide, I guess. If I have some spare time (of which I doubt, in this period), I'll try having a look at the code.

[fish2000]

If I may point out – the SYSTEMROOT environment-variable thing gave me some minor confusion when running some integration tests via Nox that interacted with one or another environment-access APIs:

… specifically, I am running these on Mac OS X 10.14 – the tests (both inline sanity-checks and those running under pytest) didn’t expect to have that variable defined but empty – I believe? I fixed everything on my end by passing an env argument to session.run(…) in the noxfile.py in one case, and amending a pytest fixture in one other.

I point it out because I am guessing it could cause deeper and/or more opaque problems for other users – it’d be great if the default passenv values took the platform into consideration in this case.

Indeed! HTH.

[theacodes]

I'd be happy to review a PR that implements the design in https://github.com/theacodes/nox/issues/253#issuecomment-542556240

[sambarluc]

Unlikely that I find the time any time soon, sorry.

[theacodes]

That's okay! Thanks for letting me know.

On Mon, Mar 8, 2021, 3:15 AM Andrea Cimatoribus [email protected] wrote:

Unlikely that I find the time any time soon, sorry.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/theacodes/nox/issues/253#issuecomment-792564904, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5I43KYGE2HKO5GXHXORTTCSBQRANCNFSM4I5L3B7A .

[franekmagiera]

Hey, spent some time thinking about this, will try to create a first pr version over the weekend.

EDIT: Created a first draft https://github.com/wntrblm/nox/pull/652

[crwilcox]

Thanks for the draft @franekmagiera . I took a pass and left a couple of comments. I'm interested to hear from other contributors, particularly on https://github.com/wntrblm/nox/pull/652#discussion_r988455097