Skip to content

Adds option for conda environments to ignore Python's user site packages #13635

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

travishathaway
Copy link
Contributor

@travishathaway travishathaway commented Feb 29, 2024

Fixes: #13337

Description

Please see the issue above for more background on why this is being done.

This pull request introduces a new include_python_user_packages configuration setting that can be set via a CLI option, environment variable or config file. When set to false, conda will create a pyvenv.cfg in the root of the conda environment with the following contents:

include-system-site-packages = false

The presence of this file will disable Python programs from looking in the user site packages location for additional libraries that can be imported.

This new configuration variable will default to false to reflect how conda currently behaves and to avoid breaking environments which have relied on this behavior in the past.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@travishathaway travishathaway requested a review from a team as a code owner February 29, 2024 11:31
Copy link

codspeed-hq bot commented Feb 29, 2024

CodSpeed Performance Report

Merging #13635 will not alter performance

⚠️ No base runs were found

Falling back to comparing travishathaway:add-no-python-user-site-packages (8ccdc9f) with main (11c21d5)

Summary

✅ 21 untouched benchmarks

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Feb 29, 2024
@travishathaway travishathaway marked this pull request as draft February 29, 2024 11:38
@@ -122,4 +123,7 @@ def execute(args: Namespace, parser: ArgumentParser) -> int:
dry_run=False,
)

return install(args, parser, "create")
install(args, parser, "create")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this return statement won't do anything bad. The install function does not return any return codes. Just None if successful.

Copy link
Contributor

Choose a reason for hiding this comment

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

The return type in the signature needs updating, or we do need to return 1 in case of errors, I think. Better double check because this is the type of oversight that ends up hitting CLI usage with unreported errors.

@isuruf
Copy link
Contributor

isuruf commented Mar 1, 2024

I think it's better to implement this as a conda package instead of as yet another configuration option. For eg: see https://github.com/conda-forge/conda-ecosystem-user-package-isolation-feedstock

@travishathaway
Copy link
Contributor Author

travishathaway commented Mar 4, 2024

@isuruf,

Thanks for linking to your project. I personally do not think packages should be used as configuration, but I know from past conversations that there is already a precedent set for this in the conda community.

I think this pull request makes this option much more discoverable (e.g. it's inclusion in the conda documentation) and easier to use (no additional packages to install).

Yes, we are adding another configuration option to conda, but I think this penalty is worth the benefits to ease of use.

If we were going to choose the package route, I would at the very least like this to be documented somewhere clearly in the conda documentation so that our users would have a chance to find it.

@itcarroll, do you have any thoughts on this?

@itcarroll
Copy link

I had the same thought originally, @isuruf, and proposed an improvement on the package you linked. Discussion there and further consideration convinced me this doesn't feel good as a package. Either 1) you would have some "isolate-me" package in each environment you want isolated, or 2) you would have an "isolate-on-create" package in the condo base environment. With 1, you introduce an oddball dependency having nothing to do with the needs of the project the environment was built for. With 2, you still need a command line option so you could disable isolation in particular cases.

Generally, I don't think it's a problem to have a plethora of well-documented configuration options for conda. I believe the very early resistance to solving #13337 was due to the necessary configuration being Python specific, not to it being a configuration option somewhere.

@jezdez
Copy link
Member

jezdez commented Apr 5, 2024

@jaimergp Could you take a look at this if this is somehow conflicting with your current work on conda-pip?

@jezdez jezdez requested review from jaimergp and jezdez April 5, 2024 17:23
@@ -0,0 +1,19 @@
### Enhancements

* Adds a new setting, `isolate-python-env`, that can be used to disable Python programs in a conda environment from using the user site packages directory. (#13635)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we were trying to stay away from adding language-specific options in conda itself now, and instead resorting to plugins. That's what prompted the inclusion of EXTERNALLY-MANAGED in conda-pip, at least, and I wonder if that project would be a better home for this type of contribution.

If that's not a good idea, then I'd lean towards having a setting that is a bit more abstract, like environment-isolation, that accepts a series of "features" to enable, one of them being pyvenv.cfg.

Copy link
Member

Choose a reason for hiding this comment

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

That's the conflict of concept that I fear as well. Venv-style environment management is part of the conda core experience and has an outsized impact on how pip (etc) install into it.

I outlined a possible change of policy in my comment here: #13337 (comment)

Could we tweak this solution here, that when conda-pip becomes ready to roll out to users (as a default dependency for the installers), it can become the package responsible for isolating conda environments from Python site-packages? Or would that overstep the role of plugins given that environments are a core aspect of conda?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the end goal to allow that kind of configuration once the behavior has changed, or will it eventually be frozen? Asking in case we want to start with a pseudo-private name like _include_system_* or similar.

Choose a reason for hiding this comment

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

[conda-pip] can become the package responsible for isolating conda environments from Python site-packages

Does that imply that an environment has to have pip (or conda-pip) in order to be isolated? Pip is not a requirement for isolation of venv environments, and consistency is nice.

Choose a reason for hiding this comment

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

[conda-pip] can become the package responsible for isolating conda environments from Python site-packages

Does that imply that an environment has to have pip (or conda-pip) in order to be isolated? Pip is not a requirement for isolation of venv environments, and consistency is nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jaimergp, I would be happy to re-write this as an environment_isolation setting that would accept a list of strings, pyvenv being one of them.

@itcarroll, would this suffice?

@jezdez, I don't think it makes sense to make it only part of the conda-pip package based on what @itcarroll has said.

Choose a reason for hiding this comment

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

Changing to environment_isolation seems okay, even though I can't think of another package (other than python) that could benefit. I could see that someone might want the equivalent of a $PATH that only looks in the environment, as another form of isolation. So perhaps environment_isolation would be a conda feature and not just a python feature.

Copy link

Hi there, thank you for your contribution!

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.

If you would like this pull request to remain open please:

  1. Rebase and verify the changes still work
  2. Leave a comment with the current status

NOTE: If this pull request was closed prematurely, please leave a comment.

Thanks!

@github-actions github-actions bot added the stale [bot] marked as stale due to inactivity label Apr 18, 2025
@itcarroll
Copy link

Checked conda==25.3.1 for any isolation related argument as well as current behaviour. This PR and #13337 are still relevant.

@github-actions github-actions bot added stale::recovered [bot] recovered after being marked as stale and removed stale [bot] marked as stale due to inactivity labels May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA stale::recovered [bot] recovered after being marked as stale
Projects
Status: 👀 In Review
Development

Successfully merging this pull request may close these issues.

isolating Python in conda environments from site-packages
6 participants