Enhance environment variables passing

[franekmagiera]

Closes #253

Introduced a new parameter include_invocation_env_vars to the session.run related methods. It can be a boolean or an iterable of vars that should be included from the nox invocation environment. Some env vars (inspired by tox are always passed in to ensure proper functionality of stdlib functions and pip.

[crwilcox]

I think this has the potential to be a race as written (possible the environment var is there when checked on the first check, but not on assignment).

[crwilcox]

I think this approach is worth discussing. In a way it is a subtle improvement over the suggestion in the bug thread (where passing an env var name with None as value meant "use the system". I also think it may more clear as it keeps this to just keys.

That said, I am not a huge fan of type of this var and that, in a way, it ends up overriding truthiness of the dict.

I think having the on/off makes a lot of sense, as this allows users of nox to control their sessions fully. If most folks are going to use this to bring 1-2 vars, it doesn't seem too overwhelming to need to do it via env:

result = session.run(
            ...
            env={"SYSTEM_ROOT": os.environ["SYSTEM_ROOT"]},
            ...
        )

That said, if it is more common for a user need many of them, this does cut down a bit on boilerplate. Though I can also imagine using a dict comp to do this in my code anyway:

env = {k: os.environ.get(k) for k in ["SYSTEM_ROOT", "LANGUAGE"]}

[FollowTheProcess]

I think the switch by itself is a good idea, potentially a rename to something a bit easier e.g. include_outer? But I agree on the type I think it should be just a bool on/off (with a default of True so the current default behaviour does not change).

I think 90%+ people who have a need to define env vars will have only a few. And if not the dictionary comp above works well, could even use a .env file or similar

[FollowTheProcess]

I'm also not a huge fan of maintaining a list of "special" env vars to always pass through, although I'm not entirely sure how else we'd do it

[franekmagiera]

Yeah, I agree that on/off is better, writing out the type annotation for that parameter also rubbed me the wrong way a bit, but wanted it to behave the way it was described in the issue. I guess it makes sense to just stick to on/off and keep the old behavior when we only care about passing SYSTEM_ROOT on windows, when it comes to other env variables it is up to the user to include them when include_outer=False/include_invovation_env_vars=False. WDYT?

[franekmagiera]

Thanks for doing the review! I agree @crwilcox's proposal. I think it addresses the original issue sufficiently, is clearer and easier to understand. This should be ready for another look.

[theacodes]

This looks great to me, merging. :)