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

Add --python-executable option for pip-sync #1333

Merged
merged 38 commits into from
Jun 13, 2021
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
2fb4874
Support custom python-executable
maratk-ms Feb 27, 2021
5a68984
add tests
maratk-ms Mar 9, 2021
1100dab
Merge remote-tracking branch 'piptools/master' into feature/python-ex…
maratk-ms Mar 9, 2021
1c321cd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 9, 2021
f78866f
Merge commit 'f49b3fa8aa1b5d8c6dda80f719eaae706feff6b0' into feature/…
maratk-ms Mar 16, 2021
a3fd7a5
fix test
maratk-ms Mar 16, 2021
fc4b11e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 16, 2021
7cde62d
fix tests for older python versions
maratk-ms Mar 16, 2021
86ad4cc
Merge branch 'feature/python-executable' of https://github.com/MaratF…
maratk-ms Mar 16, 2021
786671a
code coverage
maratk-ms Mar 16, 2021
c25dc6f
comsetic updates
maratk-ms Mar 16, 2021
9733491
use type=click.Path
maratk-ms Mar 17, 2021
13bb9a6
Merge branch 'master' into feature/python-executable
MaratFM Mar 17, 2021
b01feb5
apply suggestion
maratk-ms Mar 18, 2021
75a13c5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 18, 2021
a5a4bf0
Merge remote-tracking branch 'piptools/master' into feature/python-ex…
maratk-ms Apr 1, 2021
b3e9529
add pip version check and support python executable resolving
maratk-ms Apr 1, 2021
92247e4
add test for invalid pip ver
maratk-ms Apr 1, 2021
d8750ba
fix tests for *nix
maratk-ms Apr 1, 2021
7c7f15b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 1, 2021
9f370e9
Merge branch 'master' into feature/python-executable
MaratFM Apr 12, 2021
3fb2077
Merge remote-tracking branch 'pip-tools/master' into feature/python-e…
maratk-ms Apr 22, 2021
451cc99
Merge branch 'master' into feature/python-executable
MaratFM May 3, 2021
3d20383
Merge remote-tracking branch 'pip-tools/master' into feature/python-e…
maratk-ms May 11, 2021
17cc535
pr comments
maratk-ms May 11, 2021
1e96d26
Merge branch 'master' into feature/python-executable
webknjaz Jun 8, 2021
36667bc
Extract _validate_python_executable, use logging formatting, PEP 257 …
maratk-ms Jun 8, 2021
1d6e2a8
fix mypy warning
maratk-ms Jun 8, 2021
24e2149
fix tests in python 3.6
maratk-ms Jun 8, 2021
86f13b5
Merge branch 'master' into feature/python-executable
ssbarnea Jun 9, 2021
7319cc0
revert log formatting
maratk-ms Jun 9, 2021
5849b41
Merge branch 'feature/python-executable' of https://github.com/MaratF…
maratk-ms Jun 9, 2021
5e8c59d
review comments
maratk-ms Jun 10, 2021
cd26a3a
Update piptools/utils.py
MaratFM Jun 11, 2021
390a150
try no cover
maratk-ms Jun 11, 2021
13eb866
Update piptools/utils.py
MaratFM Jun 12, 2021
cdb6c71
Merge branch 'master' into feature/python-executable
atugushev Jun 12, 2021
a138d07
fix tests
maratk-ms Jun 12, 2021
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
44 changes: 42 additions & 2 deletions piptools/scripts/sync.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import itertools
import os
import shlex
import shutil
import sys
from typing import List, Optional, Tuple, cast

Expand All @@ -15,7 +16,12 @@
from ..exceptions import PipToolsError
from ..logging import log
from ..repositories import PyPIRepository
from ..utils import flat_map
from ..utils import (
flat_map,
get_pip_version_for_python_executable,
get_required_pip_specification,
get_sys_path_for_python_executable,
)

DEFAULT_REQUIREMENTS_FILE = "requirements.txt"

