From 6243a28da17079571117d480572f4962ec70c441 Mon Sep 17 00:00:00 2001 From: scheibelp Date: Thu, 5 Oct 2017 10:33:04 -0700 Subject: Don't change properties on already-installed packages (#5580) * edits to address issues where spack concretization attempts to set properties on already-installed specs * most added checks only need to check if the spec is concrete; they dont also need to check if the package is installed * add test to ensure that patches are not applied to an installed spec * add test to ensure that an error is detected when a dependent requests a dependency constraint which conflicts with a requested installed dependency --- lib/spack/spack/spec.py | 15 ++++++++ lib/spack/spack/test/install.py | 30 ++++++++++++++++ .../packages/conflicting-dependent/package.py | 41 ++++++++++++++++++++++ .../packages/dependency-install/package.py | 38 ++++++++++++++++++++ .../packages/dependent-install/package.py | 39 ++++++++++++++++++++ 5 files changed, 163 insertions(+) create mode 100644 var/spack/repos/builtin.mock/packages/conflicting-dependent/package.py create mode 100644 var/spack/repos/builtin.mock/packages/dependency-install/package.py create mode 100644 var/spack/repos/builtin.mock/packages/dependent-install/package.py diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 624c28285a..24da1af564 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1612,6 +1612,10 @@ class Spec(object): if self.name in visited: return False + if self.concrete: + visited.add(self.name) + return False + changed = False # Concretize deps first -- this is a bottom-up process. @@ -1805,6 +1809,9 @@ class Spec(object): if s.namespace is None: s.namespace = spack.repo.repo_for_pkg(s.name).namespace + if s.concrete: + continue + # Add any patches from the package to the spec. patches = [] for cond, patch_list in s.package_class.patches.items(): @@ -1826,6 +1833,9 @@ class Spec(object): if dspec.spec.name not in pkg_deps: continue + if dspec.spec.concrete: + continue + patches = [] for cond, dependency in pkg_deps[dspec.spec.name].items(): if dspec.parent.satisfies(cond): @@ -1878,6 +1888,8 @@ class Spec(object): unless there is a need to force a spec to be concrete. """ for s in self.traverse(deptype_query=all): + if s.concrete and s.package.installed: + continue s._normal = value s._concrete = value @@ -2067,6 +2079,9 @@ class Spec(object): # caller, it's a copy from _evaluate_dependency_conditions. If it # comes from a vdep, it's a defensive copy from _find_provider. if dep.name not in spec_deps: + if self.concrete: + return False + spec_deps[dep.name] = dep changed = True else: diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index 8887de8584..454adbac5e 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -111,6 +111,36 @@ def test_partial_install_delete_prefix_and_stage(install_mockery, mock_fetch): pass +def test_dont_add_patches_to_installed_package(install_mockery, mock_fetch): + import sys + dependency = Spec('dependency-install') + dependency.concretize() + dependency.package.do_install() + + dependency.package.patches['dependency-install'] = [ + sys.modules['spack.patch'].Patch.create( + None, 'file://fake.patch', sha256='unused-hash')] + + dependency_hash = dependency.dag_hash() + dependent = Spec('dependent-install ^/' + dependency_hash) + dependent.concretize() + + assert dependent['dependency-install'] == dependency + + +def test_installed_dependency_request_conflicts( + install_mockery, mock_fetch, refresh_builtin_mock): + dependency = Spec('dependency-install') + dependency.concretize() + dependency.package.do_install() + + dependency_hash = dependency.dag_hash() + dependent = Spec( + 'conflicting-dependent ^/' + dependency_hash) + with pytest.raises(spack.spec.UnsatisfiableSpecError): + dependent.concretize() + + def test_partial_install_keep_prefix(install_mockery, mock_fetch): spec = Spec('canfail') spec.concretize() diff --git a/var/spack/repos/builtin.mock/packages/conflicting-dependent/package.py b/var/spack/repos/builtin.mock/packages/conflicting-dependent/package.py new file mode 100644 index 0000000000..255ad1fc1c --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/conflicting-dependent/package.py @@ -0,0 +1,41 @@ +############################################################################## +# Copyright (c) 2013-2017, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the NOTICE and LICENSE files 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 Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, 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 Lesser 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 ConflictingDependent(Package): + """By itself this package does not have conflicts, but it is used to + ensure that if a user tries to build with an installed instance + of dependency-install@2 that there is a failure.""" + + homepage = "http://www.example.com" + url = "http://www.example.com/a-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') + + depends_on('dependency-install@:1.0') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/dependency-install/package.py b/var/spack/repos/builtin.mock/packages/dependency-install/package.py new file mode 100644 index 0000000000..72e2ce18af --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/dependency-install/package.py @@ -0,0 +1,38 @@ +############################################################################## +# Copyright (c) 2013-2017, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the NOTICE and LICENSE files 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 Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, 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 Lesser 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 DependencyInstall(Package): + """Dependency which has a working install method""" + + homepage = "http://www.example.com" + url = "http://www.example.com/a-1.0.tar.gz" + + version('1.0', 'hash1.0') + version('2.0', 'hash2.0') + + def install(self, spec, prefix): + touch(join_path(prefix, 'an_installation_file')) diff --git a/var/spack/repos/builtin.mock/packages/dependent-install/package.py b/var/spack/repos/builtin.mock/packages/dependent-install/package.py new file mode 100644 index 0000000000..450bdacbf5 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/dependent-install/package.py @@ -0,0 +1,39 @@ +############################################################################## +# Copyright (c) 2013-2017, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the NOTICE and LICENSE files 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 Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, 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 Lesser 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 DependentInstall(Package): + """Dependent which has a working install method""" + + homepage = "http://www.example.com" + url = "http://www.example.com/a-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') + + depends_on('dependency-install') + + def install(self, spec, prefix): + touch(join_path(prefix, 'an_installation_file')) -- cgit v1.2.3-70-g09d2