package_prefs: move class-level cache to PackagePref instance

`PackagePrefs` has had a class-level cache of data from `packages.yaml` for
a long time, but it complicates testing and leads to subtle errors,
especially now that we frequently manipulate custom config scopes and
environments.

Moving the cache to instance-level doesn't slow down concretization or
the test suite, and it just caches for the life of a `PackagePrefs`
instance (i.e., for a single cocncretization) so we don't need to worry
about global state anymore.

- [x] Remove class-level caches from `PackagePrefs`
- [x] Add a cached _spec_order object on each `PackagePrefs` instance
- [x] Remove all calls to `PackagePrefs.clear_caches()`

lib/spack/llnl/util/lang.py

@@ -19,12 +19,6 @@
 ignore_modules = [r'^\.#', '~$']
 
 
-class classproperty(property):
-    """classproperty decorator: like property but for classmethods."""
-    def __get__(self, cls, owner):
-        return self.fget.__get__(None, owner)()
-
-
 def index_by(objects, *funcs):
     """Create a hierarchy of dictionaries by splitting the supplied
        set of objects on unique values of the supplied functions.

lib/spack/spack/package_prefs.py

@@ -8,8 +8,6 @@
 from six import string_types
 from six import iteritems
 
-from llnl.util.lang import classproperty
-
 import spack.repo
 import spack.error
 from spack.util.path import canonicalize_path
@@ -75,14 +73,13 @@ class PackagePrefs(object):
        provider_spec_list.sort(key=kf)
 
     """
-    _packages_config_cache = None
-    _spec_cache = {}
-
     def __init__(self, pkgname, component, vpkg=None):
         self.pkgname = pkgname
         self.component = component
         self.vpkg = vpkg
 
+        self._spec_order = None
+
     def __call__(self, spec):
         """Return a key object (an index) that can be used to sort spec.
 
@@ -90,8 +87,10 @@ def __call__(self, spec):
            this function as Python's sort functions already ensure that the
            key function is called at most once per sorted element.
         """
-        spec_order = self._specs_for_pkg(
+        if self._spec_order is None:
-            self.pkgname, self.component, self.vpkg)
+            self._spec_order = self._specs_for_pkg(
+                self.pkgname, self.component, self.vpkg)
+        spec_order = self._spec_order
 
         # integer is the index of the first spec in order that satisfies
         # spec, or it's a number larger than any position in the order.
@@ -107,13 +106,6 @@ def __call__(self, spec):
             match_index -= 0.5
         return match_index
 
-    @classproperty
-    @classmethod
-    def _packages_config(cls):
-        if cls._packages_config_cache is None:
-            cls._packages_config_cache = get_packages_config()
-        return cls._packages_config_cache
-
     @classmethod
     def order_for_package(cls, pkgname, component, vpkg=None, all=True):
         """Given a package name, sort component (e.g, version, compiler, ...),
@@ -124,7 +116,7 @@ def order_for_package(cls, pkgname, component, vpkg=None, all=True):
             pkglist.append('all')
 
         for pkg in pkglist:
-            pkg_entry = cls._packages_config.get(pkg)
+            pkg_entry = get_packages_config().get(pkg)
             if not pkg_entry:
                 continue
 
@@ -150,21 +142,9 @@ def _specs_for_pkg(cls, pkgname, component, vpkg=None):
            return a list of CompilerSpecs, VersionLists, or Specs for
            that sorting list.
         """
-        key = (pkgname, component, vpkg)
+        pkglist = cls.order_for_package(pkgname, component, vpkg)
-
+        spec_type = _spec_type(component)
-        specs = cls._spec_cache.get(key)
+        return [spec_type(s) for s in pkglist]
-        if specs is None:
-            pkglist = cls.order_for_package(pkgname, component, vpkg)
-            spec_type = _spec_type(component)
-            specs = [spec_type(s) for s in pkglist]
-            cls._spec_cache[key] = specs
-
-        return specs
-
-    @classmethod
-    def clear_caches(cls):
-        cls._packages_config_cache = None
-        cls._spec_cache = {}
 
     @classmethod
     def has_preferred_providers(cls, pkgname, vpkg):
