Skip to content

Commit

Permalink
feat: ensure hash consistency between lockfile and venv (#207)
Browse files Browse the repository at this point in the history
This change fixes an issue observable in this test run in which riot generated a requirements lockfile for a Venv instance other than the one it was running tests for. This behavior was happening due to riot logic that skipped Venv instances with pkgs == None while preparing the environment, but not while running tests. Thus the fix is to stop riot from ignoring pkgs-less Venvs in all cases.

Note these two lines of output from the example test run:

```
Compiling requirements file .riot/requirements/118238b.in
RIOT_VENV_HASH=32bd6c2
```

This illustrates the mismatch. RIOT_VENV_HASH is the hash of the environment in which the command will be run, and for which the lockfile should be generated. The prepare() function had been ignoring that environment and instead preparing an environment for one of its ancestors.
  • Loading branch information
emmettbutler authored May 2, 2023
1 parent 5a7e061 commit fe9c1c1
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 4 deletions.
6 changes: 6 additions & 0 deletions releasenotes/notes/emptypkgs-21c00f01a644170d.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
fixes:
- |
Fixes an issue in which riot generated a requirements lockfile for a Venv instance other than the one it was running tests for.
This behavior was happening due to logic that skipped Venv instances with pkgs == None while preparing the environment,
but not while running tests. The fix is to stop riot from ignoring pkgs-less Venvs in all cases.
5 changes: 1 addition & 4 deletions riot/riot.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ def prefix(self) -> t.Optional[str]:
This will return a python version + package specific path name.
If no packages are defined it will return ``None``.
"""
if self.py is None or not self.pkgs:
if self.py is None:
return None

venv_path = self.py.venv_path
Expand Down Expand Up @@ -416,8 +416,6 @@ def venv_path(self) -> t.Optional[str]:
@property
def ident(self) -> t.Optional[str]:
"""Return prefix identifier string based on packages."""
if not self.pkgs:
return None
return "_".join(
(
f"{rmchars('<=>.,:+@/', n)}"
Expand Down Expand Up @@ -587,7 +585,6 @@ def prepare(
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.
Expand Down
33 changes: 33 additions & 0 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,39 @@ def test_requirements_lockfile_created(
assert "#" in lines[0]


def test_consistent_hashes_between_prepare_and_run_when_child_has_no_pkgs(
tmp_path: pathlib.Path, tmp_run: _T_TmpRun
) -> None:
version = "{}.{}".format(sys.version_info[0], sys.version_info[1])
rf_path = tmp_path / "riotfile.py"
rf_path.write_text(
"""
from riot import Venv
venv = Venv(
pkgs={{"requests": ""}},
name="parent",
command="env",
venvs=[Venv(pys=["{}"], name="child")]
)
""".format(
version
),
)
_hash = tmp_run("riot list --hash-only child").stdout.strip("\n")
result = tmp_run("riot run -p {} {}".format(version, _hash))
assert result.returncode == 0, result.stderr

lockfile_path = str(
tmp_path
/ os.path.join(
".riot",
"requirements",
"{}.txt".format(_hash),
)
)
assert os.path.exists(lockfile_path)


def test_recompile_flag(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None:
rf_path = tmp_path / "riotfile.py"
rf_path.write_text(
Expand Down

0 comments on commit fe9c1c1

Please sign in to comment.