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.

Sign up for GitHub

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

Jump to bottom

[WIP] fix poetry env remove does not work with in-project = true #2925

Closed
wants to merge 1 commit into from
Closed

[WIP] fix poetry env remove does not work with in-project = true #2925

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion poetry/utils/env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ def remove(self, python): # type: (str) -> Env
envs_file = TomlFile(venv_path / self.ENVS_FILE)
base_env_name = self.generate_env_name(self._poetry.package.name, str(cwd))

if python.startswith(base_env_name):
if python.startswith(base_env_name) or python == ".venv":
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't really do this if the environment was not created by us. There needs to be some flag somewhere (maybe even a marker file) in the env directory to determine if we created it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about checking self._poetry.config.get("virtualenvs.in-project") is true, similar to what's done when listing the envs?

Copy link
Member

Choose a reason for hiding this comment

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

The issue is that poetry supports existing environments. So, in cases where, lets say, someone needs some wierd system lib available etc., they might create .venv manually and also have virtualenvs.in-project set to true. In this scenario, there is no guarentee that we created it. Obviously other simpler scenarios exists too. Just picked an obtuse one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does a marker file / flag like you say exist somewhere already or will that need to be added when Poetry creates a .venv?

Copy link
Member

Choose a reason for hiding this comment

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

At the moment no. If we need to support this cleanly we should add one and add a good message if one is not found when user tries to delete this.

Virtualenv already creates a pyvenv.cfg, either relying on that or placing a poetry.toml file in the environment root maybe sufficient. Not sure what is the better approach here at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

The latter affords expansion later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So for a workaround (other than rm as noted in the initial issue) until #2924 (presumably?) changes the approach for a .venv would the following be sufficient?

  1. Check poetry.toml exists in the project root.
  2. Check poetry.toml contains in-project = true in the appropriate place.

Would a situation exist where a user has set locally in-project = true but has manually created .venv and doesn't want to remove it when running poetry env remove .venv?

venvs = self.list()
for venv in venvs:
if venv.path.name == python:
Expand Down