Skip to content

Commit

Permalink
Run subprocesses in isolated directory (#1237)
Browse files Browse the repository at this point in the history
* Run subprocesses in destination dir

* Amend overlooked subprocess calls and add test

* Add changelog

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Evan Lewis <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 2, 2024
1 parent 10cbc9c commit e9fd497
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog.d/1091.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix installation issues when files in the working directory interfere with venv creation process.
4 changes: 3 additions & 1 deletion src/pipx/shared_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ def site_packages(self) -> Path:
def create(self, verbose: bool = False) -> None:
if not self.is_valid:
with animate("creating shared libraries", not verbose):
create_process = run_subprocess([DEFAULT_PYTHON, "-m", "venv", "--clear", self.root])
create_process = run_subprocess(
[DEFAULT_PYTHON, "-m", "venv", "--clear", self.root], run_dir=str(self.root)
)
subprocess_post_check(create_process)

# ignore installed packages to ensure no unexpected patches from the OS vendor
Expand Down
7 changes: 7 additions & 0 deletions src/pipx/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def run_subprocess(
log_cmd_str: Optional[str] = None,
log_stdout: bool = True,
log_stderr: bool = True,
run_dir: Optional[str] = None,
) -> "subprocess.CompletedProcess[str]":
"""Run arbitrary command as subprocess, capturing stderr and stout"""
env = dict(os.environ)
Expand All @@ -166,6 +167,8 @@ def run_subprocess(
if log_cmd_str is None:
log_cmd_str = " ".join(str(c) for c in cmd)
logger.info(f"running {log_cmd_str}")
if run_dir:
os.makedirs(run_dir, exist_ok=True)
# windows cannot take Path objects, only strings
cmd_str_list = [str(c) for c in cmd]
# Make sure to call the binary using its real path. This matters especially on Windows when using the packaged app
Expand All @@ -176,6 +179,9 @@ def run_subprocess(
# symlink in argv[0] so that it can locate the venv.
if not os.path.islink(cmd_str_list[0]) and WINDOWS:
cmd_str_list[0] = os.path.realpath(cmd_str_list[0])

# TODO: Switch to using `-P` / PYTHONSAFEPATH instead of running in
# separate directory in Python 3.11
completed_process = subprocess.run(
cmd_str_list,
env=env,
Expand All @@ -184,6 +190,7 @@ def run_subprocess(
encoding="utf-8",
universal_newlines=True,
check=False,
cwd=run_dir,
)

if capture_stdout and log_stdout:
Expand Down
8 changes: 4 additions & 4 deletions src/pipx/venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def create_venv(self, venv_args: List[str], pip_args: List[str], override_shared
cmd = [self.python, "-m", "venv"]
if not override_shared:
cmd.append("--without-pip")
venv_process = run_subprocess(cmd + venv_args + [str(self.root)])
venv_process = run_subprocess(cmd + venv_args + [str(self.root)], run_dir=str(self.root))
subprocess_post_check(venv_process)

shared_libs.create(self.verbose)
Expand Down Expand Up @@ -248,7 +248,7 @@ def install_package(
]
# no logging because any errors will be specially logged by
# subprocess_post_check_handle_pip_error()
pip_process = run_subprocess(cmd, log_stdout=False, log_stderr=False)
pip_process = run_subprocess(cmd, log_stdout=False, log_stderr=False, run_dir=str(self.root))
subprocess_post_check_handle_pip_error(pip_process)
if pip_process.returncode:
raise PipxError(f"Error installing {full_package_description(package_name, package_or_url)}.")
Expand Down Expand Up @@ -292,7 +292,7 @@ def install_unmanaged_packages(self, requirements: List[str], pip_args: List[str
]
# no logging because any errors will be specially logged by
# subprocess_post_check_handle_pip_error()
pip_process = run_subprocess(cmd, log_stdout=False, log_stderr=False)
pip_process = run_subprocess(cmd, log_stdout=False, log_stderr=False, run_dir=str(self.root))
subprocess_post_check_handle_pip_error(pip_process)
if pip_process.returncode:
raise PipxError(f"Error installing {', '.join(requirements)}.")
Expand Down Expand Up @@ -454,7 +454,7 @@ def _run_pip(self, cmd: List[str]) -> "CompletedProcess[str]":
cmd = [str(self.python_path), "-m", "pip"] + cmd
if not self.verbose:
cmd.append("-q")
return run_subprocess(cmd)
return run_subprocess(cmd, run_dir=str(self.root))

def run_pip_get_exit_code(self, cmd: List[str]) -> ExitCode:
cmd = [str(self.python_path), "-m", "pip"] + cmd
Expand Down
8 changes: 8 additions & 0 deletions tests/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,11 @@ def test_passed_python_and_force_flag_warning(pipx_temp_env, capsys):
assert not run_pipx_cli(["install", "pycowsay", "--force"])
captured = capsys.readouterr()
assert "--python is ignored when --force is passed." not in captured.out


def test_install_run_in_separate_directory(caplog, capsys, pipx_temp_env, monkeypatch, tmp_path):
monkeypatch.chdir(tmp_path)
f = Path("argparse.py")
f.touch()

install_packages(capsys, pipx_temp_env, caplog, ["pycowsay"], ["pycowsay"])

0 comments on commit e9fd497

Please sign in to comment.