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-70-g09d2 From 52158d9316aabd9c02a719825928607cb5204377 Mon Sep 17 00:00:00 2001 From: "Adam J. Stewart" Date: Fri, 21 Oct 2016 09:49:36 -0500 Subject: Add new Version property to handle joined version numbers (#2062) * Add new version property to handle joined version numbers * Add unit test for new joined property * Add documentation on version.up_to() and version.joined --- lib/spack/docs/packaging_guide.rst | 59 +++++++++++++++------- lib/spack/spack/test/versions.py | 1 + lib/spack/spack/version.py | 4 ++ var/spack/repos/builtin/packages/cdd/package.py | 8 +-- .../repos/builtin/packages/cfitsio/package.py | 4 +- .../repos/builtin/packages/cryptopp/package.py | 9 ++-- var/spack/repos/builtin/packages/lrslib/package.py | 8 +-- var/spack/repos/builtin/packages/nag/package.py | 6 +-- var/spack/repos/builtin/packages/nauty/package.py | 8 +-- 9 files changed, 68 insertions(+), 39 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index efd4c459a2..bf5763f4f8 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -526,32 +526,57 @@ in the package. For example, Spack is smart enough to download version ``8.2.1.`` of the ``Foo`` package above from ``http://example.com/foo-8.2.1.tar.gz``. -If spack *cannot* extrapolate the URL from the ``url`` field by -default, you can write your own URL generation algorithm in place of -the ``url`` declaration. For example: +If the URL is particularly complicated or changes based on the release, +you can override the default URL generation algorithm by defining your +own ``url_for_version()`` function. For example, the developers of HDF5 +keep changing the archive layout, so the ``url_for_version()`` function +looks like: + +.. literalinclude:: ../../../var/spack/repos/builtin/packages/hdf5/package.py + :pyobject: Hdf5.url_for_version + +With the use of this ``url_for_version()``, Spack knows to download HDF5 ``1.8.16`` +from ``http://www.hdfgroup.org/ftp/HDF5/releases/hdf5-1.8.16/src/hdf5-1.8.16.tar.gz`` +but download HDF5 ``1.10.0`` from ``http://www.hdfgroup.org/ftp/HDF5/releases/hdf5-1.10/hdf5-1.10.0/src/hdf5-1.10.0.tar.gz``. + +You'll notice that HDF5's ``url_for_version()`` function makes use of a special +``Version`` function called ``up_to()``. When you call ``version.up_to(2)`` on a +version like ``1.10.0``, it returns ``1.10``. ``version.up_to(1)`` would return +``1``. This can be very useful for packages that place all ``X.Y.*`` versions in +a single directory and then places all ``X.Y.Z`` versions in a subdirectory. + +There are a few ``Version`` properties you should be aware of. We generally +prefer numeric versions to be separated by dots for uniformity, but not all +tarballs are named that way. For example, ``icu4c`` separates its major and minor +versions with underscores, like ``icu4c-57_1-src.tgz``. The value ``57_1`` can be +obtained with the use of the ``version.underscored`` property. Note that Python +properties don't need parentheses. There are other separator properties as well: + +=================== ====== +Property Result +=================== ====== +version.dotted 1.2.3 +version.dashed 1-2-3 +version.underscored 1_2_3 +version.joined 123 +=================== ====== -.. code-block:: python - :linenos: +.. note:: - class Foo(Package): - version('8.2.1', '4136d7b4c04df68b686570afa26988ac') - ... - def url_for_version(self, version): - return 'http://example.com/version_%s/foo-%s.tar.gz' \ - % (version, version) - ... + Python properties don't need parentheses. ``version.dashed`` is correct. + ``version.dashed()`` is incorrect. -If a URL cannot be derived systematically, you can add an explicit URL -for a particular version: +If a URL cannot be derived systematically, or there is a special URL for one +of its versions, you can add an explicit URL for a particular version: .. code-block:: python version('8.2.1', '4136d7b4c04df68b686570afa26988ac', url='http://example.com/foo-8.2.1-special-version.tar.gz') -For the URL above, you might have to add an explicit URL because the -version can't simply be substituted in the original ``url`` to -construct the new one for ``8.2.1``. +This is common for Python packages that download from PyPi. Since newer +download URLs often contain a unique hash for each version, there is no +way to guess the URL systematically. When you supply a custom URL for a version, Spack uses that URL *verbatim* and does not perform extrapolation. diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py index 9b4dc29f35..c1d427783c 100644 --- a/lib/spack/spack/test/versions.py +++ b/lib/spack/spack/test/versions.py @@ -392,6 +392,7 @@ class VersionsTest(unittest.TestCase): self.assertEqual(v.dotted, '1.2.3') self.assertEqual(v.dashed, '1-2-3') self.assertEqual(v.underscored, '1_2_3') + self.assertEqual(v.joined, '123') def test_repr_and_str(self): diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py index 67a22f4660..0d68a709e8 100644 --- a/lib/spack/spack/version.py +++ b/lib/spack/spack/version.py @@ -146,6 +146,10 @@ class Version(object): def dashed(self): return '-'.join(str(x) for x in self.version) + @property + def joined(self): + return ''.join(str(x) for x in self.version) + def up_to(self, index): """Return a version string up to the specified component, exclusive. e.g., if this is 10.8.2, self.up_to(2) will return '10.8'. diff --git a/var/spack/repos/builtin/packages/cdd/package.py b/var/spack/repos/builtin/packages/cdd/package.py index bff942af25..4a0a0aefef 100644 --- a/var/spack/repos/builtin/packages/cdd/package.py +++ b/var/spack/repos/builtin/packages/cdd/package.py @@ -35,16 +35,16 @@ class Cdd(Package): homepage = "https://www.inf.ethz.ch/personal/fukudak/cdd_home/cdd.html" url = "ftp://ftp.ifor.math.ethz.ch/pub/fukuda/cdd/cdd-061a.tar.gz" - def url_for_version(self, version): - return ("ftp://ftp.ifor.math.ethz.ch/pub/fukuda/cdd/cdd-%s.tar.gz" % - str(version.dotted).replace('.', '')) - version('0.61a', '22c24a7a9349dd7ec0e24531925a02d9') depends_on("libtool", type="build") patch("Makefile.spack.patch") + def url_for_version(self, version): + url = "ftp://ftp.ifor.math.ethz.ch/pub/fukuda/cdd/cdd-{0}.tar.gz" + return url.format(version.joined) + def install(self, spec, prefix): # The Makefile isn't portable; use our own instead makeargs = ["-f", "Makefile.spack", "PREFIX=%s" % prefix] diff --git a/var/spack/repos/builtin/packages/cfitsio/package.py b/var/spack/repos/builtin/packages/cfitsio/package.py index 35d9662f6f..79af31ae21 100644 --- a/var/spack/repos/builtin/packages/cfitsio/package.py +++ b/var/spack/repos/builtin/packages/cfitsio/package.py @@ -34,9 +34,9 @@ class Cfitsio(Package): version('3.370', 'abebd2d02ba5b0503c633581e3bfa116') - def url_for_version(self, v): + def url_for_version(self, version): url = 'ftp://heasarc.gsfc.nasa.gov/software/fitsio/c/cfitsio{0}.tar.gz' - return url.format(str(v).replace('.', '')) + return url.format(version.joined) def install(self, spec, prefix): configure('--prefix=' + prefix) diff --git a/var/spack/repos/builtin/packages/cryptopp/package.py b/var/spack/repos/builtin/packages/cryptopp/package.py index e9294a14a6..c92f262a9a 100644 --- a/var/spack/repos/builtin/packages/cryptopp/package.py +++ b/var/spack/repos/builtin/packages/cryptopp/package.py @@ -36,12 +36,15 @@ class Cryptopp(Package): """ homepage = "http://www.cryptopp.com" - base_url = "http://www.cryptopp.com" version('5.6.3', '3c5b70e2ec98b7a24988734446242d07') version('5.6.2', '7ed022585698df48e65ce9218f6c6a67') version('5.6.1', '96cbeba0907562b077e26bcffb483828') + def url_for_version(self, version): + url = "{0}/{1}{2}.zip" + return url.format(self.homepage, self.name, version.joined) + def install(self, spec, prefix): make() @@ -51,7 +54,3 @@ class Cryptopp(Package): mkdirp(prefix.lib) install('libcryptopp.a', prefix.lib) - - def url_for_version(self, version): - version_string = str(version).replace('.', '') - return '%s/cryptopp%s.zip' % (Cryptopp.base_url, version_string) diff --git a/var/spack/repos/builtin/packages/lrslib/package.py b/var/spack/repos/builtin/packages/lrslib/package.py index 68a907ea59..3825867bb6 100644 --- a/var/spack/repos/builtin/packages/lrslib/package.py +++ b/var/spack/repos/builtin/packages/lrslib/package.py @@ -33,10 +33,6 @@ class Lrslib(Package): homepage = "http://cgm.cs.mcgill.ca/~avis/C/lrs.html" url = "http://cgm.cs.mcgill.ca/~avis/C/lrslib/archive/lrslib-062.tar.gz" - def url_for_version(self, version): - return ("http://cgm.cs.mcgill.ca/~avis/C/lrslib/archive/lrslib-%s.tar.gz" % - ('0' + str(version).replace('.', ''))) - version('6.2', 'be5da7b3b90cc2be628dcade90c5d1b9') version('6.1', '0b3687c8693cd7d1f234a3f65e147551') version('6.0', 'd600a2e62969ad03f7ab2f85f1b3709c') @@ -51,6 +47,10 @@ class Lrslib(Package): patch("Makefile.spack.patch") + def url_for_version(self, version): + url = "http://cgm.cs.mcgill.ca/~avis/C/lrslib/archive/lrslib-0{0}.tar.gz" + return url.format(version.joined) + def install(self, spec, prefix): # The Makefile isn't portable; use our own instead makeargs = ["-f", "Makefile.spack", diff --git a/var/spack/repos/builtin/packages/nag/package.py b/var/spack/repos/builtin/packages/nag/package.py index 792e3fe3c7..66cb2a6a54 100644 --- a/var/spack/repos/builtin/packages/nag/package.py +++ b/var/spack/repos/builtin/packages/nag/package.py @@ -30,7 +30,7 @@ class Nag(Package): """The NAG Fortran Compiler.""" homepage = "http://www.nag.com/nagware/np.asp" - version('6.1', '1e29d9d435b7ccc2842a320150b28ba4') + version('6.1', 'f49bd548e0d5e2458b2dabb3ee01341a') version('6.0', '3fa1e7f7b51ef8a23e6c687cdcad9f96') # Licensing @@ -43,8 +43,8 @@ class Nag(Package): def url_for_version(self, version): # TODO: url and checksum are architecture dependent # TODO: We currently only support x86_64 - return 'http://www.nag.com/downloads/impl/npl6a%sna_amd64.tgz' % str( - version).replace('.', '') + url = 'http://www.nag.com/downloads/impl/npl6a{0}na_amd64.tgz' + return url.format(version.joined) def install(self, spec, prefix): # Set installation directories diff --git a/var/spack/repos/builtin/packages/nauty/package.py b/var/spack/repos/builtin/packages/nauty/package.py index 167edfe896..0d5eed251b 100644 --- a/var/spack/repos/builtin/packages/nauty/package.py +++ b/var/spack/repos/builtin/packages/nauty/package.py @@ -33,14 +33,14 @@ class Nauty(Package): homepage = "http://pallini.di.uniroma1.it/index.html" url = "http://pallini.di.uniroma1.it/nauty26r7.tar.gz" - def url_for_version(self, version): - return ("http://pallini.di.uniroma1.it/nauty%s.tar.gz" % - str(version).replace('.', '')) - version('2.6r7', 'b2b18e03ea7698db3fbe06c5d76ad8fe') version('2.6r5', '91b03a7b069962e94fc9aac8831ce8d2') version('2.5r9', 'e8ecd08b0892a1fb13329c147f08de6d') + def url_for_version(self, version): + url = "http://pallini.di.uniroma1.it/nauty{0}.tar.gz" + return url.format(version.joined) + def install(self, spec, prefix): configure('--prefix=%s' % prefix) make() -- cgit v1.2.3-70-g09d2 From 73b46a92bcf0a415d5b4a3b29ca47df0d0544076 Mon Sep 17 00:00:00 2001 From: Matthew LeGendre Date: Fri, 21 Oct 2016 11:59:41 -0700 Subject: Fix concretize bug where provider sort couldn't handle version ranges --- lib/spack/spack/concretize.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 9c9e9e10ff..3da5efc9fa 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -95,7 +95,11 @@ class DefaultConcretizer(object): not b.external and b.external_module): # We're choosing between different providers, so # maintain order from provider sort - return candidates.index(a) - candidates.index(b) + index_of_a = next(i for i in range(0, len(candidates)) \ + if a.satisfies(candidates[i])) + index_of_b = next(i for i in range(0, len(candidates)) \ + if b.satisfies(candidates[i])) + return index_of_a - index_of_b result = cmp_specs(a, b) if result != 0: -- cgit v1.2.3-70-g09d2 From 5ff08386af70818e48d81c1d49c0c6e373763c16 Mon Sep 17 00:00:00 2001 From: Matthew LeGendre Date: Fri, 21 Oct 2016 13:17:23 -0700 Subject: Remove unnecessary blackslash for flake8 --- lib/spack/spack/concretize.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 3da5efc9fa..2351e2bfc9 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -95,9 +95,9 @@ class DefaultConcretizer(object): not b.external and b.external_module): # We're choosing between different providers, so # maintain order from provider sort - index_of_a = next(i for i in range(0, len(candidates)) \ + index_of_a = next(i for i in range(0, len(candidates)) if a.satisfies(candidates[i])) - index_of_b = next(i for i in range(0, len(candidates)) \ + index_of_b = next(i for i in range(0, len(candidates)) if b.satisfies(candidates[i])) return index_of_a - index_of_b -- cgit v1.2.3-70-g09d2 From 859d296105cbb51b6ae47baa64e4aaf7e8d0af6c Mon Sep 17 00:00:00 2001 From: Matthew LeGendre Date: Fri, 21 Oct 2016 16:25:12 -0700 Subject: Don't clear LD_LIBRARY_PATH and friends from compiler wrappers (#2074) * Don't clear LD_LIBRARY_PATH and friends from compiler wrappers * remove debugging print --- lib/spack/env/cc | 7 ------- 1 file changed, 7 deletions(-) (limited to 'lib') diff --git a/lib/spack/env/cc b/lib/spack/env/cc index 60e24979c8..84a17abf20 100755 --- a/lib/spack/env/cc +++ b/lib/spack/env/cc @@ -317,13 +317,6 @@ case "$mode" in args=("${args[@]}" ${SPACK_LDLIBS[@]}) ;; esac -# -# Unset pesky environment variables that could affect build sanity. -# -unset LD_LIBRARY_PATH -unset LD_RUN_PATH -unset DYLD_LIBRARY_PATH - full_command=("$command" "${args[@]}") # In test command mode, write out full command for Spack tests. -- cgit v1.2.3-70-g09d2