Skip to content

Commit 2def357

Browse files
authored
update env remove logic (#6195)
Resolves: #6018 1. Added a check so that if `python` argument is a file (then it should be a python path) - extract it's venv name and raise `IncorrectEnvError` if it doesn't belong to this project **Before** ``` └❯ poetry env remove ~/.cache/pypoetry/virtualenvs/different-project-OKfJHH_5-py3.10/bin/python /bin/sh: 1: different-project-OKfJHH_5-py3.10: not found Deleted virtualenv: ~/.cache/pypoetry/virtualenvs/poetry-4pWfmigs-py3.10 ``` Removes current project's env, which is wrong. **After** ``` └❯ poetry env remove ~/.cache/pypoetry/virtualenvs/different-project-OKfJHH_5-py3.10/bin/python Env different-project-OKfJHH_5-py3.10 doesn't belong to this project. ``` 2. Added the exact same check as before ^, but for cases where env name is passed. **Before** ``` └❯ poetry env remove different-project-OKfJHH_5-py3.10 /bin/sh: 1: different-project-OKfJHH_5-py3.10: not found Command different-project-OKfJHH_5-py3.10 -c "import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))" errored with the following return code 127, and output: ``` Errors while trying to exec env name as an interpreter, error is not clear. **After** ``` └❯ poetry env remove different-project-OKfJHH_5-py3.10 Env different-project-OKfJHH_5-py3.10 doesn't belong to this project. ``` 3. Added a couple of tests for **new** and for **old** scenarios which weren't tested. 4. Added `venv_name` fixture for `tests/utils` directory to use in `test_env`. Also replaced some of `"simple-project"` hardcoded value to use `poetry.package.name` It's up to maintainers to choose what they want for this project - I'm happy either way if we at least fix the bug. I can remove/change any of the stuff I added on top of the fix. But yeah I just decided that if we fix the bug, we might also make some improvements/changes in this area of code. Any thoughts on this are welcome, thanks!
1 parent b61a4dd commit 2def357

File tree

4 files changed

+229
-29
lines changed

4 files changed

+229
-29
lines changed

Diff for: src/poetry/utils/env.py

+39-1
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ def _version_nodot(version):
175175
GET_PYTHON_VERSION_ONELINER = (
176176
"\"import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))\""
177177
)
178+
GET_ENV_PATH_ONELINER = '"import sys; print(sys.prefix)"'
178179

179180
GET_SYS_PATH = """\
180181
import json
@@ -461,6 +462,12 @@ class EnvError(Exception):
461462
pass
462463

463464

465+
class IncorrectEnvError(EnvError):
466+
def __init__(self, env_name: str) -> None:
467+
message = f"Env {env_name} doesn't belong to this project."
468+
super().__init__(message)
469+
470+
464471
class EnvCommandError(EnvError):
465472
def __init__(self, e: CalledProcessError, input: str | None = None) -> None:
466473
self.e = e
@@ -740,14 +747,39 @@ def list(self, name: str | None = None) -> list[VirtualEnv]:
740747
env_list.insert(0, VirtualEnv(venv))
741748
return env_list
742749

750+
@staticmethod
751+
def check_env_is_for_current_project(env: str, base_env_name: str) -> bool:
752+
"""
753+
Check if env name starts with projects name.
754+
755+
This is done to prevent action on other project's envs.
756+
"""
757+
return env.startswith(base_env_name)
758+
743759
def remove(self, python: str) -> Env:
744760
venv_path = self._poetry.config.virtualenvs_path
745761

746762
cwd = self._poetry.file.parent
747763
envs_file = TOMLFile(venv_path / self.ENVS_FILE)
748764
base_env_name = self.generate_env_name(self._poetry.package.name, str(cwd))
749765

750-
if python.startswith(base_env_name):
766+
python_path = Path(python)
767+
if python_path.is_file():
768+
# Validate env name if provided env is a full path to python
769+
try:
770+
env_dir = decode(
771+
subprocess.check_output(
772+
list_to_shell_command([python, "-c", GET_ENV_PATH_ONELINER]),
773+
shell=True,
774+
)
775+
).strip("\n")
776+
env_name = Path(env_dir).name
777+
if not self.check_env_is_for_current_project(env_name, base_env_name):
778+
raise IncorrectEnvError(env_name)
779+
except CalledProcessError as e:
780+
raise EnvCommandError(e)
781+
782+
if self.check_env_is_for_current_project(python, base_env_name):
751783
venvs = self.list()
752784
for venv in venvs:
753785
if venv.path.name == python:
@@ -778,6 +810,12 @@ def remove(self, python: str) -> Env:
778810
raise ValueError(
779811
f'<warning>Environment "{python}" does not exist.</warning>'
780812
)
813+
else:
814+
venv_path = self._poetry.config.virtualenvs_path
815+
# Get all the poetry envs, even for other projects
816+
env_names = [Path(p).name for p in sorted(venv_path.glob("*-*-py*"))]
817+
if python in env_names:
818+
raise IncorrectEnvError(python)
781819

782820
try:
783821
python_version = Version.parse(python)

Diff for: tests/console/commands/env/conftest.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818

1919
@pytest.fixture
2020
def venv_name(app: PoetryTestApplication) -> str:
21-
return EnvManager.generate_env_name("simple-project", str(app.poetry.file.parent))
21+
return EnvManager.generate_env_name(
22+
app.poetry.package.name,
23+
str(app.poetry.file.parent),
24+
)
2225

2326

2427
@pytest.fixture

Diff for: tests/utils/conftest.py

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from __future__ import annotations
2+
3+
from typing import TYPE_CHECKING
4+
5+
import pytest
6+
7+
8+
if TYPE_CHECKING:
9+
from poetry.poetry import Poetry
10+
from poetry.utils.env import EnvManager
11+
12+
13+
@pytest.fixture
14+
def venv_name(
15+
manager: EnvManager,
16+
poetry: Poetry,
17+
) -> str:
18+
return manager.generate_env_name(
19+
poetry.package.name,
20+
str(poetry.file.parent),
21+
)

0 commit comments

Comments
 (0)