Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run subprocesses in isolated directory #1237

Merged
merged 4 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"])