diff --git a/releasenotes/notes/fix-sitepkgs-3ef98bad9dc20f70.yaml b/releasenotes/notes/fix-sitepkgs-3ef98bad9dc20f70.yaml new file mode 100644 index 0000000..f8d9072 --- /dev/null +++ b/releasenotes/notes/fix-sitepkgs-3ef98bad9dc20f70.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed handling of environment variables, package installations and namespace + packages. diff --git a/riot/riot.py b/riot/riot.py index 36a5e8b..fcc931e 100644 --- a/riot/riot.py +++ b/riot/riot.py @@ -106,7 +106,7 @@ def bin_path(self) -> t.Optional[str]: return os.path.join(self.venv_path, "bin") @property - def site_packages_path(self) -> t.Optional[str]: + def site_packages_path(self) -> str: version = ".".join((str(_) for _ in self.version_info()[:2])) return os.path.join(self.venv_path, "lib", f"python{version}", "site-packages") @@ -210,13 +210,18 @@ def instances( parent_inst: t.Optional["VenvInstance"] = None, ) -> t.Generator["VenvInstance", None, None]: # Expand out the instances for the venv. - for env in expand_specs(self.env): + for env_spec in expand_specs(self.env): + # Bubble up env + env = parent_inst.env.copy() if parent_inst else {} + env.update(dict(env_spec)) + # Bubble up pys pys = ( self.pys or (parent.pys if parent else None) or [parent_inst.py if parent_inst else None] ) + for py in pys: for pkgs in expand_specs(self.pkgs): inst = VenvInstance( @@ -226,7 +231,7 @@ def instances( or (parent_inst.command if parent_inst else None), py=py, env=env, - pkgs=pkgs, + pkgs=dict(pkgs), parent=parent_inst, ) if not self.venvs: @@ -238,9 +243,9 @@ def instances( @dataclasses.dataclass class VenvInstance: - pkgs: t.Tuple[t.Tuple[str, str]] + pkgs: t.Dict[str, str] py: Interpreter - env: t.Tuple[t.Tuple[str, str]] + env: t.Dict[str, str] name: t.Optional[str] = None command: t.Optional[str] = None parent: t.Optional["VenvInstance"] = None @@ -258,7 +263,8 @@ def venv_path(self) -> t.Optional[str]: venv_path = self.py.venv_path if self.needs_venv: venv_path = "_".join( - [venv_path] + [f"{n}{rmchars('<=>.,', v)}" for n, v in self.pkgs] + [venv_path] + + [f"{n}{rmchars('<=>.,', v)}" for n, v in self.pkgs.items()] ) return venv_path @@ -275,23 +281,22 @@ def needs_venv(self) -> bool: @property def pkg_str(self) -> str: """Return pip friendly install string from defined packages.""" - chain: t.List[VenvInstance] = [] + return pip_deps(self.pkgs) + + @property + def full_pkg_str(self) -> str: + """Return pip friendly install string from defined packages.""" + chain: t.List[VenvInstance] = [self] current: t.Optional[VenvInstance] = self while current is not None: - chain.append(current) + chain.insert(0, current) current = current.parent pkgs: t.Dict[str, str] = {} for inst in chain: pkgs.update(dict(inst.pkgs)) - return " ".join( - [ - f"'{get_pep_dep(lib, version)}'" - for lib, version in pkgs.items() - if version is not None - ] - ) + return pip_deps(pkgs) @property def bin_path(self) -> t.Optional[str]: @@ -326,8 +331,13 @@ def site_packages_path(self) -> t.Optional[str]: return os.path.join(target, "lib", f"python{version}", "site-packages") @property - def pythonpath(self) -> str: - paths = [] + def site_packages_list(self) -> t.List[str]: + """Return a list of all the site-packages paths along the parenting relation. + + The list starts with the empty string and is followed by the site-packages path + of the current instance, then the parent site-packages paths follow. + """ + paths = ["", os.getcwd()] # mimick 'python -m' current: t.Optional[VenvInstance] = self while current is not None: @@ -340,10 +350,11 @@ def pythonpath(self) -> str: if self.py.site_packages_path is not None: paths.append(self.py.site_packages_path) - # Finally add the CWD so that we can, e.g., find and run tests without 'python -m' - paths.append(os.getcwd()) + return paths - return ":".join(paths) + @property + def pythonpath(self) -> str: + return ":".join(self.site_packages_list) def prepare( self, env: t.Dict[str, str], py: t.Optional[Interpreter] = None @@ -380,7 +391,11 @@ def prepare( ) else: if self.pkg_str: - logger.info("Installing venv dependencies %s.", self.pkg_str) + logger.info( + "Installing venv dependencies %s in prefix %s.", + self.pkg_str, + self.target, + ) try: Session.run_cmd_venv( venv_path, @@ -512,8 +527,6 @@ def run( else: raise - logger.info("Running with %s", inst.py) - venv_path = inst.venv_path assert venv_path is not None @@ -523,9 +536,11 @@ def run( ) continue + logger.info("Running with %s", inst.py) + # Result which will be updated with the test outcome. result = VenvInstanceResult( - instance=inst, venv_name=venv_path, pkgstr=inst.pkg_str + instance=inst, venv_name=venv_path, pkgstr=inst.full_pkg_str ) # Generate the environment for the instance. @@ -556,19 +571,54 @@ def run( assert command is not None if cmdargs is not None: command = command.format(cmdargs=(" ".join(cmdargs))).strip() - env_str = " ".join(f"{k}={v}" for k, v in env.items()) + env_str = "\n".join(f"{k}={v}" for k, v in env.items()) logger.info( - "Running command '%s' with environment '%s'.", command, env_str + "Running command '%s' in venv '%s' with environment:\n%s.", + command, + venv_path, + env_str, ) + # Copy over *nspkg.pth files so that we can handle namespace packages. + # This is strictly required by Python 2.7 and <3.3 (see PEP 420). + ns_files = [] try: - # Pipe the command output directly to `out` since we - # don't need to store it. + sitepkgs = inst.site_packages_path + assert sitepkgs is not None, inst + for path in (_ for _ in inst.site_packages_list if _ != sitepkgs): + if path: + try: + for ns in ( + _ + for _ in os.listdir(path) + if _.endswith("nspkg.pth") + ): + ns_dest = os.path.join(sitepkgs, ns) + if os.path.isfile(ns_dest): + # child overrides parent + continue + ns_src = os.path.join(path, ns) + with open(ns_src) as ns_in: + # https://github.com/pypa/setuptools/blob/b62705a84ab599a2feff059ececd33800f364555/setuptools/namespaces.py#L44 + content = ns_in.read().replace( + "sys._getframe(1).f_locals['sitedir']", + f"'{path}'", + ) + with open(ns_dest, "w") as ns_out: + ns_out.write(content) + + ns_files.append(ns_dest) + except FileNotFoundError: + pass output = self.run_cmd_venv(venv_path, command, stdout=out, env=env) result.output = output.stdout except CmdFailure as e: raise CmdFailure( f"Test failed with exit code {e.proc.returncode}", e.proc ) + finally: + # We need to clean up as some layers might be shared. + for ns in ns_files: + os.remove(ns) except CmdFailure as e: result.code = e.code click.echo(click.style(e.msg, fg="red")) @@ -628,9 +678,7 @@ def list_venvs(self, pattern, venv_pattern, pythons=None, out=sys.stdout): if not venv_pattern.search(inst.venv_path): continue - pkgs_str = " ".join( - f"'{get_pep_dep(name, version)}'" for name, version in inst.pkgs - ) + pkgs_str = inst.full_pkg_str env_str = env_to_str(inst.env) py_str = f"Python {inst.py}" click.echo(f"{inst.name} {env_str} {py_str} {pkgs_str}") @@ -731,15 +779,15 @@ def get_pep_dep(libname: str, version: str) -> str: return f"{libname}{version}" -def env_to_str(envs: t.Sequence[t.Tuple[str, str]]) -> str: +def env_to_str(envs: t.Dict[str, str]) -> str: """Return a human-friendly representation of environment variables. - >>> env_to_str([("FOO", "BAR")]) + >>> env_to_str({"FOO": "BAR"}) 'FOO=BAR' - >>> env_to_str([("K", "V"), ("K2", "V2")]) + >>> env_to_str({"K": "V", "K2": "V2"}) 'K=V K2=V2' """ - return " ".join(f"{k}={v}" for k, v in envs) + return " ".join(f"{k}={v}" for k, v in envs.items()) def run_cmd( @@ -800,3 +848,13 @@ def expand_specs(specs: t.Dict[_K, t.List[_V]]) -> t.Iterator[t.Tuple[t.Tuple[_K # Need to cast because the * star typeshed of itertools.product returns Any return t.cast(t.Iterator[t.Tuple[t.Tuple[_K, _V]]], itertools.product(*all_vals)) + + +def pip_deps(pkgs: t.Dict[str, str]) -> str: + return " ".join( + [ + f"'{get_pep_dep(lib, version)}'" + for lib, version in pkgs.items() + if version is not None + ] + ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 014e3b8..f1c2ff1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -67,6 +67,8 @@ def test_list_with_venv_pattern(cli: click.testing.CliRunner) -> None: "pytest543$", ], ) + if result.exception: + raise result.exception assert result.exit_code == 0, result.stdout assert result.stdout == "test Python Interpreter(_hint='3') 'pytest==5.4.3'\n" @@ -317,7 +319,7 @@ def test_run_suites_cmdargs( subprocess_run.return_value.returncode = 0 args = ["run", name] + cmdargs result = cli.invoke(riot.cli.main, args, catch_exceptions=False) - assert result.exit_code == 0 + assert result.exit_code == 0, result.stdout subprocess_run.assert_called() diff --git a/tests/test_integration.py b/tests/test_integration.py index 9817e22..ee4eb22 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -555,6 +555,8 @@ def test_interpreter_pythonpath(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> N py_dot_version = ".".join((str(_) for _ in sys.version_info[:2])) assert env["PYTHONPATH"] == ":".join( ( + "", + str(tmp_path), str( tmp_path / os.path.join( @@ -565,7 +567,6 @@ def test_interpreter_pythonpath(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> N "site-packages", ) ), - str(tmp_path), ) ) @@ -628,7 +629,7 @@ def test_venv_instance_pythonpath(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> ) paths = env["PYTHONPATH"].split(":") - assert paths == [venv_path, parent_venv_path, py_venv_path, str(tmp_path)] + assert paths == ["", str(tmp_path), venv_path, parent_venv_path, py_venv_path] def test_venv_instance_path(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None: @@ -647,8 +648,7 @@ def test_venv_instance_path(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None: ) result = tmp_run("riot run -s test") env = dict(_.split("=") for _ in result.stdout.splitlines() if "=" in _) - print(result.stderr) - assert result.returncode == 0 + assert result.returncode == 0, result.stderr venv_name = "venv_py{}_pytest".format( "".join((str(_) for _ in sys.version_info[:3])) @@ -677,3 +677,28 @@ def test_venv_instance_path(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None: paths = env["PATH"].split(":") assert paths.index(venv_path) < paths.index(parent_venv_path) + + +def test_env_bubble_up(tmp_path: pathlib.Path, tmp_run: _T_TmpRun) -> None: + rf_path = tmp_path / "riotfile.py" + rf_path.write_text( + """ +from riot import Venv +venv = Venv( + pys=[3], + pkgs={"pip": ""}, + name="test", + command="env", + env={"foo": "42", "baz": "stillthesame"}, + venvs=[Venv(pkgs={"pytest": ""}, env={"bar": "43", "foo": "40"})], +) +""", + ) + result = tmp_run("riot -v run -s test") + print(result.stdout) + env = dict(_.split("=", maxsplit=1) for _ in result.stdout.splitlines() if "=" in _) + assert result.returncode == 0, (result.stdout, result.stderr) + + assert env["foo"] == "40" + assert env["bar"] == "43" + assert env["baz"] == "stillthesame" diff --git a/tests/test_unit.py b/tests/test_unit.py index c49a765..5c5e167 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -56,9 +56,9 @@ def test_interpreter_venv_path(current_interpreter: Interpreter) -> None: def test_venv_instance_venv_path(current_interpreter: Interpreter) -> None: venv = VenvInstance( command="echo test", - env=(("env", "test"),), + env={"env": "test"}, name="test", - pkgs=(("pip", ""),), + pkgs={"pip": ""}, py=current_interpreter, )