From 508d79c47575da16385a1ec7b0a94abb1838348c Mon Sep 17 00:00:00 2001 From: scheibelp Date: Wed, 5 Oct 2016 22:45:02 -0700 Subject: Handle packages with unparseable extensions (#1758) This closes #1757 which provides an example of a url scheme where the version appears after the extension. Instead of extending the parsing logic to handle this case, this commit allows the user to specify their extension type. This helps Spack choose the appropriate decompressor and mirror archive filename. --- lib/spack/spack/fetch_strategy.py | 4 +++- lib/spack/spack/mirror.py | 9 ++++++++- lib/spack/spack/package.py | 12 ------------ lib/spack/spack/stage.py | 8 ++++++-- lib/spack/spack/url.py | 4 +--- lib/spack/spack/util/compression.py | 5 +++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 21802c4556..4b8829c32f 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -159,6 +159,8 @@ class URLFetchStrategy(FetchStrategy): self.expand_archive = kwargs.get('expand', True) + self.extension = kwargs.get('extension', None) + if not self.url: raise ValueError("URLFetchStrategy requires a url for fetching.") @@ -270,7 +272,7 @@ class URLFetchStrategy(FetchStrategy): "URLFetchStrategy couldn't find archive file", "Failed on expand() for URL %s" % self.url) - decompress = decompressor_for(self.archive_file) + decompress = decompressor_for(self.archive_file, self.extension) # Expand all tarballs in their own directory to contain # exploding tarballs. diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py index 2361fb5448..0f72e4e25c 100644 --- a/lib/spack/spack/mirror.py +++ b/lib/spack/spack/mirror.py @@ -52,7 +52,14 @@ def mirror_archive_filename(spec, fetcher): if isinstance(fetcher, fs.URLFetchStrategy): if fetcher.expand_archive: # If we fetch with a URLFetchStrategy, use URL's archive type - ext = url.downloaded_file_extension(fetcher.url) + ext = url.determine_url_file_extension(fetcher.url) + ext = ext or spec.package.versions[spec.package.version].get( + 'extension', None) + ext = ext.lstrip('.') + if not ext: + raise MirrorError( + "%s version does not specify an extension" % spec.name + + " and could not parse extension from %s" % fetcher.url) else: # If the archive shouldn't be expanded, don't check extension. ext = None diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index a596a363ca..23ae8bd98f 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -53,14 +53,12 @@ import spack.repository import spack.url import spack.util.web -from urlparse import urlparse from StringIO import StringIO from llnl.util.filesystem import * from llnl.util.lang import * from llnl.util.link_tree import LinkTree from llnl.util.tty.log import log_output from spack.stage import Stage, ResourceStage, StageComposite -from spack.util.compression import allowed_archive from spack.util.environment import dump_environment from spack.util.executable import ProcessError, which from spack.version import * @@ -1473,16 +1471,6 @@ def flatten_dependencies(spec, flat_dir): dep_files.merge(flat_dir + '/' + name) -def validate_package_url(url_string): - """Determine whether spack can handle a particular URL or not.""" - url = urlparse(url_string) - if url.scheme not in _ALLOWED_URL_SCHEMES: - tty.die("Invalid protocol in URL: '%s'" % url_string) - - if not allowed_archive(url_string): - tty.die("Invalid file type in URL: '%s'" % url_string) - - def dump_packages(spec, path): """Dump all package information for a spec and its dependencies. diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index b659cfb2fb..01e069ded1 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -302,9 +302,11 @@ class Stage(object): # the checksum will be the same. digest = None expand = True + extension = None if isinstance(self.default_fetcher, fs.URLFetchStrategy): digest = self.default_fetcher.digest expand = self.default_fetcher.expand_archive + extension = self.default_fetcher.extension # Have to skip the checksum for things archived from # repositories. How can this be made safer? @@ -313,10 +315,12 @@ class Stage(object): # Add URL strategies for all the mirrors with the digest for url in urls: fetchers.insert( - 0, fs.URLFetchStrategy(url, digest, expand=expand)) + 0, fs.URLFetchStrategy( + url, digest, expand=expand, extension=extension)) fetchers.insert( 0, spack.fetch_cache.fetcher( - self.mirror_path, digest, expand=expand)) + self.mirror_path, digest, expand=expand, + extension=extension)) # Look for the archive in list_url package_name = os.path.dirname(self.mirror_path) diff --git a/lib/spack/spack/url.py b/lib/spack/spack/url.py index 02c9c04380..af4b8a51ef 100644 --- a/lib/spack/spack/url.py +++ b/lib/spack/spack/url.py @@ -142,7 +142,7 @@ def split_url_extension(path): return prefix, ext, suffix -def downloaded_file_extension(path): +def determine_url_file_extension(path): """This returns the type of archive a URL refers to. This is sometimes confusing because of URLs like: @@ -160,8 +160,6 @@ def downloaded_file_extension(path): return 'tar.gz' prefix, ext, suffix = split_url_extension(path) - if not ext: - raise UrlParseError("Cannot deduce archive type in %s" % path, path) return ext diff --git a/lib/spack/spack/util/compression.py b/lib/spack/spack/util/compression.py index 64554ab2f7..982a02d021 100644 --- a/lib/spack/spack/util/compression.py +++ b/lib/spack/spack/util/compression.py @@ -40,9 +40,10 @@ def allowed_archive(path): return any(path.endswith(t) for t in ALLOWED_ARCHIVE_TYPES) -def decompressor_for(path): +def decompressor_for(path, extension=None): """Get the appropriate decompressor for a path.""" - if path.endswith(".zip"): + if ((extension and re.match(r'\.?zip$', extension)) or + path.endswith('.zip')): unzip = which('unzip', required=True) return unzip tar = which('tar', required=True) -- cgit v1.2.3-60-g2f50