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

sync: Use options from the txt file #464

Closed
wants to merge 2 commits into from
Closed
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
59 changes: 59 additions & 0 deletions piptools/scripts/_repo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import optparse

import pip

from ..repositories import PyPIRepository


class PipCommand(pip.basecommand.Command):
name = 'PipCommand'


def get_pip_options_and_pypi_repository(
index_url=None, extra_index_url=None, no_index=None,
find_links=None, cert=None, client_cert=None, pre=None,
trusted_host=None):
pip_command = get_pip_command()

pip_args = []
if find_links:
for link in find_links:
pip_args.extend(['-f', link])
if index_url:
pip_args.extend(['-i', index_url])
if no_index:
pip_args.extend(['--no-index'])
if extra_index_url:
for extra_index in extra_index_url:
pip_args.extend(['--extra-index-url', extra_index])
if cert:
pip_args.extend(['--cert', cert])
if client_cert:
pip_args.extend(['--client-cert', client_cert])
if pre:
pip_args.extend(['--pre'])
if trusted_host:
for host in trusted_host:
pip_args.extend(['--trusted-host', host])

pip_options, _ = pip_command.parse_args(pip_args)

session = pip_command._build_session(pip_options)
repository = PyPIRepository(pip_options, session)
return (pip_options, repository)


def get_pip_command():
# Use pip's parser for pip.conf management and defaults.
# General options (find_links, index_url, extra_index_url, trusted_host,
# and pre) are defered to pip.
pip_command = PipCommand()
index_opts = pip.cmdoptions.make_option_group(
pip.cmdoptions.index_group,
pip_command.parser,
)
pip_command.parser.insert_option_group(0, index_opts)
pip_command.parser.add_option(
optparse.Option('--pre', action='store_true', default=False))

return pip_command
48 changes: 6 additions & 42 deletions piptools/scripts/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from __future__ import (absolute_import, division, print_function,
unicode_literals)

import optparse
import os
import sys
import tempfile
Expand All @@ -13,11 +12,12 @@
from .. import click
from ..exceptions import PipToolsError
from ..logging import log
from ..repositories import LocalRequirementsRepository, PyPIRepository
from ..repositories import LocalRequirementsRepository
from ..resolver import Resolver
from ..utils import (assert_compatible_pip_version, dedup, is_pinned_requirement,
key_from_req, UNSAFE_PACKAGES)
from ..writer import OutputWriter
from ._repo import get_pip_options_and_pypi_repository

