Move out all the config code to a separate module

pip/baseparser.py

@@ -3,21 +3,12 @@
 
 import sys
 import optparse
-import os
-import re
 import textwrap
 from distutils.util import strtobool
 
 from pip._vendor.six import string_types
-from pip._vendor.six.moves import configparser
-from pip.locations import (
-    legacy_config_file, config_basename, running_under_virtualenv,
-    site_config_files
-)
-from pip.utils import appdirs, get_terminal_size
-
-
-_environ_prefix_re = re.compile(r"^PIP_", re.I)
+from pip.configuration import Configuration
+from pip.utils import get_terminal_size
 
 
 class PrettyHelpFormatter(optparse.IndentedHelpFormatter):
@@ -140,55 +131,12 @@ class ConfigOptionParser(CustomOptionParser):
     isolated = False
 
     def __init__(self, *args, **kwargs):
-        self.config = configparser.RawConfigParser()
         self.name = kwargs.pop('name')
         self.isolated = kwargs.pop("isolated", False)
-        self.files = self.get_config_files()
-        if self.files:
-            self.config.read(self.files)
+        self.config = Configuration()
         assert self.name
         optparse.OptionParser.__init__(self, *args, **kwargs)
 
-    def get_config_files(self):
-        # the files returned by this method will be parsed in order with the
-        # first files listed being overridden by later files in standard
-        # ConfigParser fashion
-        config_file = os.environ.get('PIP_CONFIG_FILE', False)
-        if config_file == os.devnull:
-            return []
-
-        # at the base we have any site-wide configuration
-        files = list(site_config_files)
-
-        # per-user configuration next
-        if not self.isolated:
-            if config_file and os.path.exists(config_file):
-                files.append(config_file)
-            else:
-                # This is the legacy config file, we consider it to be a lower
-                # priority than the new file location.
-                files.append(legacy_config_file)
-
-                # This is the new config file, we consider it to be a higher
-                # priority than the legacy file.
-                files.append(
-                    os.path.join(
-                        appdirs.user_config_dir("pip"),
-                        config_basename,
-                    )
-                )
-
-        # finally virtualenv configuration first trumping others
-        if running_under_virtualenv():
-            venv_config_file = os.path.join(
-                sys.prefix,
-                config_basename,
-            )
-            if os.path.exists(venv_config_file):
-                files.append(venv_config_file)
-
-        return files
-
     def check_default(self, option, key, val):
         try:
             return option.check_value(key, val)
@@ -200,26 +148,23 @@ def _update_defaults(self, defaults):
         """Updates the given defaults with values from the config files and
         the environ. Does a little special handling for certain types of
         options (lists)."""
-        # Then go and look for the other sources of configuration:
-        config = {}
-        # 1. config files
-        for section in ('global', self.name):
-            config.update(
-                self.normalize_keys(self.get_config_section(section))
-            )
+        self.config.load_config_files(self.name, self.isolated)
         # 2. environmental variables
         if not self.isolated:
-            config.update(self.normalize_keys(self.get_environ_vars()))
+            self.config.load_environment_vars()
+
         # Accumulate complex default state.
         self.values = optparse.Values(self.defaults)
         late_eval = set()
         # Then set the options with those values
-        for key, val in config.items():
+        for key, val in self.config.items():
             # ignore empty values
             if not val:
                 continue
 
-            option = self.get_option(key)
+            # '--' because configuration supports only long names
+            option = self.get_option('--' + key)
+
             # Ignore options not present in this parser. E.g. non-globals put
             # in [global] by users that want them to apply to all applicable
             # commands.
@@ -249,30 +194,6 @@ def _update_defaults(self, defaults):
         self.values = None
         return defaults
 
-    def normalize_keys(self, items):
-        """Return a config dictionary with normalized keys regardless of
-        whether the keys were specified in environment variables or in config
-        files"""
-        normalized = {}
-        for key, val in items:
-            key = key.replace('_', '-')
-            if not key.startswith('--'):
-                key = '--%s' % key  # only prefer long opts
-            normalized[key] = val
-        return normalized
-
-    def get_config_section(self, name):
-        """Get a section of a configuration"""
-        if self.config.has_section(name):
-            return self.config.items(name)
-        return []
-
-    def get_environ_vars(self):
-        """Returns a generator with all environmental vars with prefix PIP_"""
-        for key, val in os.environ.items():
-            if _environ_prefix_re.search(key):
-                yield (_environ_prefix_re.sub("", key).lower(), val)
-
     def get_default_values(self):
         """Overriding to make updating the defaults after instantiation of
         the option parser possible, _update_defaults() does the dirty work."""

pip/configuration.py

