Skip to content

Allow PATH modifications via prependpath and appendpath. #1560

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

Closed
wants to merge 2 commits into from

Conversation

aklajnert
Copy link

Closes #1423

Two changes here:

  • Warn if a user attempts to change PATH via setenv,
  • Allow extending PATH via prependenv and appendenv variables.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Rather than adding two new settings why don't we just handle PATH instead?

@aklajnert
Copy link
Author

Rather than adding two new settings why don't we just handle PATH instead?

You mean, should we allow to change PATH via the setenv? This is potentially breaking change. I imagine that some people might have it now in their tox.ini files, without realizing that it doesn't do anything. I guess that after an update, tox might not even start due to the wrong PATH setting.

@gaborbernat
Copy link
Member

I'd consider those people to have a bug in their config so I don't mind breaking those users 👍

@aklajnert
Copy link
Author

@gaborbernat Should I implement any safety mechanism to ensure that the user won't shoot himself in the foot? E.g. a warning if the original PATH is not part of the new one?

@gaborbernat
Copy link
Member

I would not warn here, IMHO you should not care about host PATH post interpreter discovery.

@aklajnert
Copy link
Author

OK, I'll create a fresh PR as I'll have to revert most of the changes anyway.

@aklajnert aklajnert closed this Apr 21, 2020
@aklajnert aklajnert deleted the allow_path_manipulations branch April 22, 2020 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PATH modification is not passed to the test commands
2 participants