# Make sure we're using a compatible version of pip
assert_compatible_pip_version()
Expand Down Expand Up @@ -103,31 +103,10 @@ def cli(verbose, dry_run, pre, rebuild, find_links, index_url, extra_index_url,
# Setup
###

pip_command = get_pip_command()

pip_args = []
if find_links:
for link in find_links:
pip_args.extend(['-f', link])
if index_url:
pip_args.extend(['-i', index_url])
if extra_index_url:
for extra_index in extra_index_url:
pip_args.extend(['--extra-index-url', extra_index])
if cert:
pip_args.extend(['--cert', cert])
if client_cert:
pip_args.extend(['--client-cert', client_cert])
if pre:
pip_args.extend(['--pre'])
if trusted_host:
for host in trusted_host:
pip_args.extend(['--trusted-host', host])

pip_options, _ = pip_command.parse_args(pip_args)

session = pip_command._build_session(pip_options)
repository = PyPIRepository(pip_options, session)
(pip_options, repository) = get_pip_options_and_pypi_repository(
index_url=index_url, extra_index_url=extra_index_url,
find_links=find_links, cert=cert, client_cert=client_cert,
pre=pre, trusted_host=trusted_host)

# Proxy with a LocalRequirementsRepository if --upgrade is not specified
# (= default invocation)
Expand Down Expand Up @@ -244,18 +223,3 @@ def cli(verbose, dry_run, pre, rebuild, find_links, index_url, extra_index_url,

if dry_run:
log.warning('Dry-run, so nothing updated.')


def get_pip_command():
# Use pip's parser for pip.conf management and defaults.
# General options (find_links, index_url, extra_index_url, trusted_host,
# and pre) are defered to pip.
pip_command = PipCommand()
index_opts = pip.cmdoptions.make_option_group(
pip.cmdoptions.index_group,
pip_command.parser,
)
pip_command.parser.insert_option_group(0, index_opts)
pip_command.parser.add_option(optparse.Option('--pre', action='store_true', default=False))

return pip_command
26 changes: 17 additions & 9 deletions piptools/scripts/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from ..exceptions import PipToolsError
from ..logging import log
from ..utils import assert_compatible_pip_version, flat_map
from ._repo import get_pip_options_and_pypi_repository

# Make sure we're using a compatible version of pip
assert_compatible_pip_version()
Expand Down Expand Up @@ -47,8 +48,15 @@ def cli(dry_run, force, find_links, index_url, extra_index_url, no_index, quiet,
log.error('ERROR: ' + msg)
sys.exit(2)

requirements = flat_map(lambda src: pip.req.parse_requirements(src, session=True),
src_files)
(pip_options, repository) = get_pip_options_and_pypi_repository(
index_url=index_url, extra_index_url=extra_index_url,
no_index=no_index, find_links=find_links)

def parse_req_file(filename):
return pip.req.parse_requirements(
filename, session=True, finder=repository.finder)

requirements = flat_map(parse_req_file, src_files)

try:
requirements = sync.merge(requirements, ignore_conflicts=force)
Expand All @@ -60,15 +68,15 @@ def cli(dry_run, force, find_links, index_url, extra_index_url, no_index, quiet,
to_install, to_uninstall = sync.diff(requirements, installed_dists)

install_flags = []
for link in find_links or []:
for link in repository.finder.find_links or []:
install_flags.extend(['-f', link])
if no_index:
Copy link
Contributor

Choose a reason for hiding this comment

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

With this change the no_index flag is inhibited. Is this wanted?

Copy link
Contributor

Choose a reason for hiding this comment

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

The no_index flag is passed to the get_pip_options_and_pypi_repository call which then passes it along to the finder in the constructed repository. That'll make the repository.finder.index_urls to be empty if no_index is True. It'll also consider --no-index specified in the parsed txt file.

if not repository.finder.index_urls:
install_flags.append('--no-index')
if index_url:
install_flags.extend(['-i', index_url])
if extra_index_url:
for extra_index in extra_index_url:
install_flags.extend(['--extra-index-url', extra_index])
for (i, index_url) in enumerate(repository.finder.index_urls):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not quite the same intent. I see that you are reconstructing the parameter invocation based upon the index_urls, but that exposes too much of the implementation detail. I don't quite grasp why you are doing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

The point here is that the call to pip.req.parse_requirements above (line 56) will update the flags in the repository.finder (which is passed to it) according to the flags specified in the txt file. Therefore the repository.finder.index_urls will contain combination of command line options and txt file options and this is then used here to construct the options given to pip install.

Copy link
Contributor

@suutari suutari Jan 7, 2018

Choose a reason for hiding this comment

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

I don't understand what you mean by the "implementation detail", since there wasn't really any separation between implementation and interface here before my changes either. If implementation details should be hidden, then there should be a class like RequirementsFileParser which then could be used to parse the txt files and as a result would output the list of requirements and pip options from the parsed files. That would be nice improvement, but completely another issue.

Can you elaborate what do you mean by exposing the implementation detail here?

if i == 0:
install_flags.extend(['-i', index_url])
else:
install_flags.extend(['--extra-index-url', index_url])

sys.exit(sync.sync(to_install, to_uninstall, verbose=(not quiet), dry_run=dry_run,
install_flags=install_flags))
33 changes: 33 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,39 @@ def test_sync_quiet(tmpdir):
assert '-q' in call[0][0]


@pytest.mark.parametrize('option_name', [
'find-links', 'f', 'no-index', 'index-url', 'i', 'extra-index-url'])
def test_sync_uses_opts_from_txt_file(option_name):
"""sync command uses pip options from the txt file."""
(opt_in_txt_file, pip_opt) = {
'find-links': ('--find-links ./pkg-dir', '-f ./pkg-dir'),
'f': ('-f ./pkg-dir', '-f ./pkg-dir'),
'no-index': ('--no-index', '--no-index'),
'index-url': ('--index-url http://index-url', '-i http://index-url'),
'i': ('-i http://index.localhost', '-i http://index.localhost'),
'extra-index-url': (
'--extra-index-url http://extra-index.localhost',
'--extra-index-url http://extra-index.localhost'),
}[option_name]
runner = CliRunner()
with runner.isolated_filesystem():
with open('requirements.txt', 'w') as req_txt:
req_txt.write('{}\n'.format(opt_in_txt_file))
req_txt.write('foobar==0.42\n')

with mock.patch('piptools.sync.check_call') as check_call:
run_result = runner.invoke(sync_cli, ['-q'])
assert run_result.output == ''
assert run_result.exit_code == 0
number_of_install_calls = 0
for (call_args, _call_kwargs) in check_call.call_args_list:
cmd = ' '.join(call_args[0])
if cmd.startswith('pip install'):
assert pip_opt in cmd
number_of_install_calls += 1
assert number_of_install_calls == 1


def test_editable_package(tmpdir):
""" piptools can compile an editable """
fake_package_dir = os.path.join(os.path.split(__file__)[0], 'fixtures', 'small_fake_package')
Expand Down
2 changes: 1 addition & 1 deletion tests/test_repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from pip.req import InstallRequirement

from piptools.repositories.pypi import PyPIRepository
from piptools.scripts.compile import get_pip_command
from piptools.scripts._repo import get_pip_command


def test_pypirepo_build_dir_is_str():
Expand Down
2 changes: 1 addition & 1 deletion tests/test_repository_pypi.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from piptools.scripts.compile import get_pip_command
from piptools.scripts._repo import get_pip_command
from piptools.repositories.pypi import PyPIRepository


Expand Down
2 changes: 1 addition & 1 deletion tests/test_top_level_editable.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import pytest

from piptools.repositories import PyPIRepository
from piptools.scripts.compile import get_pip_command
from piptools.scripts._repo import get_pip_command


class MockedPyPIRepository(PyPIRepository):
Expand Down