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
This commit is contained in:
Todd Gamblin
parent
19b7b15929
commit
527ff860f0
7 changed files with 119 additions and 101 deletions
|
|
@ -36,12 +36,12 @@ class OpenMpi(Package):
|
|||
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 _execute_patch(pkg_or_dep):
|
|||
# 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
|
||||
|
||||
|
|
|
|||
|
|
@ -200,6 +200,29 @@ def _decorator(func):
|
|||
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 @@ def possible_dependencies(
|
|||
|
||||
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 @@ def content_hash(self, content=None):
|
|||
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 @@ def build_log_path(self):
|
|||
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):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
Args:
|
||||
pkg: a valid package object, or a Dependency object.
|
||||
"""
|
||||
if isinstance(pkg, spack.dependency.Dependency):
|
||||
pkg = pkg.pkg
|
||||
m = inspect.getmodule(pkg)
|
||||
return os.path.abspath(m.__file__)
|
||||
|
||||
|
||||
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
|
||||
"""Make either a FilePatch or a UrlPatch, depending on arguments.
|
||||
|
||||
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 '.')
|
||||
working_dir (str): relative path within the package stage;
|
||||
change to this before before applying (default '.')
|
||||
|
||||
Returns:
|
||||
instance of some Patch class
|
||||
(Patch): a patch object on which ``apply(stage)`` can be called
|
||||
"""
|
||||
# Check if we are dealing with a URL
|
||||
# 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)
|
||||
# Assume patches are stored in the repository
|
||||
return FilePatch(pkg, path_or_url, level, working_dir)
|
||||
|
||||
def __init__(self, path_or_url, level, working_dir):
|
||||
# Check on 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.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, 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)
|
||||
|
||||
if not isinstance(self.level, int) or not self.level >= 0:
|
||||
raise ValueError("Patch level needs to be a non-negative integer.")
|
||||
|
||||
def apply(self, stage):
|
||||
"""Apply the patch at self.path to the source code in the
|
||||
supplied stage
|
||||
def apply_patch(stage, patch_path, level=1, working_dir='.'):
|
||||
"""Apply the patch at patch_path to code in the stage.
|
||||
|
||||
Args:
|
||||
stage: stage for the package that needs to be patched
|
||||
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 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)
|
||||
with llnl.util.filesystem.working_dir(stage.source_path):
|
||||
patch('-s',
|
||||
'-p', str(level),
|
||||
'-i', patch_path,
|
||||
'-d', 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):
|
||||
# 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 # needed for debug output
|
||||
self.level = level
|
||||
self.working_dir = working_dir
|
||||
|
||||
# path needs to be set by subclasses before calling self.apply()
|
||||
self.path = None
|
||||
|
||||
def apply(self, stage):
|
||||
"""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)
|
||||
def __init__(self, path, level, working_dir):
|
||||
super(FilePatch, self).__init__(path, 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))
|
||||
if not os.path.isfile(path):
|
||||
raise NoSuchPatchError("No such patch: %s" % path)
|
||||
self.path = path
|
||||
self._sha256 = None
|
||||
|
||||
@property
|
||||
|
|
@ -106,21 +102,20 @@ def sha256(self):
|
|||
|
||||
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
|
||||
def __init__(self, url, level, working_dir, **kwargs):
|
||||
super(UrlPatch, self).__init__(url, level, working_dir)
|
||||
|
||||
self.archive_sha256 = None
|
||||
if allowed_archive(self.url):
|
||||
if 'archive_sha256' not in kwargs:
|
||||
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.archive_sha256 = kwargs.get('archive_sha256')
|
||||
|
||||
if 'sha256' not in kwargs:
|
||||
raise PatchDirectiveError("URL patches require a sha256 checksum")
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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 @@ def mock_stage(tmpdir, monkeypatch):
|
|||
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
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@
|
|||
import os
|
||||
import subprocess
|
||||
import llnl.util.tty as tty
|
||||
from spack.patch import absolute_path_for_package
|
||||
|
||||
|
||||
class Catalyst(CMakePackage):
|
||||
|
|
@ -52,11 +51,10 @@ def patch(self):
|
|||
at the package dir to the source code in
|
||||
root_cmakelists_dir."""
|
||||
patch_name = 'vtkm-catalyst-pv551.patch'
|
||||
pkg_dir = os.path.dirname(absolute_path_for_package(self))
|
||||
patch = which("patch", required=True)
|
||||
with working_dir(self.root_cmakelists_dir):
|
||||
patch('-s', '-p', '1', '-i',
|
||||
join_path(pkg_dir, patch_name),
|
||||
join_path(self.package_dir, patch_name),
|
||||
"-d", '.')
|
||||
|
||||
def url_for_version(self, version):
|
||||
|
|
|
|||
Loading…
Reference in a new issue