@@ -180,7 +160,7 @@ def has_preferred_targets(cls, pkg_name):
     def preferred_variants(cls, pkg_name):
         """Return a VariantMap of preferred variants/values for a spec."""
         for pkg in (pkg_name, 'all'):
-            variants = cls._packages_config.get(pkg, {}).get('variants', '')
+            variants = get_packages_config().get(pkg, {}).get('variants', '')
             if variants:
                 break
 

lib/spack/spack/test/cmd/env.py

@@ -403,8 +403,6 @@ def test_env_with_config():
     mpileaks:
       version: [2.2]
 """
-    spack.package_prefs.PackagePrefs.clear_caches()
-
     _env_create('test', StringIO(test_config))
 
     e = ev.read('test')
@@ -423,8 +421,6 @@ def test_env_with_included_config_file():
   specs:
   - mpileaks
 """
-    spack.package_prefs.PackagePrefs.clear_caches()
-
     _env_create('test', StringIO(test_config))
     e = ev.read('test')
 
@@ -452,7 +448,6 @@ def test_env_with_included_config_scope():
   - mpileaks
 """ % config_scope_path
 
-    spack.package_prefs.PackagePrefs.clear_caches()
     _env_create('test', StringIO(test_config))
 
     e = ev.read('test')
@@ -483,9 +478,6 @@ def test_env_config_precedence():
   specs:
   - mpileaks
 """
-
-    spack.package_prefs.PackagePrefs.clear_caches()
-
     _env_create('test', StringIO(test_config))
     e = ev.read('test')
 
@@ -519,8 +511,6 @@ def test_included_config_precedence():
   specs:
   - mpileaks
 """
-    spack.package_prefs.PackagePrefs.clear_caches()
-
     _env_create('test', StringIO(test_config))
     e = ev.read('test')
 

lib/spack/spack/test/concretize_preferences.py

@@ -23,7 +23,6 @@ def concretize_scope(config, tmpdir):
     yield
 
     config.pop_scope()
-    spack.package_prefs.PackagePrefs.clear_caches()
     spack.repo.path._provider_index = None
 
 
@@ -60,7 +59,6 @@ def update_packages(pkgname, section, value):
     """Update config and reread package list"""
     conf = {pkgname: {section: value}}
     spack.config.set('packages', conf, scope='concretize')
-    spack.package_prefs.PackagePrefs.clear_caches()
 
 
 def assert_variant_values(spec, **variants):
@@ -204,7 +202,6 @@ def test_all_is_not_a_virtual(self):
         spack.config.set('packages', conf, scope='concretize')
 
         # should be no error for 'all':
-        spack.package_prefs.PackagePrefs.clear_caches()
         spack.package_prefs.get_packages_config()
 
     def test_external_mpi(self):

lib/spack/spack/test/conftest.py

@@ -349,8 +349,6 @@ def configuration_dir(tmpdir_factory, linux_os):
 def config(configuration_dir):
     """Hooks the mock configuration files into spack.config"""
     # Set up a mock config scope
-    spack.package_prefs.PackagePrefs.clear_caches()
-
     real_configuration = spack.config.config
 
     defaults = spack.config.InternalConfigScope(
@@ -367,14 +365,11 @@ def config(configuration_dir):
     yield spack.config.config
 
     spack.config.config = real_configuration
-    spack.package_prefs.PackagePrefs.clear_caches()
 
 
 @pytest.fixture(scope='function')
 def mutable_config(tmpdir_factory, configuration_dir, monkeypatch):
     """Like config, but tests can modify the configuration."""
-    spack.package_prefs.PackagePrefs.clear_caches()
-
     mutable_dir = tmpdir_factory.mktemp('mutable_config').join('tmp')
     configuration_dir.copy(mutable_dir)
 
@@ -389,8 +384,6 @@ def mutable_config(tmpdir_factory, configuration_dir, monkeypatch):
 
     yield spack.config.config
 
-    spack.package_prefs.PackagePrefs.clear_caches()
-
 
 @pytest.fixture()
 def mock_config(tmpdir):