Skip to content

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

Closed
ghost wants to merge 1 commit intomasterfrom
unknown repository
Closed

[WIP] fix poetry env remove does not work with in-project = true#2925
ghost wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 16, 2020

#2908

Fixing issue where a poetry env remove .venv fails in a project with virtualenvs.in-project = true in poetry.toml (so the virtualenv is stored in .venv).

Pull Request Check List

Resolves: #issue-number-here

  • Added tests for changed code.
  • Updated documentation for changed code.

Comment thread poetry/utils/env.py
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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?

@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 27, 2020

Original issue duplicate of #2124, which has earlier PR #2748, so closing.

@ghost ghost closed this Oct 27, 2020
@ghost ghost deleted the in-project-env-remove branch October 27, 2020 14:52
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 1, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant