From 18a0784a048870e2644405696c29508b5f261fa9 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 16 Jan 2024 11:20:33 -0500 Subject: [PATCH] fix: nicer tracebacks in run mode on scripts (#1191) Co-authored-by: Philipp A. Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- changelog.d/1191.fix.md | 1 + src/pipx/commands/run.py | 22 +++++++++++++++------- tests/test_run.py | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 changelog.d/1191.fix.md diff --git a/changelog.d/1191.fix.md b/changelog.d/1191.fix.md new file mode 100644 index 0000000000..7ea5acdc7d --- /dev/null +++ b/changelog.d/1191.fix.md @@ -0,0 +1 @@ +Report correct filename in tracebacks with `pipx run ` diff --git a/src/pipx/commands/run.py b/src/pipx/commands/run.py index eff9af0b07..1e036de6e6 100644 --- a/src/pipx/commands/run.py +++ b/src/pipx/commands/run.py @@ -8,7 +8,7 @@ import urllib.request from pathlib import Path from shutil import which -from typing import List, NoReturn, Optional +from typing import List, NoReturn, Optional, Union from packaging.requirements import InvalidRequirement, Requirement @@ -42,14 +42,14 @@ {app_lines}""" -def maybe_script_content(app: str, is_path: bool) -> Optional[str]: +def maybe_script_content(app: str, is_path: bool) -> Optional[Union[str, Path]]: # If the app is a script, return its content. # Return None if it should be treated as a package name. # Look for a local file first. app_path = Path(app) if app_path.is_file(): - return app_path.read_text(encoding="utf-8") + return app_path elif is_path: raise PipxError(f"The specified path {app} does not exist") @@ -71,7 +71,7 @@ def maybe_script_content(app: str, is_path: bool) -> Optional[str]: def run_script( - content: str, + content: Union[str, Path], app_args: List[str], python: str, pip_args: List[str], @@ -81,7 +81,7 @@ def run_script( ) -> NoReturn: requirements = _get_requirements_from_script(content) if requirements is None: - exec_app([python, "-c", content, *app_args]) + python_path = Path(python) else: # Note that the environment name is based on the identified # requirements, and *not* on the script name. This is deliberate, as @@ -99,7 +99,12 @@ def run_script( venv = Venv(venv_dir, python=python, verbose=verbose) venv.create_venv(venv_args, pip_args) venv.install_unmanaged_packages(requirements, pip_args) - exec_app([venv.python_path, "-c", content, *app_args]) + python_path = venv.python_path + + if isinstance(content, Path): + exec_app([python_path, content, *app_args]) + else: + exec_app([python_path, "-c", content, *app_args]) def run_package( @@ -323,11 +328,14 @@ def _http_get_request(url: str) -> str: INLINE_SCRIPT_METADATA = re.compile(r"(?m)^# /// (?P[a-zA-Z0-9-]+)$\s(?P(^#(| .*)$\s)+)^# ///$") -def _get_requirements_from_script(content: str) -> Optional[List[str]]: +def _get_requirements_from_script(content: Union[str, Path]) -> Optional[List[str]]: """ Supports inline script metadata. """ + if isinstance(content, Path): + content = content.read_text(encoding="utf-8") + name = "script" # Windows is currently getting un-normalized line endings, so normalize diff --git a/tests/test_run.py b/tests/test_run.py index 0de39206e5..7d93556641 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -243,6 +243,24 @@ def test_run_with_requirements_old(caplog, pipx_temp_env, tmp_path): run_pipx_cli_exit(["run", script.as_uri()]) +@mock.patch("os.execvpe", new=execvpe_mock) +def test_run_correct_traceback(capfd, pipx_temp_env, tmp_path): + script = tmp_path / "test.py" + script.write_text( + textwrap.dedent( + """ + raise RuntimeError("Should fail") + """ + ).strip() + ) + + with pytest.raises(SystemExit): + run_pipx_cli(["run", str(script)]) + + captured = capfd.readouterr() + assert "test.py" in captured.err + + @mock.patch("os.execvpe", new=execvpe_mock) def test_run_with_args(caplog, pipx_temp_env, tmp_path): script = tmp_path / "test.py"