From aa86432ec6a6b8d173feebf50cc6c98202915fe6 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Fri, 21 Oct 2016 16:32:52 +0200 Subject: patch directive : fixed retrieval from urls ( fixes #1584 ) (#2039) * patch directive : fixed retrieval from urls fixes #1584 - add support for 'gz' archives - fixed bugs with URL patches - updated nwchem * patch directive : added checksum to UrlPatch - refactored classes in patch.py - updated nwchem * patch directive : added caching --- lib/spack/spack/directives.py | 4 +- lib/spack/spack/fetch_strategy.py | 5 +- lib/spack/spack/patch.py | 118 +++++++++++++++++++++++++----------- lib/spack/spack/stage.py | 4 ++ lib/spack/spack/util/compression.py | 3 + 5 files changed, 94 insertions(+), 40 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index dda9fb32d8..9cf00334a8 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -259,7 +259,7 @@ def provides(pkg, *specs, **kwargs): @directive('patches') -def patch(pkg, url_or_filename, level=1, when=None): +def patch(pkg, url_or_filename, level=1, when=None, **kwargs): """Packages can declare patches to apply to source. You can optionally provide a when spec to indicate that a particular patch should only be applied when the package's spec meets @@ -271,7 +271,7 @@ def patch(pkg, url_or_filename, level=1, when=None): cur_patches = pkg.patches.setdefault(when_spec, []) # if this spec is identical to some other, then append this # patch to the existing list. - cur_patches.append(Patch(pkg, url_or_filename, level)) + cur_patches.append(Patch.create(pkg, url_or_filename, level, **kwargs)) @directive('variants') diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 2becf9ed04..4374587250 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -286,6 +286,8 @@ class URLFetchStrategy(FetchStrategy): "URLFetchStrategy couldn't find archive file", "Failed on expand() for URL %s" % self.url) + if not self.extension: + self.extension = extension(self.archive_file) decompress = decompressor_for(self.archive_file, self.extension) # Expand all tarballs in their own directory to contain @@ -313,7 +315,8 @@ class URLFetchStrategy(FetchStrategy): shutil.move(os.path.join(tarball_container, f), os.path.join(self.stage.path, f)) os.rmdir(tarball_container) - + if not files: + os.rmdir(tarball_container) # Set the wd back to the stage when done. self.stage.chdir() diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py index 0bd9f5d29d..ee83748319 100644 --- a/lib/spack/spack/patch.py +++ b/lib/spack/spack/patch.py @@ -24,62 +24,106 @@ ############################################################################## import os -from llnl.util.filesystem import join_path - import spack -import spack.stage import spack.error +import spack.stage +import spack.fetch_strategy as fs +from llnl.util.filesystem import join_path from spack.util.executable import which -# Patch tool for patching archives. -_patch = which("patch", required=True) - class Patch(object): - """This class describes a patch to be applied to some expanded - source code.""" + """Base class to describe a patch that needs to be applied to some + expanded source code. + """ + + @staticmethod + def create(pkg, path_or_url, level, **kwargs): + """ + Factory method that creates an instance of some class derived from + Patch + + Args: + pkg: package that needs to be patched + path_or_url: path or url where the patch is found + level: patch level + + Returns: + instance of some Patch class + """ + # Check if we are dealing with a URL + if '://' in path_or_url: + return UrlPatch(pkg, path_or_url, level, **kwargs) + # Assume patches are stored in the repository + return FilePatch(pkg, path_or_url, level) def __init__(self, pkg, path_or_url, level): - self.pkg_name = pkg.name + # 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.path = None - self.url = None self.level = level + # 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.") - if '://' in path_or_url: - self.url = path_or_url - else: - pkg_dir = spack.repo.dirname_for_package_name(self.pkg_name) - self.path = join_path(pkg_dir, path_or_url) - if not os.path.isfile(self.path): - raise NoSuchPatchFileError(pkg_name, self.path) - def apply(self, stage): - """Fetch this patch, if necessary, and apply it to the source - code in the supplied 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 """ stage.chdir_to_source() + # Use -N to allow the same patches to be applied multiple times. + _patch = which("patch", required=True) + _patch('-s', '-p', str(self.level), '-i', self.path) + + +class FilePatch(Patch): + """Describes a patch that is retrieved from a file in the repository""" + def __init__(self, pkg, path_or_url, level): + super(FilePatch, self).__init__(pkg, path_or_url, level) - patch_stage = None - try: - if self.url: - # use an anonymous stage to fetch the patch if it is a URL - patch_stage = spack.stage.Stage(self.url) - patch_stage.fetch() - patch_file = patch_stage.archive_file - else: - patch_file = self.path - - # Use -N to allow the same patches to be applied multiple times. - _patch('-s', '-p', str(self.level), '-i', patch_file) - - finally: - if patch_stage: - patch_stage.destroy() + pkg_dir = spack.repo.dirname_for_package_name(pkg.name) + self.path = join_path(pkg_dir, path_or_url) + if not os.path.isfile(self.path): + raise NoSuchPatchFileError(pkg.name, self.path) + + +class UrlPatch(Patch): + """Describes a patch that is retrieved from a URL""" + def __init__(self, pkg, path_or_url, level, **kwargs): + super(UrlPatch, self).__init__(pkg, path_or_url, level) + self.url = path_or_url + self.md5 = kwargs.get('md5') + + def apply(self, stage): + """Retrieve the patch in a temporary stage, computes + self.path and calls `super().apply(stage)` + + Args: + stage: stage for the package that needs to be patched + """ + fetcher = fs.URLFetchStrategy(self.url, digest=self.md5) + mirror = join_path( + os.path.dirname(stage.mirror_path), + os.path.basename(self.url) + ) + with spack.stage.Stage(fetcher, mirror_path=mirror) as patch_stage: + patch_stage.fetch() + patch_stage.check() + patch_stage.cache_local() + patch_stage.expand_archive() + self.path = os.path.abspath( + os.listdir(patch_stage.path).pop() + ) + super(UrlPatch, self).apply(stage) class NoSuchPatchFileError(spack.error.SpackError): diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index c0dfbba987..7e6b543799 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -545,6 +545,10 @@ class StageComposite: def archive_file(self): return self[0].archive_file + @property + def mirror_path(self): + return self[0].mirror_path + class DIYStage(object): """Simple class that allows any directory to be a spack stage.""" diff --git a/lib/spack/spack/util/compression.py b/lib/spack/spack/util/compression.py index 982a02d021..806465dc4e 100644 --- a/lib/spack/spack/util/compression.py +++ b/lib/spack/spack/util/compression.py @@ -46,6 +46,9 @@ def decompressor_for(path, extension=None): path.endswith('.zip')): unzip = which('unzip', required=True) return unzip + if extension and re.match(r'gz', extension): + gunzip = which('gunzip', required=True) + return gunzip tar = which('tar', required=True) tar.add_default_arg('-xf') return tar -- cgit v1.2.3-60-g2f50