Merge pull request #5693 from sinscary/format_control_refactor

Refactoring: Move FormatControl to separate class

src/pip/_internal/cache.py

@@ -8,7 +8,6 @@ import os
 
 from pip._vendor.packaging.utils import canonicalize_name
 
-from pip._internal import index
 from pip._internal.download import path_to_url
 from pip._internal.models.link import Link
 from pip._internal.utils.compat import expanduser
@@ -23,7 +22,7 @@ class Cache(object):
 
 
         :param cache_dir: The root of the cache.
-        :param format_control: A pip.index.FormatControl object to limit
+        :param format_control: An object of FormatControl class to limit
             binaries being read from the cache.
         :param allowed_formats: which formats of files the cache should store.
             ('binary' and 'source' are the only allowed values)
@@ -73,8 +72,8 @@ class Cache(object):
             return []
 
         canonical_name = canonicalize_name(package_name)
-        formats = index.fmt_ctl_formats(
-            self.format_control, canonical_name
+        formats = self.format_control.get_allowed_formats(
+            canonical_name
         )
         if not self.allowed_formats.intersection(formats):
             return []

src/pip/_internal/cli/cmdoptions.py

@@ -14,10 +14,8 @@ from functools import partial
 from optparse import SUPPRESS_HELP, Option, OptionGroup
 
 from pip._internal.exceptions import CommandError
-from pip._internal.index import (
-    FormatControl, fmt_ctl_handle_mutual_exclude, fmt_ctl_no_binary,
-)
 from pip._internal.locations import USER_CACHE_DIR, src_prefix
+from pip._internal.models.format_control import FormatControl
 from pip._internal.models.index import PyPI
 from pip._internal.utils.hashes import STRONG_HASHES
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
@@ -54,7 +52,7 @@ def check_install_build_global(options, check_options=None):
     names = ["build_options", "global_options", "install_options"]
     if any(map(getname, names)):
         control = options.format_control
-        fmt_ctl_no_binary(control)
+        control.disallow_binaries()
         warnings.warn(
             'Disabling all use of wheels due to the use of --build-options '
             '/ --global-options / --install-options.', stacklevel=2,
@@ -405,24 +403,25 @@ def _get_format_control(values, option):
 
 
 def _handle_no_binary(option, opt_str, value, parser):
-    existing = getattr(parser.values, option.dest)
-    fmt_ctl_handle_mutual_exclude(
+    existing = _get_format_control(parser.values, option)
+    FormatControl.handle_mutual_excludes(
         value, existing.no_binary, existing.only_binary,
     )
 
 
 def _handle_only_binary(option, opt_str, value, parser):
-    existing = getattr(parser.values, option.dest)
-    fmt_ctl_handle_mutual_exclude(
+    existing = _get_format_control(parser.values, option)
+    FormatControl.handle_mutual_excludes(
         value, existing.only_binary, existing.no_binary,
     )
 
 
 def no_binary():
+    format_control = FormatControl(set(), set())
     return Option(
         "--no-binary", dest="format_control", action="callback",
         callback=_handle_no_binary, type="str",
-        default=FormatControl(set(), set()),
+        default=format_control,
         help="Do not use binary packages. Can be supplied multiple times, and "
              "each time adds to the existing value. Accepts either :all: to "
              "disable all binary packages, :none: to empty the set, or one or "
@@ -433,10 +432,11 @@ def no_binary():
 
 
 def only_binary():
+    format_control = FormatControl(set(), set())
     return Option(
         "--only-binary", dest="format_control", action="callback",
         callback=_handle_only_binary, type="str",
-        default=FormatControl(set(), set()),
+        default=format_control,
         help="Do not use source packages. Can be supplied multiple times, and "
              "each time adds to the existing value. Accepts either :all: to "
              "disable all source packages, :none: to empty the set, or one or "

src/pip/_internal/commands/freeze.py

@@ -2,9 +2,9 @@ from __future__ import absolute_import
 
 import sys
 
-from pip._internal import index
 from pip._internal.cache import WheelCache
 from pip._internal.cli.base_command import Command
+from pip._internal.models.format_control import FormatControl
 from pip._internal.operations.freeze import freeze
 from pip._internal.utils.compat import stdlib_pkgs
 
@@ -71,7 +71,7 @@ class FreezeCommand(Command):
         self.parser.insert_option_group(0, self.cmd_opts)
 
     def run(self, options, args):
-        format_control = index.FormatControl(set(), set())
+        format_control = FormatControl(set(), set())
         wheel_cache = WheelCache(options.cache_dir, format_control)
         skip = set(stdlib_pkgs)
         if not options.freeze_all:

src/pip/_internal/index.py

@@ -26,6 +26,7 @@ from pip._internal.exceptions import (
     UnsupportedWheel,
 )
 from pip._internal.models.candidate import InstallationCandidate
+from pip._internal.models.format_control import FormatControl
 from pip._internal.models.index import PyPI
 from pip._internal.models.link import Link
 from pip._internal.pep425tags import get_supported
@@ -39,7 +40,7 @@ from pip._internal.utils.misc import (
 from pip._internal.utils.packaging import check_requires_python
 from pip._internal.wheel import Wheel, wheel_ext
 
-__all__ = ['FormatControl', 'fmt_ctl_handle_mutual_exclude', 'PackageFinder']
+__all__ = ['FormatControl', 'PackageFinder']
 
 
 SECURE_ORIGINS = [
@@ -401,7 +402,7 @@ class PackageFinder(object):
             logger.debug('* %s', location)
 
         canonical_name = canonicalize_name(project_name)
-        formats = fmt_ctl_formats(self.format_control, canonical_name)
+        formats = self.format_control.get_allowed_formats(canonical_name)
         search = Search(project_name, canonical_name, formats)
         find_links_versions = self._package_versions(
             # We trust every directly linked archive in find_links
@@ -870,54 +871,6 @@ class HTMLPage(object):
             lambda match: '%%%2x' % ord(match.group(0)), url)
 
 
-FormatControl = namedtuple('FormatControl', 'no_binary only_binary')
-"""This object has two fields, no_binary and only_binary.
-
-If a field is falsy, it isn't set. If it is {':all:'}, it should match all
-packages except those listed in the other field. Only one field can be set
-to {':all:'} at a time. The rest of the time exact package name matches
-are listed, with any given package only showing up in one field at a time.
-"""
-
-
-def fmt_ctl_handle_mutual_exclude(value, target, other):
-    new = value.split(',')
-    while ':all:' in new:
-        other.clear()
-        target.clear()
-        target.add(':all:')
-        del new[:new.index(':all:') + 1]
-        if ':none:' not in new:
-            # Without a none, we want to discard everything as :all: covers it
-            return
-    for name in new:
-        if name == ':none:':
-            target.clear()
-            continue
-        name = canonicalize_name(name)
-        other.discard(name)
-        target.add(name)
-
-
-def fmt_ctl_formats(fmt_ctl, canonical_name):
-    result = {"binary", "source"}
-    if canonical_name in fmt_ctl.only_binary:
-        result.discard('source')
-    elif canonical_name in fmt_ctl.no_binary:
-        result.discard('binary')
-    elif ':all:' in fmt_ctl.only_binary:
-        result.discard('source')
-    elif ':all:' in fmt_ctl.no_binary:
-        result.discard('binary')
-    return frozenset(result)
-
-
-def fmt_ctl_no_binary(fmt_ctl):
-    fmt_ctl_handle_mutual_exclude(
-        ':all:', fmt_ctl.no_binary, fmt_ctl.only_binary,
-    )
-
-
 Search = namedtuple('Search', 'supplied canonical formats')
 """Capture key aspects of a search.
 

src/pip/_internal/models/format_control.py

@@ -0,0 +1,62 @@
+from pip._vendor.packaging.utils import canonicalize_name
+
+
+class FormatControl(object):
+    """A helper class for controlling formats from which packages are installed.
+    If a field is falsy, it isn't set. If it is {':all:'}, it should match all
+    packages except those listed in the other field. Only one field can be set
+    to {':all:'} at a time. The rest of the time exact package name matches
+    are listed, with any given package only showing up in one field at a time.
+    """
+    def __init__(self, no_binary=None, only_binary=None):
+        self.no_binary = set() if no_binary is None else no_binary
+        self.only_binary = set() if only_binary is None else only_binary
+
+    def __eq__(self, other):
+        return self.__dict__ == other.__dict__
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __repr__(self):
+        return "{}({}, {})".format(
+            self.__class__.__name__,
+            self.no_binary,
+            self.only_binary
+        )
+
+    @staticmethod
+    def handle_mutual_excludes(value, target, other):
+        new = value.split(',')
+        while ':all:' in new:
+            other.clear()
+            target.clear()
+            target.add(':all:')
+            del new[:new.index(':all:') + 1]
+            # Without a none, we want to discard everything as :all: covers it
+            if ':none:' not in new:
+                return
+        for name in new:
+            if name == ':none:':
+                target.clear()
+                continue
+            name = canonicalize_name(name)
+            other.discard(name)
+            target.add(name)
+
+    def get_allowed_formats(self, canonical_name):
+        result = {"binary", "source"}
+        if canonical_name in self.only_binary:
+            result.discard('source')
+        elif canonical_name in self.no_binary:
+            result.discard('binary')
+        elif ':all:' in self.only_binary:
+            result.discard('source')
+        elif ':all:' in self.no_binary:
+            result.discard('binary')
+        return frozenset(result)
+
+    def disallow_binaries(self):
+        self.handle_mutual_excludes(
+            ':all:', self.no_binary, self.only_binary,
+        )

src/pip/_internal/wheel.py

@@ -718,6 +718,7 @@ class WheelBuilder(object):
         assert building_is_possible
 
         buildset = []
+        format_control = self.finder.format_control
         for req in requirements:
             if req.constraint:
                 continue
@@ -741,8 +742,7 @@ class WheelBuilder(object):
                     if index.egg_info_matches(base, None, link) is None:
                         # E.g. local directory. Build wheel just for this run.
                         ephem_cache = True
-                    if "binary" not in index.fmt_ctl_formats(
-                            self.finder.format_control,
+                    if "binary" not in format_control.get_allowed_formats(
                             canonicalize_name(req.name)):
                         logger.info(
                             "Skipping bdist_wheel for %s, due to binaries "

tests/unit/test_cmdoptions.py

@@ -1,53 +0,0 @@
-from pip._internal import index
-from pip._internal.cli import cmdoptions
-from pip._internal.cli.base_command import Command
-
-
-class SimpleCommand(Command):
-    name = 'fake'
-    summary = name
-
-    def __init__(self):
-        super(SimpleCommand, self).__init__()
-        self.cmd_opts.add_option(cmdoptions.no_binary())
-        self.cmd_opts.add_option(cmdoptions.only_binary())
-
-    def run(self, options, args):
-        self.options = options
-
-
-def test_no_binary_overrides():
-    cmd = SimpleCommand()
-    cmd.main(['fake', '--only-binary=:all:', '--no-binary=fred'])
-    expected = index.FormatControl({'fred'}, {':all:'})
-    assert cmd.options.format_control == expected
-
-
-def test_only_binary_overrides():
-    cmd = SimpleCommand()
-    cmd.main(['fake', '--no-binary=:all:', '--only-binary=fred'])
-    expected = index.FormatControl({':all:'}, {'fred'})
-    assert cmd.options.format_control == expected
-
-
-def test_none_resets():
-    cmd = SimpleCommand()
-    cmd.main(['fake', '--no-binary=:all:', '--no-binary=:none:'])
-    expected = index.FormatControl(set(), set())
-    assert cmd.options.format_control == expected
-
-
-def test_none_preserves_other_side():
-    cmd = SimpleCommand()
-    cmd.main(
-        ['fake', '--no-binary=:all:', '--only-binary=fred',
-         '--no-binary=:none:'])
-    expected = index.FormatControl(set(), {'fred'})
-    assert cmd.options.format_control == expected
-
-
-def test_comma_separated_values():
-    cmd = SimpleCommand()
-    cmd.main(['fake', '--no-binary=1,2,3'])
-    expected = index.FormatControl({'1', '2', '3'}, set())
-    assert cmd.options.format_control == expected

tests/unit/test_finder.py

@@ -11,9 +11,7 @@ from pip._internal.download import PipSession
 from pip._internal.exceptions import (
     BestVersionAlreadyInstalled, DistributionNotFound,
 )
-from pip._internal.index import (
-    FormatControl, InstallationCandidate, Link, PackageFinder, fmt_ctl_formats,
-)
+from pip._internal.index import InstallationCandidate, Link, PackageFinder
 from pip._internal.req.constructors import install_req_from_line
 
 
@@ -586,16 +584,3 @@ def test_find_all_candidates_find_links_and_index(data):
     versions = finder.find_all_candidates('simple')
     # first the find-links versions then the page versions
     assert [str(v.version) for v in versions] == ['3.0', '2.0', '1.0', '1.0']
-
-
-def test_fmt_ctl_matches():
-    fmt = FormatControl(set(), set())
-    assert fmt_ctl_formats(fmt, "fred") == frozenset(["source", "binary"])
-    fmt = FormatControl({"fred"}, set())
-    assert fmt_ctl_formats(fmt, "fred") == frozenset(["source"])
-    fmt = FormatControl({"fred"}, {":all:"})
-    assert fmt_ctl_formats(fmt, "fred") == frozenset(["source"])
-    fmt = FormatControl(set(), {"fred"})
-    assert fmt_ctl_formats(fmt, "fred") == frozenset(["binary"])
-    fmt = FormatControl({":all:"}, {"fred"})
-    assert fmt_ctl_formats(fmt, "fred") == frozenset(["binary"])

tests/unit/test_format_control.py

@@ -0,0 +1,69 @@
+import pytest
+
+from pip._internal.cli import cmdoptions
+from pip._internal.cli.base_command import Command
+from pip._internal.models.format_control import FormatControl
+
+
+class SimpleCommand(Command):
+    name = 'fake'
+    summary = name
+
+    def __init__(self):
+        super(SimpleCommand, self).__init__()
+        self.cmd_opts.add_option(cmdoptions.no_binary())
+        self.cmd_opts.add_option(cmdoptions.only_binary())
+
+    def run(self, options, args):
+        self.options = options
+
+
+def test_no_binary_overrides():
+    cmd = SimpleCommand()
+    cmd.main(['fake', '--only-binary=:all:', '--no-binary=fred'])
+    format_control = FormatControl({'fred'}, {':all:'})
+    assert cmd.options.format_control == format_control
+
+
+def test_only_binary_overrides():
+    cmd = SimpleCommand()
+    cmd.main(['fake', '--no-binary=:all:', '--only-binary=fred'])
+    format_control = FormatControl({':all:'}, {'fred'})
+    assert cmd.options.format_control == format_control
+
+
+def test_none_resets():
+    cmd = SimpleCommand()
+    cmd.main(['fake', '--no-binary=:all:', '--no-binary=:none:'])
+    format_control = FormatControl(set(), set())
+    assert cmd.options.format_control == format_control
+
+
+def test_none_preserves_other_side():
+    cmd = SimpleCommand()
+    cmd.main(
+        ['fake', '--no-binary=:all:', '--only-binary=fred',
+         '--no-binary=:none:'])
+    format_control = FormatControl(set(), {'fred'})
+    assert cmd.options.format_control == format_control
+
+
+def test_comma_separated_values():
+    cmd = SimpleCommand()
+    cmd.main(['fake', '--no-binary=1,2,3'])
+    format_control = FormatControl({'1', '2', '3'}, set())
+    assert cmd.options.format_control == format_control
+
+
+@pytest.mark.parametrize(
+    "no_binary,only_binary,argument,expected",
+    [
+        ({"fred"}, set(), "fred", frozenset(["source"])),
+        ({"fred"}, {":all:"}, "fred", frozenset(["source"])),
+        (set(), {"fred"}, "fred", frozenset(["binary"])),
+        ({":all:"}, {"fred"}, "fred", frozenset(["binary"]))
+    ]
+)
+def test_fmt_ctl_matches(no_binary, only_binary, argument, expected):
+    fmt = FormatControl(no_binary, only_binary)
+    assert fmt.get_allowed_formats(argument) == expected

tests/unit/test_req_file.py

@@ -12,6 +12,7 @@ from pip._internal.exceptions import (
     InstallationError, RequirementsFileParseError,
 )
 from pip._internal.index import PackageFinder
+from pip._internal.models.format_control import FormatControl
 from pip._internal.req.constructors import (
     install_req_from_editable, install_req_from_line,
 )
@@ -37,7 +38,7 @@ def options(session):
     return stub(
         isolated_mode=False, index_url='default_url',
         skip_requirements_regex=False,
-        format_control=pip._internal.index.FormatControl(set(), set()))
+        format_control=FormatControl(set(), set()))
 
 
 class TestPreprocess(object):
@@ -562,8 +563,7 @@ class TestParseRequirements(object):
         list(parse_requirements(
             data.reqfiles.join("supported_options2.txt"), finder,
             session=PipSession()))
-        expected = pip._internal.index.FormatControl(
-            {'fred'}, {'wilma'})
+        expected = FormatControl({'fred'}, {'wilma'})
         assert finder.format_control == expected
 
     def test_req_file_parse_comment_start_of_line(self, tmpdir, finder):