@@ -0,0 +1,118 @@
+"""Configuration management setup
+"""
+
+import re
+import os
+import sys
+
+from pip._vendor.six.moves import configparser
+from pip.locations import (
+    legacy_config_file, config_basename, running_under_virtualenv,
+    site_config_files
+)
+from pip.utils import appdirs
+
+
+_environ_prefix_re = re.compile(r"^PIP_", re.I)
+
+
+class Configuration(object):
+    """Handles the loading of configuration files and providing an interface to
+    accessing data within them.
+    """
+
+    def __init__(self):
+        self._configparser = configparser.RawConfigParser()
+        self._config = {}
+
+    def load_config_files(self, name, isolated):
+        """Loads configuration from configuration files
+        """
+        files = self._get_config_files(isolated)
+
+        if files:
+            self._configparser.read(files)
+
+        for section in ('global', name):
+            self._config.update(
+                self._normalize_keys(self._get_config_section(section))
+            )
+
+    def load_environment_vars(self):
+        """Loads configuration from environment variables
+        """
+        self._config.update(self._normalize_keys(self._get_environ_vars()))
+
+    def items(self):
+        """Returns key-value pairs like dict.values() representing the loaded
+        configuration
+        """
+        return self._config.items()
+
+    def _normalize_keys(self, items):
+        """Return a config dictionary with normalized keys regardless of
+        whether the keys were specified in environment variables or in config
+        files"""
+        normalized = {}
+        for key, val in items:
+            key = key.replace('_', '-')
+            if key.startswith('--'):
+                key = key[2:]  # only prefer long opts
+            normalized[key] = val
+        return normalized
+
+    def _get_environ_vars(self):
+        """Returns a generator with all environmental vars with prefix PIP_"""
+        for key, val in os.environ.items():
+            if _environ_prefix_re.search(key):
+                yield (_environ_prefix_re.sub("", key).lower(), val)
+
+    def _get_config_files(self, isolated):
+        """Returns configuration files in a defined order.
+
+        The order is that the first files are overridden by the latter files;
+        like what ConfigParser expects.
+        """
+        # the files returned by this method will be parsed in order with the
+        # first files listed being overridden by later files in standard
+        # ConfigParser fashion
+        config_file = os.environ.get('PIP_CONFIG_FILE', False)
+        if config_file == os.devnull:
+            return []
+
+        # at the base we have any site-wide configuration
+        files = list(site_config_files)
+
+        # per-user configuration next
+        if not isolated:
+            if config_file and os.path.exists(config_file):
+                files.append(config_file)
+            else:
+                # This is the legacy config file, we consider it to be a lower
+                # priority than the new file location.
+                files.append(legacy_config_file)
+
+                # This is the new config file, we consider it to be a higher
+                # priority than the legacy file.
+                files.append(
+                    os.path.join(
+                        appdirs.user_config_dir("pip"),
+                        config_basename,
+                    )
+                )
+
+        # finally virtualenv configuration first trumping others
+        if running_under_virtualenv():
+            venv_config_file = os.path.join(
+                sys.prefix,
+                config_basename,
+            )
+            if os.path.exists(venv_config_file):
+                files.append(venv_config_file)
+
+        return files
+
+    def _get_config_section(self, section):
+        if self._configparser.has_section(section):
+            return self._configparser.items(section)
+        return []

tests/unit/test_options.py

@@ -1,6 +1,6 @@
 import os
 import pytest
-import pip.baseparser
+import pip.configuration
 from pip import main
 from pip import cmdoptions
 from pip.basecommand import Command
@@ -107,8 +107,8 @@ def test_environment_override_config(self, monkeypatch):
         Test an environment variable overrides the config file
         """
         monkeypatch.setattr(
-            pip.baseparser.ConfigOptionParser,
-            "get_config_section",
+            pip.configuration.Configuration,
+            "_get_config_section",
             self.get_config_section,
         )
         os.environ['PIP_TIMEOUT'] = '-1'
@@ -120,8 +120,8 @@ def test_commmand_config_override_global_config(self, monkeypatch):
         Test that command config overrides global config
         """
         monkeypatch.setattr(
-            pip.baseparser.ConfigOptionParser,
-            "get_config_section",
+            pip.configuration.Configuration,
+            "_get_config_section",
             self.get_config_section,
         )
         options, args = main(['fake'])
@@ -132,8 +132,8 @@ def test_global_config_is_used(self, monkeypatch):
         Test that global config is used
         """
         monkeypatch.setattr(
-            pip.baseparser.ConfigOptionParser,
-            "get_config_section",
+            pip.configuration.Configuration,
+            "_get_config_section",
             self.get_config_section_global,
         )
         options, args = main(['fake'])
@@ -265,23 +265,16 @@ def test_client_cert(self):
 class TestOptionsConfigFiles(object):
 
     def test_venv_config_file_found(self, monkeypatch):
-        # We only want a dummy object to call the get_config_files method
-        monkeypatch.setattr(
-            pip.baseparser.ConfigOptionParser,
-            '__init__',
-            lambda self: None,
-        )
-
         # strict limit on the site_config_files list
-        monkeypatch.setattr(pip.baseparser, 'site_config_files', ['/a/place'])
+        monkeypatch.setattr(pip.configuration, 'site_config_files', ['/a/place'])
 
         # If we are running in a virtualenv and all files appear to exist,
         # we should see two config files.
         monkeypatch.setattr(
-            pip.baseparser,
+            pip.configuration,
             'running_under_virtualenv',
             lambda: True,
         )
         monkeypatch.setattr(os.path, 'exists', lambda filename: True)
-        cp = pip.baseparser.ConfigOptionParser()
-        assert len(cp.get_config_files()) == 4
+        cp = pip.configuration.Configuration()
+        assert len(cp._get_config_files(isolated=False)) == 4