Expand Down Expand Up @@ -55,6 +61,10 @@
is_flag=True,
help="Ignore package index (only looking at --find-links URLs instead)",
)
@click.option(
"--python-executable",
atugushev marked this conversation as resolved.
Show resolved Hide resolved
atugushev marked this conversation as resolved.
Show resolved Hide resolved
help="Custom python executable path if targeting an environment other than current",
)
@click.option("-v", "--verbose", count=True, help="Show more output")
@click.option("-q", "--quiet", count=True, help="Give less output")
@click.option(
Expand All @@ -77,6 +87,7 @@ def cli(
extra_index_url: Tuple[str, ...],
trusted_host: Tuple[str, ...],
no_index: bool,
python_executable: Optional[str],
verbose: int,
quiet: int,
user_only: bool,
Expand Down Expand Up @@ -108,6 +119,26 @@ def cli(
log.error("ERROR: " + msg)
sys.exit(2)

if python_executable:
resolved_python_executable = shutil.which(python_executable)
if resolved_python_executable is None:
msg = "Could not resolve '{}' as valid executable path or alias"
log.error(msg.format(python_executable))
sys.exit(2)

# Ensure that target python executable has the right version of pip installed
pip_version = get_pip_version_for_python_executable(python_executable)
required_pip_specification = get_required_pip_specification()
if not required_pip_specification.contains(pip_version):
msg = (
"Target python executable '{}' has pip version {} installed. "
"Version {} is expected."
)
log.error(
msg.format(python_executable, pip_version, required_pip_specification)
atugushev marked this conversation as resolved.
Show resolved Hide resolved
)
sys.exit(2)
atugushev marked this conversation as resolved.
Show resolved Hide resolved

install_command = cast(InstallCommand, create_command("install"))
options, _ = install_command.parse_args([])
session = install_command._build_session(options)
Expand All @@ -125,7 +156,15 @@ def cli(
log.error(str(e))
sys.exit(2)

installed_dists = get_installed_distributions(skip=[], user_only=user_only)
paths = (
None
if python_executable is None
else get_sys_path_for_python_executable(python_executable)
)

installed_dists = get_installed_distributions(
skip=[], user_only=user_only, paths=paths, local_only=python_executable is None
)
to_install, to_uninstall = sync.diff(merged_requirements, installed_dists)

install_flags = (
Expand All @@ -149,6 +188,7 @@ def cli(
dry_run=dry_run,
install_flags=install_flags,
ask=ask,
python_executable=python_executable,
)
)

Expand Down
18 changes: 18 additions & 0 deletions piptools/subprocess_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# WARNING! BE CAREFUL UPDATING THIS FILE
# Consider possible security implications associated with subprocess module.
import subprocess # nosec


def run_python_snippet(python_executable: str, code_to_run: str) -> str:
"""
Executes python code by calling python_executable with '-c' option.
"""
py_exec_cmd = python_executable, "-c", code_to_run

# subprocess module should never be used with untrusted input
return subprocess.check_output( # nosec
py_exec_cmd,
shell=False,
text=True,
universal_newlines=True,
)
7 changes: 5 additions & 2 deletions piptools/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,15 @@ def sync(
dry_run: bool = False,
install_flags: Optional[List[str]] = None,
ask: bool = False,
python_executable: Optional[str] = None,
) -> int:
"""
Install and uninstalls the given sets of modules.
"""
exit_code = 0

python_executable = python_executable or sys.executable

if not to_uninstall and not to_install:
log.info("Everything up-to-date", err=False)
return exit_code
Expand Down Expand Up @@ -216,7 +219,7 @@ def sync(
if to_uninstall:
run( # nosec
[
sys.executable,
python_executable,
"-m",
"pip",
"uninstall",
Expand Down Expand Up @@ -244,7 +247,7 @@ def sync(
try:
run( # nosec
[
sys.executable,
python_executable,
"-m",
"pip",
"install",
Expand Down
38 changes: 38 additions & 0 deletions piptools/utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import collections
import itertools
import json
import os
import shlex
from typing import (
Callable,
Expand All @@ -23,6 +25,9 @@
from pip._vendor.packaging.markers import Marker
from pip._vendor.packaging.specifiers import SpecifierSet
from pip._vendor.packaging.version import Version
from pip._vendor.pkg_resources import get_distribution

from piptools.subprocess_utils import run_python_snippet

_KT = TypeVar("_KT")
_VT = TypeVar("_VT")
Expand Down Expand Up @@ -358,3 +363,36 @@ def get_compile_command(click_ctx: click.Context) -> str:
left_args.append(f"{option_long_name}={shlex.quote(str(val))}")

return " ".join(["pip-compile", *sorted(left_args), *sorted(right_args)])


def get_required_pip_specification() -> SpecifierSet:
project_dist = get_distribution("pip-tools")
requirement = next((r for r in project_dist.requires() if r.name == "pip"), None)
atugushev marked this conversation as resolved.
Show resolved Hide resolved
assert (
requirement is not None
), "'pip' is expected to be in the list of pip-tools requirements"
return requirement.specifier


def get_pip_version_for_python_executable(python_executable: str) -> Version:
"""
Returns pip version for the given python executable.
"""
str_version = run_python_snippet(
python_executable, "import pip;print(pip.__version__)"
)
return Version(str_version)


def get_sys_path_for_python_executable(python_executable: str) -> List[str]:
"""
Returns sys.path list for the given python executable.
"""
result = run_python_snippet(
python_executable, "import sys;import json;print(json.dumps(sys.path))"
)

paths = json.loads(result)
assert isinstance(paths, list)
assert all(isinstance(i, str) for i in paths)
return [os.path.abspath(path) for path in paths]
115 changes: 115 additions & 0 deletions tests/test_cli_sync.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import os
import subprocess
import sys
from unittest import mock

import pytest
from pip._vendor.packaging.version import Version

from piptools.scripts.sync import DEFAULT_REQUIREMENTS_FILE, cli

Expand Down Expand Up @@ -242,3 +244,116 @@ def test_sync_dry_run_returns_non_zero_exit_code(runner):
out = runner.invoke(cli, ["--dry-run"])

assert out.exit_code == 1


@mock.patch("piptools.scripts.sync.get_pip_version_for_python_executable")
@mock.patch("piptools.scripts.sync.get_sys_path_for_python_executable")
@mock.patch("piptools.scripts.sync.get_installed_distributions")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are too much of monkeypatches. Ideally, there should be only @mock.patch("piptools.sync.run").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored

@mock.patch("piptools.sync.run")
atugushev marked this conversation as resolved.
Show resolved Hide resolved
def test_python_executable_option(
run,
get_installed_distributions,
get_sys_path_for_python_executable,
get_pip_version_for_python_executable,
runner,
fake_dist,
):
"""
Make sure sync command can run with `--python-executable` option
atugushev marked this conversation as resolved.
Show resolved Hide resolved
"""
with open("requirements.txt", "w") as req_in:
req_in.write("small-fake-a==1.10.0")

custom_executable = os.path.abspath("custom_executable")
with open(custom_executable, "w") as exec_file:
exec_file.write("")

os.chmod(custom_executable, 0o700)

sys_paths = ["", "./"]
get_sys_path_for_python_executable.return_value = sys_paths
get_installed_distributions.return_value = [fake_dist("django==1.8")]
get_pip_version_for_python_executable.return_value = Version("20.3")

runner.invoke(cli, ["--python-executable", custom_executable])

get_installed_distributions.assert_called_once_with(
skip=[], user_only=False, paths=sys_paths, local_only=False
)

assert run.call_count == 2

call_args = [call[0][0] for call in run.call_args_list]
called_uninstall_options = [args for args in call_args if args[3] == "uninstall"]
called_install_options = [args[:-1] for args in call_args if args[3] == "install"]

assert called_uninstall_options == [
[custom_executable, "-m", "pip", "uninstall", "-y", "django"]
]
assert called_install_options == [[custom_executable, "-m", "pip", "install", "-r"]]


@pytest.mark.parametrize(
"python_executable",
(
"/tmp/invalid_executable",
"invalid_python",
),
)
def test_invalid_python_executable(runner, python_executable):
with open("requirements.txt", "w") as req_in:
req_in.write("small-fake-a==1.10.0")

out = runner.invoke(cli, ["--python-executable", python_executable])
assert out.exit_code == 2, out
message = "Could not resolve '{}' as valid executable path or alias\n"
assert out.stderr == message.format(python_executable)


@mock.patch("piptools.scripts.sync.get_pip_version_for_python_executable")
def test_invalid_pip_version_in_python_executable(
get_pip_version_for_python_executable, runner
):
with open("requirements.txt", "w") as req_in:
req_in.write("small-fake-a==1.10.0")

custom_executable = os.path.abspath("custom_executable")
with open(custom_executable, "w") as exec_file:
exec_file.write("")

os.chmod(custom_executable, 0o700)

get_pip_version_for_python_executable.return_value = Version("19.1")

out = runner.invoke(cli, ["--python-executable", custom_executable])
assert out.exit_code == 2, out
message = (
"Target python executable '{}' has pip version 19.1 installed. "
"Version" # ">=20.3 is expected.\n" part is omitted
)
assert out.stderr.startswith(message.format(custom_executable))


@mock.patch("piptools.sync.run")
def test_default_python_executable_option(run, runner):
"""
Make sure sys.executable is used when --python-executable is not provided
"""
with open("requirements.txt", "w") as req_in:
req_in.write("small-fake-a==1.10.0")

runner.invoke(cli)

assert run.call_count == 2

call_args = [call[0][0] for call in run.call_args_list]
called_install_options = [args[:-1] for args in call_args if args[3] == "install"]
assert called_install_options == [
[
sys.executable,
"-m",
"pip",
"install",
"-r",
]
]
8 changes: 8 additions & 0 deletions tests/test_subprocess_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import sys

from piptools.subprocess_utils import run_python_snippet


def test_run_python_snippet_returns_multilne():
result = run_python_snippet(sys.executable, r'print("MULTILINE\nOUTPUT", end="")')
assert result == "MULTILINE\nOUTPUT"
17 changes: 17 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
import operator
import os
import shlex
import sys

import pip
import pytest
from pip._vendor.packaging.version import Version

from piptools.scripts.compile import cli as compile_cli
from piptools.utils import (
Expand All @@ -15,6 +18,8 @@
format_specifier,
get_compile_command,
get_hashes_from_ireq,
get_pip_version_for_python_executable,
get_sys_path_for_python_executable,
is_pinned_requirement,
is_url_requirement,
lookup_table,
Expand Down Expand Up @@ -423,3 +428,15 @@ def test_drop_extras(from_line, given, expected):
assert ireq.markers is None
else:
assert str(ireq.markers).replace("'", '"') == expected.replace("'", '"')


def test_get_pip_version_for_python_executable():
result = get_pip_version_for_python_executable(sys.executable)
assert Version(pip.__version__) == result


def test_get_sys_path_for_python_executable():
result = get_sys_path_for_python_executable(sys.executable)
# not testing for equality, because pytest adds extra paths into current sys.path
for path in result:
atugushev marked this conversation as resolved.
Show resolved Hide resolved
assert path in sys.path