Skip to content

Commit

Permalink
chore: don't install requirements in intermediate venvs (#202)
Browse files Browse the repository at this point in the history
* do not install requirements from parent venvs because they have already been installed at the root

* continue trying to install as we move up the tree until at one venv has installed

* mark as installed when the venv already exists

* handle prefix None

* undo

* fix subtle boolean bug

* propagate knowledge of child venv installation all the way up the tree

* count preexisting as installed

* use full_pkg_str in venv path to make it accurately represent the packages installed in it

* update tests to match code behavior
  • Loading branch information
emmettbutler committed Mar 9, 2023
1 parent a4994aa commit d4d394f
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 14 deletions.
19 changes: 14 additions & 5 deletions riot/riot.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ def ident(self) -> t.Optional[str]:
return None
return "_".join(
(
f"{rmchars('<=>.,:+@/', n)}{rmchars('<=>.,:+@/', v)}"
for n, v in self.pkgs.items()
f"{rmchars('<=>.,:+@/', n)}"
for n in self.full_pkg_str.replace("'", "").split()
)
)

Expand Down Expand Up @@ -574,19 +574,24 @@ def prepare(
recreate: bool = False,
skip_deps: bool = False,
recompile_reqs: bool = False,
child_was_installed: bool = False,
) -> None:
# Propagate the interpreter down the parenting relation
self.py = py = py or self.py
if recompile_reqs:
recreate = True

# We only install dependencies if the prefix directory does not
# exist already. If it does exist, we assume it is in a good state.
exists = self.prefix is not None and os.path.isdir(self.prefix)

installed = False
if (
py is not None
and self.pkgs
and self.prefix is not None
# We only install dependencies if the prefix directory does not
# exist already. If it does exist, we assume it is in a good state.
and (not os.path.isdir(self.prefix) or recreate or recompile_reqs)
and not child_was_installed
):
venv_path = self.venv_path
assert venv_path is not None, py
Expand Down Expand Up @@ -623,9 +628,13 @@ def prepare(
f"Failed to install venv dependencies {pkg_str}\n{e.proc.stdout}",
e.proc,
)
else:
installed = True

if not self.created and self.parent is not None:
self.parent.prepare(env, py)
self.parent.prepare(
env, py, child_was_installed=installed or exists or child_was_installed
)


@dataclasses.dataclass
Expand Down
3 changes: 1 addition & 2 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,7 @@ def test_success():
assert os.environ["RIOT"] == "1"
assert os.environ["RIOT_PYTHON_HINT"] == "Interpreter(_hint='3')"
assert os.environ["RIOT_PYTHON_VERSION"].startswith("3.")
assert os.environ["RIOT_VENV_HASH"] == "f8691e0"
assert os.environ["RIOT_VENV_IDENT"] == "packaging213"
assert os.environ["RIOT_VENV_IDENT"] == "pytest_packaging213"
assert os.environ["RIOT_VENV_NAME"] == "envtest"
assert os.environ["RIOT_VENV_PKGS"] == "'packaging>=21.3'"
assert os.environ["RIOT_VENV_FULL_PKGS"] == "'pytest' 'packaging>=21.3'"
Expand Down
6 changes: 3 additions & 3 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ def test_venv_instance_pythonpath(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) ->

version = "".join((str(_) for _ in sys.version_info[:3]))

venv_name = f"venv_py{version}_pytest"
venv_name = f"venv_py{version}_pip_pytest"
parent_venv_name = f"venv_py{version}_pip"
py_dot_version = ".".join((str(_) for _ in sys.version_info[:2]))

Expand Down Expand Up @@ -708,7 +708,7 @@ def test_venv_instance_path(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None:
env = dict(_.split("=", maxsplit=1) for _ in result.stdout.splitlines() if "=" in _)
assert result.returncode == 0, result.stderr

venv_name = "venv_py{}_pytest".format(
venv_name = "venv_py{}_pip_pytest".format(
"".join((str(_) for _ in sys.version_info[:3]))
)
parent_venv_name = "venv_py{}_pip".format(
Expand Down Expand Up @@ -783,7 +783,7 @@ def test_create(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None:
""",
)
result = tmp_run("riot -Pv run -s child")
venv_path = tmp_path / ".riot/venv_py{}_pytest_pip".format(
venv_path = tmp_path / ".riot/venv_py{}_pip_pytest".format(
"".join((str(_) for _ in sys.version_info[:3]))
)
assert f"Creating virtualenv '{venv_path}'" in result.stderr, result.stderr
Expand Down
8 changes: 4 additions & 4 deletions tests/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def test_session_run(session_virtualenv: Session) -> None:
"""
session_virtualenv.run(re.compile(""), re.compile(""), False, False)

env_name = "venv_py%s%s%s_six1150_isort5101_itsdangerous110" % sys.version_info[:3]
env_name = "venv_py%s%s%s_itsdangerous110_isort5101_six1150" % sys.version_info[:3]
command_env = _get_env(env_name)
# Check exists and is empty of packages
result = _get_pip_freeze(command_env)
Expand All @@ -291,7 +291,7 @@ def test_session_run_check_environment_modifications(
"""
session_virtualenv.run(re.compile(""), re.compile(""), False, False)

env_name = "venv_py%s%s%s_six1150_isort5101_itsdangerous110" % sys.version_info[:3]
env_name = "venv_py%s%s%s_itsdangerous110_isort5101_six1150" % sys.version_info[:3]
command_env = _get_env(env_name)
_run_pip_install("itsdangerous==0.24", command_env)
# Check exists and is empty of packages
Expand All @@ -312,7 +312,7 @@ def test_session_run_check_environment_modifications_and_recreate_false(
"""
session_virtualenv.run(re.compile(""), re.compile(""), False, False)

env_name = "venv_py%s%s%s_six1150_isort5101_itsdangerous110" % sys.version_info[:3]
env_name = "venv_py%s%s%s_itsdangerous110_isort5101_six1150" % sys.version_info[:3]
command_env = _get_env(env_name)
_run_pip_install("itsdangerous==0.24", command_env)

Expand All @@ -335,7 +335,7 @@ def test_session_run_check_environment_modifications_and_recreate_true(
"""
session_virtualenv.run(re.compile(""), re.compile(""), False, False)

env_name = "venv_py%s%s%s_six1150_isort5101_itsdangerous110" % sys.version_info[:3]
env_name = "venv_py%s%s%s_itsdangerous110_isort5101_six1150" % sys.version_info[:3]
command_env = _get_env(env_name)
_run_pip_install("itsdangerous==0.24", command_env)

Expand Down

0 comments on commit d4d394f

Please sign in to comment.