From 1c7f754e5b8cd7740f3c4a91ee22a0354b40844a Mon Sep 17 00:00:00 2001 From: Matthew LeGendre Date: Fri, 11 Mar 2016 10:00:00 -0800 Subject: Invert and rename the `nobuild` option in package.yaml configs to `buildable`. --- lib/spack/docs/site_configuration.rst | 15 +++++++-------- lib/spack/spack/concretize.py | 8 ++++---- lib/spack/spack/config.py | 16 ++++++++-------- lib/spack/spack/test/mock_packages_test.py | 4 ++-- 4 files changed, 21 insertions(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/site_configuration.rst b/lib/spack/docs/site_configuration.rst index 7ae541d4a3..3abfa21a9d 100644 --- a/lib/spack/docs/site_configuration.rst +++ b/lib/spack/docs/site_configuration.rst @@ -78,8 +78,7 @@ This example lists three installations of OpenMPI, one built with gcc, one built with gcc and debug information, and another built with Intel. If Spack is asked to build a package that uses one of these MPIs as a dependency, it will use the the pre-installed OpenMPI in -the given directory. This example also specifies that Spack should never -build its own OpenMPI via the ``nobuild: True`` option. +the given directory. Each ``packages.yaml`` begins with a ``packages:`` token, followed by a list of package names. To specify externals, add a ``paths`` @@ -111,16 +110,16 @@ be: paths: openmpi@1.4.3%gcc@4.4.7=chaos_5_x86_64_ib: /opt/openmpi-1.4.3 openmpi@1.4.3%gcc@4.4.7=chaos_5_x86_64_ib+debug: /opt/openmpi-1.4.3-debug - openmpi@1.6.5%intel@10.1=chaos_5_x86_64_ib: /opt/openmpi-1.6.5-intel - nobuild: True + openmpi@1.6.5%intel@10.1=chaos_5_x86_64_ib: /opt/openmpi-1.6.5-intel + buildable: False -The addition of the ``nobuild`` flag tells Spack that it should never build +The addition of the ``buildable`` flag tells Spack that it should never build its own version of OpenMPI, and it will instead always rely on a pre-built -OpenMPI. Similar to ``paths``, ``nobuild`` is specified as a property under +OpenMPI. Similar to ``paths``, ``buildable`` is specified as a property under a package name. -The ``nobuild`` does not need to be paired with external packages. -It could also be used alone to forbid packages that may be +The ``buildable`` does not need to be paired with external packages. +It could also be used alone to forbid packages that may be buggy or otherwise undesirable. diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 2268084e56..8d29a03f93 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -69,12 +69,12 @@ class DefaultConcretizer(object): packages = [spec] # For each candidate package, if it has externals add those to the candidates - # if it's a nobuild, then only add the externals. + # if it's not buildable, then only add the externals. candidates = [] all_compilers = spack.compilers.all_compilers() for pkg in packages: externals = spec_externals(pkg) - buildable = not is_spec_nobuild(pkg) + buildable = is_spec_buildable(pkg) if buildable: candidates.append((pkg, None)) for ext in externals: @@ -369,8 +369,8 @@ class NoValidVersionError(spack.error.SpackError): class NoBuildError(spack.error.SpackError): - """Raised when a package is configured with the nobuild option, but + """Raised when a package is configured with the buildable option False, but no satisfactory external versions can be found""" def __init__(self, spec): super(NoBuildError, self).__init__( - "The spec '%s' is configured as nobuild, and no matching external installs were found" % spec.name) + "The spec '%s' is configured as not buildable, and no matching external installs were found" % spec.name) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 3a785fe692..a21dd6dbe1 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -220,9 +220,9 @@ section_schemas = { 'type' : 'array', 'default' : [], 'items' : { 'type' : 'string' } }, #compiler specs - 'nobuild': { + 'buildable': { 'type': 'boolean', - 'default': False, + 'default': True, }, 'providers': { 'type': 'object', @@ -557,15 +557,15 @@ def spec_externals(spec): return spec_locations -def is_spec_nobuild(spec): - """Return true if the spec pkgspec is configured as nobuild""" +def is_spec_buildable(spec): + """Return true if the spec pkgspec is configured as buildable""" allpkgs = get_config('packages') name = spec.name if not spec.name in allpkgs: - return False - if not 'nobuild' in allpkgs[spec.name]: - return False - return allpkgs[spec.name]['nobuild'] + return True + if not 'buildable' in allpkgs[spec.name]: + return True + return allpkgs[spec.name]['buildable'] class ConfigError(SpackError): pass diff --git a/lib/spack/spack/test/mock_packages_test.py b/lib/spack/spack/test/mock_packages_test.py index 079cbcc136..6d24a84150 100644 --- a/lib/spack/spack/test/mock_packages_test.py +++ b/lib/spack/spack/test/mock_packages_test.py @@ -52,11 +52,11 @@ compilers: mock_packages_config = """\ packages: externaltool: - nobuild: True + buildable: False paths: externaltool@1.0%gcc@4.5.0: /path/to/external_tool externalvirtual: - nobuild: True + buildable: False paths: externalvirtual@2.0%clang@3.3: /path/to/external_virtual_clang externalvirtual@1.0%gcc@4.5.0: /path/to/external_virtual_gcc -- cgit v1.2.3-70-g09d2 From bae03404f44ac60439e85e31a85bb6848db2a5e4 Mon Sep 17 00:00:00 2001 From: "Adam J. Stewart" Date: Fri, 11 Mar 2016 12:51:45 -0600 Subject: Documentation typo fixes --- lib/spack/docs/index.rst | 2 +- lib/spack/docs/packaging_guide.rst | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/index.rst b/lib/spack/docs/index.rst index 79757208c9..d6ce52b747 100644 --- a/lib/spack/docs/index.rst +++ b/lib/spack/docs/index.rst @@ -18,7 +18,7 @@ configurations can coexist on the same system. Most importantly, Spack is *simple*. It offers a simple *spec* syntax so that users can specify versions and configuration options concisely. Spack is also simple for package authors: package files -are writtin in pure Python, and specs allow package authors to +are written in pure Python, and specs allow package authors to maintain a single file for many different builds of the same package. See the :doc:`features` for examples and highlights. diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index ef9fd89b62..169899212d 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -419,7 +419,7 @@ directory to the directory containing the downloaded archive before it calls your ``install`` method. Within ``install``, the path to the downloaded archive is available as ``self.stage.archive_file``. -Here is an example snippet for packages distribuetd as self-extracting +Here is an example snippet for packages distributed as self-extracting archives. The example sets permissions on the downloaded file to make it executable, then runs it with some arguments. @@ -1556,12 +1556,12 @@ you ask for a particular spec. ``Concretization Policies`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -A user may have certain perferrences for how packages should +A user may have certain preferences for how packages should be concretized on their system. For example, one user may prefer packages built with OpenMPI and the Intel compiler. Another user may prefer packages be built with MVAPICH and GCC. -Spack can be configurated to prefer certain compilers, package +Spack can be configured to prefer certain compilers, package versions, depends_on, and variants during concretization. The preferred configuration can be controlled via the ``~/.spack/packages.yaml`` file for user configuations, or the @@ -1588,16 +1588,16 @@ At a high level, this example is specifying how packages should be concretized. The dyninst package should prefer using gcc 4.9 and be built with debug options. The gperftools package should prefer version 2.2 over 2.4. Every package on the system should prefer mvapich for -its MPI and gcc 4.4.7 (except for Dyninst, which overrides this by perfering gcc 4.9). +its MPI and gcc 4.4.7 (except for Dyninst, which overrides this by preferring gcc 4.9). These options are used to fill in implicit defaults. Any of them can be overwritten on the command line if explicitly requested. -Each packages.yaml file begin with the string ``packages:`` and +Each packages.yaml file begins with the string ``packages:`` and package names are specified on the next level. The special string ``all`` applies settings to each package. Underneath each package name is one or more components: ``compiler``, ``variants``, ``version``, or ``providers``. Each component has an ordered list of spec -``constraints``, with earlier entries in the list being prefered over +``constraints``, with earlier entries in the list being preferred over later entries. Sometimes a package installation may have constraints that forbid -- cgit v1.2.3-70-g09d2 From 3383486adc86ba03456aae3703c0176620888e77 Mon Sep 17 00:00:00 2001 From: Elizabeth F Date: Sun, 13 Mar 2016 19:38:05 -0400 Subject: Fixed typo bug. Made error comment more explicit --- lib/spack/spack/cmd/diy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/diy.py b/lib/spack/spack/cmd/diy.py index 2c3a8761ab..199362d915 100644 --- a/lib/spack/spack/cmd/diy.py +++ b/lib/spack/spack/cmd/diy.py @@ -75,8 +75,8 @@ def diy(self, args): edit_package(spec.name, spack.repo.first_repo(), None, True) return - if not spec.version.concrete: - tty.die("spack diy spec must have a single, concrete version.") + if not spec.versions.concrete: + tty.die("spack spconfig spec must have a single, concrete version. Did you forget a package version number?") spec.concretize() package = spack.repo.get(spec) -- cgit v1.2.3-70-g09d2 From 5c865b9b702695797ab29263fb629b53e56983fb Mon Sep 17 00:00:00 2001 From: Elizabeth F Date: Sun, 13 Mar 2016 19:42:15 -0400 Subject: Fixed typo in typo fix. --- lib/spack/spack/cmd/diy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/diy.py b/lib/spack/spack/cmd/diy.py index 199362d915..45f13e4463 100644 --- a/lib/spack/spack/cmd/diy.py +++ b/lib/spack/spack/cmd/diy.py @@ -76,7 +76,7 @@ def diy(self, args): return if not spec.versions.concrete: - tty.die("spack spconfig spec must have a single, concrete version. Did you forget a package version number?") + tty.die("spack diy spec must have a single, concrete version. Did you forget a package version number?") spec.concretize() package = spack.repo.get(spec) -- cgit v1.2.3-70-g09d2 From 003fd4d834d3e06ea89c7f3c2fa13241b0730f06 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 14 Mar 2016 04:55:30 -0700 Subject: Optimize __eq__ and __ne__ in key_ordering - use `is` when possible before calling `_cmp_key()` --- lib/spack/llnl/util/lang.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py index 1c4d1ed623..13d301f84e 100644 --- a/lib/spack/llnl/util/lang.py +++ b/lib/spack/llnl/util/lang.py @@ -235,11 +235,11 @@ def key_ordering(cls): if not has_method(cls, '_cmp_key'): raise TypeError("'%s' doesn't define _cmp_key()." % cls.__name__) - setter('__eq__', lambda s,o: o is not None and s._cmp_key() == o._cmp_key()) + setter('__eq__', lambda s,o: (s is o) or (o is not None and s._cmp_key() == o._cmp_key())) setter('__lt__', lambda s,o: o is not None and s._cmp_key() < o._cmp_key()) setter('__le__', lambda s,o: o is not None and s._cmp_key() <= o._cmp_key()) - setter('__ne__', lambda s,o: o is None or s._cmp_key() != o._cmp_key()) + setter('__ne__', lambda s,o: (s is not o) and (o is None or s._cmp_key() != o._cmp_key())) setter('__gt__', lambda s,o: o is None or s._cmp_key() > o._cmp_key()) setter('__ge__', lambda s,o: o is None or s._cmp_key() >= o._cmp_key()) -- cgit v1.2.3-70-g09d2 From 05c761dee9c86faf9ce6d5b98ae57c8737694898 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 14 Mar 2016 04:59:29 -0700 Subject: Add `package_class` method to spec. - Shouldn't call .package from within things like normalize() and concretize() beacuse spec may be inconsistent. - Add `.package_class` property so that we can get at package metadata without constructing a Package with a Spec. - should be faster than `.package` was, anyway. Use where possible. --- lib/spack/spack/concretize.py | 2 +- lib/spack/spack/repository.py | 9 +++++++-- lib/spack/spack/spec.py | 12 ++++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 8d29a03f93..445ecd8896 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -238,7 +238,7 @@ class DefaultConcretizer(object): the default variants from the package specification. """ changed = False - for name, variant in spec.package.variants.items(): + for name, variant in spec.package_class.variants.items(): if name not in spec.variants: spec.variants[name] = spack.spec.VariantSpec(name, variant.default) changed = True diff --git a/lib/spack/spack/repository.py b/lib/spack/spack/repository.py index 3c3ba08bcc..d2fdc937f7 100644 --- a/lib/spack/spack/repository.py +++ b/lib/spack/spack/repository.py @@ -316,6 +316,11 @@ class RepoPath(object): return self.repo_for_pkg(spec).get(spec) + def get_pkg_class(self, pkg_name): + """Find a class for the spec's package and return the class object.""" + return self.repo_for_pkg(pkg_name).get_pkg_class(pkg_name) + + @_autospec def dump_provenance(self, spec, path): """Dump provenance information for a spec to a particular path. @@ -550,7 +555,7 @@ class Repo(object): key = hash(spec) if new or key not in self._instances: - package_class = self._get_pkg_class(spec.name) + package_class = self.get_pkg_class(spec.name) try: copy = spec.copy() # defensive copy. Package owns its spec. self._instances[key] = package_class(copy) @@ -715,7 +720,7 @@ class Repo(object): return self._modules[pkg_name] - def _get_pkg_class(self, pkg_name): + def get_pkg_class(self, pkg_name): """Get the class for the package out of its module. First loads (or fetches from cache) a module for the diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index c045e80365..573e288d17 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -353,7 +353,7 @@ class VariantMap(HashableMap): @property def concrete(self): return self.spec._concrete or all( - v in self for v in self.spec.package.variants) + v in self for v in self.spec.package_class.variants) def copy(self): @@ -498,6 +498,14 @@ class Spec(object): return spack.repo.get(self) + @property + def package_class(self): + """Internal package call gets only the class object for a package. + Use this to just get package metadata. + """ + return spack.repo.get_pkg_class(self.name) + + @property def virtual(self): """Right now, a spec is virtual if no package exists with its name. @@ -1161,7 +1169,7 @@ class Spec(object): # Ensure that variants all exist. for vname, variant in spec.variants.items(): - if vname not in spec.package.variants: + if vname not in spec.package_class.variants: raise UnknownVariantError(spec.name, vname) -- cgit v1.2.3-70-g09d2 From f45b8b1083e5f628dd31fa4b7b873b6df7119d0e Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 14 Mar 2016 05:02:50 -0700 Subject: Add some tests for packages with multiple virtual dependencies. - Added mock `hypre` package, depends on `lapack` and `blas`. - test cases where some packages provide both `lapack` and `blas`, but others do not. --- lib/spack/spack/test/concretize.py | 29 +++++++++++++++- .../repos/builtin.mock/packages/hypre/package.py | 39 ++++++++++++++++++++++ .../packages/openblas-with-lapack/package.py | 38 +++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 var/spack/repos/builtin.mock/packages/hypre/package.py create mode 100644 var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py (limited to 'lib') diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 07828d8ea6..f264faf17a 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -142,6 +142,34 @@ class ConcretizeTest(MockPackagesTest): for spec in spack.repo.providers_for('mpi@3'))) + def test_concretize_two_virtuals(self): + """Test a package with multiple virtual dependencies.""" + s = Spec('hypre').concretize() + + + def test_concretize_two_virtuals_with_one_bound(self): + """Test a package with multiple virtual dependencies and one preset.""" + s = Spec('hypre ^openblas').concretize() + + + def test_concretize_two_virtuals_with_two_bound(self): + """Test a package with multiple virtual dependencies and two of them preset.""" + s = Spec('hypre ^openblas ^netlib-lapack').concretize() + + + def test_concretize_two_virtuals_with_dual_provider(self): + """Test a package with multiple virtual dependencies and force a provider + that provides both.""" + s = Spec('hypre ^openblas-with-lapack').concretize() + + + def test_concretize_two_virtuals_with_dual_provider_and_a_conflict(self): + """Test a package with multiple virtual dependencies and force a provider + that provides both, and another conflicting package that provides one.""" + s = Spec('hypre ^openblas-with-lapack ^netlib-lapack') + self.assertRaises(spack.spec.MultipleProviderError, s.concretize) + + def test_virtual_is_fully_expanded_for_callpath(self): # force dependence on fake "zmpi" by asking for MPI 10.0 spec = Spec('callpath ^mpi@10.0') @@ -281,4 +309,3 @@ class ConcretizeTest(MockPackagesTest): Spec('d')), Spec('e')) self.assertEqual(None, find_spec(s['b'], lambda s: '+foo' in s)) - diff --git a/var/spack/repos/builtin.mock/packages/hypre/package.py b/var/spack/repos/builtin.mock/packages/hypre/package.py new file mode 100644 index 0000000000..f69f16d2cc --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/hypre/package.py @@ -0,0 +1,39 @@ +############################################################################## +# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License (as published by +# the Free Software Foundation) version 2.1 dated February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + +class Hypre(Package): + """Hypre is included here as an example of a package that depends on + both LAPACK and BLAS.""" + homepage = "http://www.openblas.net" + url = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz" + + version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9') + + depends_on('lapack') + depends_on('blas') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py b/var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py new file mode 100644 index 0000000000..509bfb71e5 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py @@ -0,0 +1,38 @@ +############################################################################## +# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License (as published by +# the Free Software Foundation) version 2.1 dated February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + +class OpenblasWithLapack(Package): + """Dummy version of OpenBLAS that also provides LAPACK, for testing.""" + homepage = "http://www.openblas.net" + url = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz" + + version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9') + + provides('lapack') + provides('blas') + + def install(self, spec, prefix): + pass -- cgit v1.2.3-70-g09d2 From f2761270f3c0506a689b484b0e12d7d6e9f4300d Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 14 Mar 2016 05:04:01 -0700 Subject: Make concretization less greedy: add backtracking for virtuals. - `_expand_virtual_packages` now gets a candidate list and will try all the candidates. - Good news: If the first virtual in the list conflicts with something else in the spec, we'll keep trying until we find a good one. - Bad news: Only looks as far as the next normalize(); can't see conflicts further ahead than that if they're inevitable some other virtual expansion. - Refactor `concretize.py` to keep all the nasty spec graph stitching in `spec.py`. This is more similar to before externals support. - `concretize.py` now just returns a list of candidates sorted by ABI compatibility to `_expand_virtual_packages`, and `spec.py` handles testing the candidates. - Refactor the way external paths are handled in `config.py` and `concretize.py`: - previously, `spec_externals` returned spec/path pairs. Now it returns specs with `external` set. Makes code in `concretize.py` more natural. --- lib/spack/spack/concretize.py | 140 +++++++++++++++++------------------------- lib/spack/spack/config.py | 17 ++--- lib/spack/spack/spec.py | 129 +++++++++++++++++++++++++++++++------- 3 files changed, 174 insertions(+), 112 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 445ecd8896..8083f91982 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -51,10 +51,10 @@ class DefaultConcretizer(object): """ def _valid_virtuals_and_externals(self, spec): - """Returns a list of spec/external-path pairs for both virtuals and externals - that can concretize this spec.""" - # Get a list of candidate packages that could satisfy this spec - packages = [] + """Returns a list of candidate virtual dep providers and external + packages that coiuld be used to concretize a spec.""" + # First construct a list of concrete candidates to replace spec with. + candidates = [spec] if spec.virtual: providers = spack.repo.providers_for(spec) if not providers: @@ -64,96 +64,72 @@ class DefaultConcretizer(object): if not spec_w_preferred_providers: spec_w_preferred_providers = spec provider_cmp = partial(spack.pkgsort.provider_compare, spec_w_preferred_providers.name, spec.name) - packages = sorted(providers, cmp=provider_cmp) - else: - packages = [spec] - - # For each candidate package, if it has externals add those to the candidates - # if it's not buildable, then only add the externals. - candidates = [] - all_compilers = spack.compilers.all_compilers() - for pkg in packages: - externals = spec_externals(pkg) - buildable = is_spec_buildable(pkg) - if buildable: - candidates.append((pkg, None)) + candidates = sorted(providers, cmp=provider_cmp) + + # For each candidate package, if it has externals, add those to the usable list. + # if it's not buildable, then *only* add the externals. + usable = [] + for cspec in candidates: + if is_spec_buildable(cspec): + usable.append(cspec) + externals = spec_externals(cspec) for ext in externals: - if ext[0].satisfies(spec): - candidates.append(ext) - if not candidates: + if ext.satisfies(spec): + usable.append(ext) + + # If nothing is in the usable list now, it's because we aren't + # allowed to build anything. + if not usable: raise NoBuildError(spec) def cmp_externals(a, b): - if a[0].name != b[0].name: - #We're choosing between different providers. Maintain order from above sort + if a.name != b.name: + # We're choosing between different providers, so + # maintain order from provider sort return candidates.index(a) - candidates.index(b) - result = cmp_specs(a[0], b[0]) + + result = cmp_specs(a, b) if result != 0: return result - if not a[1] and b[1]: - return 1 - if not b[1] and a[1]: - return -1 - return cmp(a[1], b[1]) - candidates = sorted(candidates, cmp=cmp_externals) - return candidates + # prefer external packages to internal packages. + if a.external is None or b.external is None: + return -cmp(a.external, b.external) + else: + return cmp(a.external, b.external) + + usable.sort(cmp=cmp_externals) + return usable - def concretize_virtual_and_external(self, spec): - """From a list of candidate virtual and external packages, concretize to one that - is ABI compatible with the rest of the DAG.""" + def choose_virtual_or_external(self, spec): + """Given a list of candidate virtual and external packages, try to + find one that is most ABI compatible. + """ candidates = self._valid_virtuals_and_externals(spec) if not candidates: - return False - - # Find the nearest spec in the dag that has a compiler. We'll use that - # spec to test compiler compatibility. - other_spec = find_spec(spec, lambda(x): x.compiler) - if not other_spec: - other_spec = spec.root - - # Choose an ABI-compatible candidate, or the first match otherwise. - candidate = None - if other_spec: - candidate = next((c for c in candidates if spack.abi.compatible(c[0], other_spec)), None) - if not candidate: - # Try a looser ABI matching - candidate = next((c for c in candidates if spack.abi.compatible(c[0], other_spec, loose=True)), None) - if not candidate: - # No ABI matches. Pick the top choice based on the orignal preferences. - candidate = candidates[0] - candidate_spec = candidate[0] - external = candidate[1] - changed = False - - # If we're external then trim the dependencies - if external: - if (spec.dependencies): - changed = True - spec.dependencies = DependencyMap() - candidate_spec.dependencies = DependencyMap() - - def fequal(candidate_field, spec_field): - return (not candidate_field) or (candidate_field == spec_field) - if (fequal(candidate_spec.name, spec.name) and - fequal(candidate_spec.versions, spec.versions) and - fequal(candidate_spec.compiler, spec.compiler) and - fequal(candidate_spec.architecture, spec.architecture) and - fequal(candidate_spec.dependencies, spec.dependencies) and - fequal(candidate_spec.variants, spec.variants) and - fequal(external, spec.external)): - return changed - - # Refine this spec to the candidate. - if spec.virtual: - spec._replace_with(candidate_spec) - changed = True - if spec._dup(candidate_spec, deps=False, cleardeps=False): - changed = True - spec.external = external - - return changed + return candidates + + # Find the nearest spec in the dag that has a compiler. We'll + # use that spec to calibrate compiler compatibility. + abi_exemplar = find_spec(spec, lambda(x): x.compiler) + if not abi_exemplar: + abi_exemplar = spec.root + + # Make a list including ABI compatibility of specs with the exemplar. + strict = [spack.abi.compatible(c, abi_exemplar) for c in candidates] + loose = [spack.abi.compatible(c, abi_exemplar, loose=True) for c in candidates] + keys = zip(strict, loose, candidates) + + # Sort candidates from most to least compatibility. + # Note: + # 1. We reverse because True > False. + # 2. Sort is stable, so c's keep their order. + keys.sort(key=lambda k:k[:2], reverse=True) + + # Pull the candidates back out and return them in order + candidates = [c for s,l,c in keys] + return candidates def concretize_version(self, spec): diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index a21dd6dbe1..6afd69b3ac 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -539,22 +539,25 @@ def print_section(section): def spec_externals(spec): - """Return a list of spec, directory pairs for each external location for spec""" + """Return a list of external specs (with external directory path filled in), + one for each known external installation.""" allpkgs = get_config('packages') name = spec.name - spec_locations = [] + external_specs = [] pkg_paths = allpkgs.get(name, {}).get('paths', None) if not pkg_paths: return [] - for pkg,path in pkg_paths.iteritems(): - if not spec.satisfies(pkg): - continue + for external_spec, path in pkg_paths.iteritems(): if not path: + # skip entries without paths (avoid creating extra Specs) continue - spec_locations.append( (spack.spec.Spec(pkg), path) ) - return spec_locations + + external_spec = spack.spec.Spec(external_spec, external=path) + if external_spec.satisfies(spec): + external_specs.append(external_spec) + return external_specs def is_spec_buildable(spec): diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 573e288d17..d04135860e 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -418,9 +418,11 @@ class Spec(object): # cases we've read them from a file want to assume normal. # This allows us to manipulate specs that Spack doesn't have # package.py files for. - self._normal = kwargs.get('normal', False) + self._normal = kwargs.get('normal', False) self._concrete = kwargs.get('concrete', False) - self.external = None + + # Allow a spec to be constructed with an external path. + self.external = kwargs.get('external', None) # This allows users to construct a spec DAG with literals. # Note that given two specs a and b, Spec(a) copies a, but @@ -794,8 +796,30 @@ class Spec(object): """Replace this virtual spec with a concrete spec.""" assert(self.virtual) for name, dependent in self.dependents.items(): + # remove self from all dependents. del dependent.dependencies[self.name] - dependent._add_dependency(concrete) + + # add the replacement, unless it is already a dep of dependent. + if concrete.name not in dependent.dependencies: + dependent._add_dependency(concrete) + + + def _replace_node(self, replacement): + """Replace this spec with another. + + Connects all dependents of this spec to its replacement, and + disconnects this spec from any dependencies it has. New spec + will have any dependencies the replacement had, and may need + to be normalized. + + """ + for name, dependent in self.dependents.items(): + del dependent.dependencies[self.name] + dependent._add_dependency(replacement) + + for name, dep in self.dependencies.items(): + del dep.dependents[self.name] + del self.dependencies[dep.name] def _expand_virtual_packages(self): @@ -815,18 +839,80 @@ class Spec(object): this are infrequent, but should implement this before it is a problem. """ + # Make an index of stuff this spec already provides + self_index = ProviderIndex(self.traverse(), restrict=True) + changed = False done = False while not done: done = True for spec in list(self.traverse()): - if spack.concretizer.concretize_virtual_and_external(spec): - done = False + replacement = None + if spec.virtual: + replacement = self._find_provider(spec, self_index) + if replacement: + # TODO: may break if in-place on self but + # shouldn't happen if root is traversed first. + spec._replace_with(replacement) + done=False + break + + if not replacement: + # Get a list of possible replacements in order of preference. + candidates = spack.concretizer.choose_virtual_or_external(spec) + + # Try the replacements in order, skipping any that cause + # satisfiability problems. + for replacement in candidates: + if replacement is spec: + break + + # Replace spec with the candidate and normalize + copy = self.copy() + copy[spec.name]._dup(replacement.copy(deps=False)) + + try: + # If there are duplicate providers or duplicate provider + # deps, consolidate them and merge constraints. + copy.normalize(force=True) + break + except SpecError as e: + # On error, we'll try the next replacement. + continue + + # If replacement is external then trim the dependencies + if replacement.external: + if (spec.dependencies): + changed = True + spec.dependencies = DependencyMap() + replacement.dependencies = DependencyMap() + + # TODO: could this and the stuff in _dup be cleaned up? + def feq(cfield, sfield): + return (not cfield) or (cfield == sfield) + + if replacement is spec or (feq(replacement.name, spec.name) and + feq(replacement.versions, spec.versions) and + feq(replacement.compiler, spec.compiler) and + feq(replacement.architecture, spec.architecture) and + feq(replacement.dependencies, spec.dependencies) and + feq(replacement.variants, spec.variants) and + feq(replacement.external, spec.external)): + continue + + # Refine this spec to the candidate. This uses + # replace_with AND dup so that it can work in + # place. TODO: make this more efficient. + if spec.virtual: + spec._replace_with(replacement) changed = True + if spec._dup(replacement, deps=False, cleardeps=False): + changed = True + + self_index.update(spec) + done=False + break - # If there are duplicate providers or duplicate provider deps, this - # consolidates them and merge constraints. - changed |= self.normalize(force=True) return changed @@ -850,7 +936,7 @@ class Spec(object): force = False while changed: - changes = (self.normalize(force=force), + changes = (self.normalize(force), self._expand_virtual_packages(), self._concretize_helper()) changed = any(changes) @@ -976,8 +1062,8 @@ class Spec(object): def _find_provider(self, vdep, provider_index): """Find provider for a virtual spec in the provider index. - Raise an exception if there is a conflicting virtual - dependency already in this spec. + Raise an exception if there is a conflicting virtual + dependency already in this spec. """ assert(vdep.virtual) providers = provider_index.providers_for(vdep) @@ -1018,17 +1104,14 @@ class Spec(object): """ changed = False - # If it's a virtual dependency, try to find a provider and - # merge that. + # If it's a virtual dependency, try to find an existing + # provider in the spec, and merge that. if dep.virtual: visited.add(dep.name) provider = self._find_provider(dep, provider_index) if provider: dep = provider - else: - # if it's a real dependency, check whether it provides - # something already required in the spec. index = ProviderIndex([dep], restrict=True) for vspec in (v for v in spec_deps.values() if v.virtual): if index.providers_for(vspec): @@ -1125,13 +1208,14 @@ class Spec(object): # Get all the dependencies into one DependencyMap spec_deps = self.flat_dependencies(copy=False) - # Initialize index of virtual dependency providers - index = ProviderIndex(spec_deps.values(), restrict=True) + # Initialize index of virtual dependency providers if + # concretize didn't pass us one already + provider_index = ProviderIndex(spec_deps.values(), restrict=True) # traverse the package DAG and fill out dependencies according # to package files & their 'when' specs visited = set() - any_change = self._normalize_helper(visited, spec_deps, index) + any_change = self._normalize_helper(visited, spec_deps, provider_index) # If there are deps specified but not visited, they're not # actually deps of this package. Raise an error. @@ -1410,13 +1494,12 @@ class Spec(object): Whether deps should be copied too. Set to false to copy a spec but not its dependencies. """ - # We don't count dependencies as changes here changed = True if hasattr(self, 'name'): - changed = (self.name != other.name and self.versions != other.versions and \ - self.architecture != other.architecture and self.compiler != other.compiler and \ - self.variants != other.variants and self._normal != other._normal and \ + changed = (self.name != other.name and self.versions != other.versions and + self.architecture != other.architecture and self.compiler != other.compiler and + self.variants != other.variants and self._normal != other._normal and self.concrete != other.concrete and self.external != other.external) # Local node attributes get copied first. -- cgit v1.2.3-70-g09d2 From 15bbd088e6007a9a6df8c4427340a371e5871506 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Tue, 15 Mar 2016 14:38:06 -0700 Subject: Fix #551: version bug in `spack create` - `spack create` now sets a proper version in generated file, based on the filename, even if it can't find any tarballs for the package. --- lib/spack/spack/cmd/create.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py index 4564143f83..f0cd50b8df 100644 --- a/lib/spack/spack/cmd/create.py +++ b/lib/spack/spack/cmd/create.py @@ -208,7 +208,7 @@ def find_repository(spec, args): return repo -def fetch_tarballs(url, name, args): +def fetch_tarballs(url, name, version): """Try to find versions of the supplied archive by scraping the web. Prompts the user to select how many to download if many are found. @@ -222,7 +222,7 @@ def fetch_tarballs(url, name, args): archives_to_fetch = 1 if not versions: # If the fetch failed for some reason, revert to what the user provided - versions = { "version" : url } + versions = { version : url } elif len(versions) > 1: tty.msg("Found %s versions of %s:" % (len(versions), name), *spack.cmd.elide_list( @@ -256,7 +256,7 @@ def create(parser, args): tty.msg("Creating template for package %s" % name) # Fetch tarballs (prompting user if necessary) - versions, urls = fetch_tarballs(url, name, args) + versions, urls = fetch_tarballs(url, name, version) # Try to guess what configure system is used. guesser = ConfigureGuesser() -- cgit v1.2.3-70-g09d2