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

fix: outer_env issues fixed #874

Merged
merged 2 commits into from
Jan 16, 2025
Merged

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Oct 27, 2024

Fix #868. Two issues:

  • Setting include_outer_env=False does nothing at all if env= is not passed. The outer env is still included.
  • Required environment variables set by nox itself, like VIRTUAL_ENV (required for uv), and CONDA_PREFIX (required for conda-family) would not be included if you do set both.

Fixing this was a bit intrusive: now .env only is the local environment
settings, and removals are added to it using None. The type has changed to match .command, with the None possibility. And then the final environment is built in the _run, where it can correctly process things like adding the bin dir and filtering the None's.

@henryiii henryiii force-pushed the henryiii/fix/envissues branch from 03e9762 to 827c8cf Compare October 28, 2024 04:28
@henryiii henryiii force-pushed the henryiii/fix/envissues branch from 87f0848 to d52592c Compare October 30, 2024 03:49
@henryiii henryiii added the awaiting review Needs a review label Nov 14, 2024
@henryiii henryiii force-pushed the henryiii/fix/envissues branch 2 times, most recently from 9fe8f05 to 356ff7e Compare November 23, 2024 05:40
@henryiii henryiii closed this Nov 23, 2024
@henryiii henryiii reopened this Nov 23, 2024
@henryiii henryiii force-pushed the henryiii/fix/envissues branch 2 times, most recently from 7e2dfcd to 09217ba Compare December 13, 2024 02:44
Copy link
Collaborator

@DiddiLeija DiddiLeija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me, as it seems to fix the issue properly!
However for this one I'd like to hear what other maintainers think ;)

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii force-pushed the henryiii/fix/envissues branch from 09217ba to c747bc1 Compare January 15, 2025 03:30
@henryiii
Copy link
Collaborator Author

However for this one I'd like to hear what other maintainers think

Not sure about that, it's been open for nearly three months. ;)

@DiddiLeija
Copy link
Collaborator

Yup, that's true, I was just a little concerned about some spots, but I've re-checked and everything is ok!
To me, you're free to merge :)

@henryiii henryiii merged commit 642c8bb into wntrblm:main Jan 16, 2025
24 checks passed
@henryiii henryiii deleted the henryiii/fix/envissues branch January 16, 2025 15:04
@henryiii
Copy link
Collaborator Author

Thanks!

@henryiii henryiii removed the awaiting review Needs a review label Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

include_outer_env=False will include outer_env when env is not set
2 participants