summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2018-12-16 23:17:23 -0800
committerTodd Gamblin <tgamblin@llnl.gov>2018-12-30 00:19:08 -0800
commit527ff860f01de34e34f8c7be8e5682745504d774 (patch)
tree97f85936301436e89e6c371033af0c070119dda6 /lib
parent19b7b1592947c5a74fbb159c0a0820cc5013a077 (diff)
downloadspack-527ff860f01de34e34f8c7be8e5682745504d774.tar.gz
spack-527ff860f01de34e34f8c7be8e5682745504d774.tar.bz2
spack-527ff860f01de34e34f8c7be8e5682745504d774.tar.xz
spack-527ff860f01de34e34f8c7be8e5682745504d774.zip
patches: clean up patch.py, directives, and package class properties
- cleanup patch.py: - make patch.py constructors more understandable - loosen coupling of patch.py with package - in Package: make package_dir, module, and namespace class properties - These were previously instance properties and couldn't be called from directives, e.g. in patch.create() - make them class properties so that they can be used in class definition - also add some instance properties to delegate to class properties so that prior usage on Package objects still works
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/directives.py13
-rw-r--r--lib/spack/spack/package.py52
-rw-r--r--lib/spack/spack/patch.py141
-rw-r--r--lib/spack/spack/test/cmd/env.py2
-rw-r--r--lib/spack/spack/test/install.py5
-rw-r--r--lib/spack/spack/test/patch.py5
6 files changed, 119 insertions, 99 deletions
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index 9125d55e9a..57492f5c1c 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -36,12 +36,12 @@ from six import string_types
import llnl.util.lang
import spack.error
+import spack.patch
import spack.spec
import spack.url
import spack.variant
from spack.dependency import Dependency, default_deptype, canonical_deptype
from spack.fetch_strategy import from_kwargs
-from spack.patch import Patch
from spack.resource import Resource
from spack.version import Version
@@ -416,9 +416,14 @@ def patch(url_or_filename, level=1, when=None, working_dir=".", **kwargs):
# if this spec is identical to some other, then append this
# patch to the existing list.
cur_patches = pkg_or_dep.patches.setdefault(when_spec, [])
- cur_patches.append(
- Patch.create(pkg_or_dep, url_or_filename, level,
- working_dir, **kwargs))
+
+ # if pkg_or_dep is a Dependency, make it a Package
+ pkg = pkg_or_dep
+ if isinstance(pkg, Dependency):
+ pkg = pkg.pkg
+
+ cur_patches.append(spack.patch.create(
+ pkg, url_or_filename, level, working_dir, **kwargs))
return _execute_patch
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 279ed89d04..7df500039e 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -200,6 +200,29 @@ class PackageMeta(
return func
return _decorator
+ @property
+ def package_dir(self):
+ """Directory where the package.py file lives."""
+ return os.path.abspath(os.path.dirname(self.module.__file__))
+
+ @property
+ def module(self):
+ """Module object (not just the name) that this package is defined in.
+
+ We use this to add variables to package modules. This makes
+ install() methods easier to write (e.g., can call configure())
+ """
+ return __import__(self.__module__, fromlist=[self.__name__])
+
+ @property
+ def namespace(self):
+ """Spack namespace for the package, which identifies its repo."""
+ namespace, dot, module = self.__module__.rpartition('.')
+ prefix = '%s.' % spack.repo.repo_namespace
+ if namespace.startswith(prefix):
+ namespace = namespace[len(prefix):]
+ return namespace
+
def run_before(*phases):
"""Registers a method of a package to be run before a given phase"""
@@ -527,10 +550,22 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
return visited
+ # package_dir and module are *class* properties (see PackageMeta),
+ # but to make them work on instances we need these defs as well.
@property
def package_dir(self):
- """Return the directory where the package.py file lives."""
- return os.path.abspath(os.path.dirname(self.module.__file__))
+ """Directory where the package.py file lives."""
+ return type(self).package_dir
+
+ @property
+ def module(self):
+ """Module object that this package is defined in."""
+ return type(self).module
+
+ @property
+ def namespace(self):
+ """Spack namespace for the package, which identifies its repo."""
+ return type(self).namespace
@property
def global_license_dir(self):
@@ -1081,11 +1116,6 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
hashlib.sha256(bytes().join(
sorted(hash_content))).digest()).lower()
- @property
- def namespace(self):
- namespace, dot, module = self.__module__.rpartition('.')
- return namespace
-
def do_fake_install(self):
"""Make a fake install directory containing fake executables,
headers, and libraries."""
@@ -1724,14 +1754,6 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
else:
return os.path.join(self.stage.source_path, 'spack-build.out')
- @property
- def module(self):
- """Use this to add variables to the class's module's scope.
- This lets us use custom syntax in the install method.
- """
- return __import__(self.__class__.__module__,
- fromlist=[self.__class__.__name__])
-
@classmethod
def inject_flags(cls, name, flags):
"""
diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py
index cbab403f20..69949564c5 100644
--- a/lib/spack/spack/patch.py
+++ b/lib/spack/spack/patch.py
@@ -5,96 +5,92 @@
import os
import os.path
-import inspect
import hashlib
+import llnl.util.filesystem
+
import spack.error
import spack.fetch_strategy as fs
import spack.stage
from spack.util.crypto import checksum, Checker
-from llnl.util.filesystem import working_dir
+
from spack.util.executable import which
from spack.util.compression import allowed_archive
-def absolute_path_for_package(pkg):
- """Returns the absolute path to the ``package.py`` file implementing
- the recipe for the package passed as argument.
+def create(pkg, path_or_url, level=1, working_dir=".", **kwargs):
+ """Make either a FilePatch or a UrlPatch, depending on arguments.
- Args:
- pkg: a valid package object, or a Dependency object.
+ Args:
+ pkg: package that needs to be patched
+ path_or_url: path or url where the patch is found
+ level: patch level (default 1)
+ working_dir (str): relative path within the package stage;
+ change to this before before applying (default '.')
+
+ Returns:
+ (Patch): a patch object on which ``apply(stage)`` can be called
"""
- if isinstance(pkg, spack.dependency.Dependency):
- pkg = pkg.pkg
- m = inspect.getmodule(pkg)
- return os.path.abspath(m.__file__)
+ # Check if we are dealing with a URL (which will be fetched)
+ if '://' in path_or_url:
+ return UrlPatch(path_or_url, level, working_dir, **kwargs)
+ # If not, it's a file patch, which is stored within the repo directory.
+ patch_path = os.path.join(pkg.package_dir, path_or_url)
+ return FilePatch(patch_path, level, working_dir)
-class Patch(object):
- """Base class to describe a patch that needs to be applied to some
- expanded source code.
- """
- @staticmethod
- def create(pkg, path_or_url, level=1, working_dir=".", **kwargs):
- """
- Factory method that creates an instance of some class derived from
- Patch
+def apply_patch(stage, patch_path, level=1, working_dir='.'):
+ """Apply the patch at patch_path to code in the stage.
- Args:
- pkg: package that needs to be patched
- path_or_url: path or url where the patch is found
- level: patch level (default 1)
- working_dir (str): dir to change to before applying (default '.')
+ Args:
+ stage (spack.stage.Stage): stage with code that will be patched
+ patch_path (str): filesystem location for the patch to apply
+ level (int, optional): patch level (default 1)
+ working_dir (str): relative path *within* the stage to change to
+ (default '.')
+ """
+ patch = which("patch", required=True)
+ with llnl.util.filesystem.working_dir(stage.source_path):
+ patch('-s',
+ '-p', str(level),
+ '-i', patch_path,
+ '-d', working_dir)
- Returns:
- instance of some Patch class
- """
- # Check if we are dealing with a URL
- if '://' in path_or_url:
- return UrlPatch(path_or_url, level, working_dir, **kwargs)
- # Assume patches are stored in the repository
- return FilePatch(pkg, path_or_url, level, working_dir)
+class Patch(object):
+ """Base class for patches.
+
+ Defines the interface (basically just ``apply()``, at the moment) and
+ common variables.
+ """
def __init__(self, path_or_url, level, working_dir):
- # Check on level (must be an integer > 0)
+ # validate level (must be an integer >= 0)
if not isinstance(level, int) or not level >= 0:
raise ValueError("Patch level needs to be a non-negative integer.")
+
# Attributes shared by all patch subclasses
- self.path_or_url = path_or_url
+ self.path_or_url = path_or_url # needed for debug output
self.level = level
self.working_dir = working_dir
- # self.path needs to be computed by derived classes
- # before a call to apply
- self.path = None
- if not isinstance(self.level, int) or not self.level >= 0:
- raise ValueError("Patch level needs to be a non-negative integer.")
+ # path needs to be set by subclasses before calling self.apply()
+ self.path = None
def apply(self, stage):
- """Apply the patch at self.path to the source code in the
- supplied stage
-
- Args:
- stage: stage for the package that needs to be patched
- """
- patch = which("patch", required=True)
- with working_dir(stage.source_path):
- # Use -N to allow the same patches to be applied multiple times.
- patch('-s', '-p', str(self.level), '-i', self.path,
- "-d", self.working_dir)
+ """Apply this patch to code in a stage."""
+ assert self.path, "self.path must be set before Patch.apply()"
+ apply_patch(stage, self.path, self.level, self.working_dir)
class FilePatch(Patch):
"""Describes a patch that is retrieved from a file in the repository"""
- def __init__(self, pkg, path_or_url, level, working_dir):
- super(FilePatch, self).__init__(path_or_url, level, working_dir)
-
- pkg_dir = os.path.dirname(absolute_path_for_package(pkg))
- self.path = os.path.join(pkg_dir, path_or_url)
- if not os.path.isfile(self.path):
- raise NoSuchPatchError(
- "No such patch for package %s: %s" % (pkg.name, self.path))
+ def __init__(self, path, level, working_dir):
+ super(FilePatch, self).__init__(path, level, working_dir)
+
+ if not os.path.isfile(path):
+ raise NoSuchPatchError("No such patch: %s" % path)
+ self.path = path
self._sha256 = None
@property
@@ -106,21 +102,20 @@ class FilePatch(Patch):
class UrlPatch(Patch):
"""Describes a patch that is retrieved from a URL"""
- def __init__(self, path_or_url, level, working_dir, **kwargs):
- super(UrlPatch, self).__init__(path_or_url, level, working_dir)
- self.url = path_or_url
-
- self.archive_sha256 = None
- if allowed_archive(self.url):
- if 'archive_sha256' not in kwargs:
- raise PatchDirectiveError(
- "Compressed patches require 'archive_sha256' "
- "and patch 'sha256' attributes: %s" % self.url)
- self.archive_sha256 = kwargs.get('archive_sha256')
-
- if 'sha256' not in kwargs:
- raise PatchDirectiveError("URL patches require a sha256 checksum")
+ def __init__(self, url, level, working_dir, **kwargs):
+ super(UrlPatch, self).__init__(url, level, working_dir)
+
+ self.url = url
+
+ self.archive_sha256 = kwargs.get('archive_sha256')
+ if allowed_archive(self.url) and not self.archive_sha256:
+ raise PatchDirectiveError(
+ "Compressed patches require 'archive_sha256' "
+ "and patch 'sha256' attributes: %s" % self.url)
+
self.sha256 = kwargs.get('sha256')
+ if not self.sha256:
+ raise PatchDirectiveError("URL patches require a sha256 checksum")
def apply(self, stage):
"""Retrieve the patch in a temporary stage, computes
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 7085373392..965f16caa2 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -232,7 +232,7 @@ def test_env_repo():
package = e.repo.get('mpileaks')
assert package.name == 'mpileaks'
- assert package.namespace == 'spack.pkg.builtin.mock'
+ assert package.namespace == 'builtin.mock'
def test_user_removed_spec():
diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py
index a63c149d3d..94b7ff51ff 100644
--- a/lib/spack/spack/test/install.py
+++ b/lib/spack/spack/test/install.py
@@ -6,6 +6,7 @@
import os
import pytest
+import spack.patch
import spack.repo
import spack.store
from spack.spec import Spec
@@ -96,14 +97,12 @@ def test_partial_install_delete_prefix_and_stage(install_mockery, mock_fetch):
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')]
+ spack.patch.create(None, 'file://fake.patch', sha256='unused-hash')]
dependency_hash = dependency.dag_hash()
dependent = Spec('dependent-install ^/' + dependency_hash)
diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py
index e606c9113a..724924edf3 100644
--- a/lib/spack/spack/test/patch.py
+++ b/lib/spack/spack/test/patch.py
@@ -4,12 +4,12 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import os
-import sys
import filecmp
import pytest
from llnl.util.filesystem import working_dir, mkdirp
+import spack.patch
import spack.paths
import spack.util.compression
from spack.util.executable import Executable
@@ -41,8 +41,7 @@ data_path = os.path.join(spack.paths.test_path, 'data', 'patch')
def test_url_patch(mock_stage, filename, sha256, archive_sha256):
# Make a patch object
url = 'file://' + filename
- m = sys.modules['spack.patch']
- patch = m.Patch.create(
+ patch = spack.patch.create(
None, url, sha256=sha256, archive_sha256=archive_sha256)
# make a stage