Fixes #1520 (#1822)

Some packages which include resources fetched from source control
repositories terminated package installs because they failed to
archive; specifically, this included all SCM resources which identify
a specific state of the repo - for example a revision in svn or a
tag/revision in git. This is because the resource stage creation
logic did not choose an appropriate archive name for these kinds of
resources.

4 files changed, 73 insertions, 44 deletions

diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index 4374587250..d1a817d82f 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -116,6 +116,14 @@ class FetchStrategy(object):
     def archive(self, destination):
         pass  # Used to create tarball for mirror.
 
+    @property
+    def cachable(self):
+        """Return whether the fetcher is capable of caching the
+           resource it retrieves. This generally is determined by
+           whether the resource is identifiably associated with a
+           specific package version."""
+        pass
+
     def __str__(self):  # Should be human readable URL.
         return "FetchStrategy.__str___"
 
@@ -272,6 +280,10 @@ class URLFetchStrategy(FetchStrategy):
         """Path to the source archive within this stage directory."""
         return self.stage.archive_file
 
+    @property
+    def cachable(self):
+        return bool(self.digest)
+
     @_needs_stage
     def expand(self):
         if not self.expand_archive:
@@ -555,6 +567,10 @@ class GitFetchStrategy(VCSFetchStrategy):
 
         return self._git
 
+    @property
+    def cachable(self):
+        return bool(self.commit or self.tag)
+
     @_needs_stage
     def fetch(self):
         self.stage.chdir()
@@ -671,6 +687,10 @@ class SvnFetchStrategy(VCSFetchStrategy):
             self._svn = which('svn', required=True)
         return self._svn
 
+    @property
+    def cachable(self):
+        return bool(self.revision)
+
     @_needs_stage
     def fetch(self):
         self.stage.chdir()
@@ -754,6 +774,10 @@ class HgFetchStrategy(VCSFetchStrategy):
             self._hg = which('hg', required=True)
         return self._hg
 
+    @property
+    def cachable(self):
+        return bool(self.revision)
+
     @_needs_stage
     def fetch(self):
         self.stage.chdir()
@@ -860,22 +884,30 @@ def for_package_version(pkg, version):
     raise InvalidArgsError(pkg, version)
 
 
+def from_list_url(pkg):
+    """If a package provides a URL which lists URLs for resources by
+       version, this can can create a fetcher for a URL discovered for
+       the specified package's version."""
+    if pkg.list_url:
+        try:
+            versions = pkg.fetch_remote_versions()
+            try:
+                url_from_list = versions[pkg.version]
+                return URLFetchStrategy(url=url_from_list, digest=None)
+            except KeyError:
+                tty.msg("Can not find version %s in url_list" %
+                        self.version)
+        except:
+            tty.msg("Could not determine url from list_url.")
+
+
 class FsCache(object):
 
     def __init__(self, root):
         self.root = os.path.abspath(root)
 
     def store(self, fetcher, relativeDst):
-        unique = False
-        uidGroups = [['tag', 'commit'], ['digest'], ['revision']]
-        for grp in uidGroups:
-            try:
-                unique |= any(getattr(fetcher, x) for x in grp)
-            except AttributeError:
-                pass
-            if unique:
-                break
-        if not unique:
+        if not fetcher.cachable:
             return
 
         dst = join_path(self.root, relativeDst)
diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index 0f72e4e25c..97aeef5434 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -44,7 +44,7 @@ from spack.version import *
 from spack.util.compression import allowed_archive
 
 
-def mirror_archive_filename(spec, fetcher):
+def mirror_archive_filename(spec, fetcher, resourceId=None):
     """Get the name of the spec's archive in the mirror."""
     if not spec.version.concrete:
         raise ValueError("mirror.path requires spec with concrete version.")
@@ -67,15 +67,18 @@ def mirror_archive_filename(spec, fetcher):
         # Otherwise we'll make a .tar.gz ourselves
         ext = 'tar.gz'
 
-    filename = "%s-%s" % (spec.package.name, spec.version)
-    if ext:
-        filename += ".%s" % ext
+    if resourceId:
+        filename = "%s-%s" % (resourceId, spec.version) + ".%s" % ext
+    else:
+        filename = "%s-%s" % (spec.package.name, spec.version) + ".%s" % ext
+
     return filename
 
 
-def mirror_archive_path(spec, fetcher):
+def mirror_archive_path(spec, fetcher, resourceId=None):
     """Get the relative path to the spec's archive within a mirror."""
-    return join_path(spec.name, mirror_archive_filename(spec, fetcher))
+    return join_path(
+        spec.name, mirror_archive_filename(spec, fetcher, resourceId))
 
 
 def get_matching_versions(specs, **kwargs):
@@ -204,8 +207,9 @@ def add_single_spec(spec, mirror_root, categories, **kwargs):
                     name = spec.format("$_$@")
                 else:
                     resource = stage.resource
-                    archive_path = join_path(
-                        subdir, suggest_archive_basename(resource))
+                    archive_path = os.path.abspath(join_path(
+                        mirror_root,
+                        mirror_archive_path(spec, fetcher, resource.name)))
                     name = "{resource} ({pkg}).".format(
                         resource=resource.name, pkg=spec.format("$_$@"))
                 subdir = os.path.dirname(archive_path)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 0d3c3c0ad5..df40d34d33 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -687,7 +687,8 @@ class PackageBase(object):
 
     def _make_resource_stage(self, root_stage, fetcher, resource):
         resource_stage_folder = self._resource_stage(resource)
-        resource_mirror = join_path(self.name, os.path.basename(fetcher.url))
+        resource_mirror = spack.mirror.mirror_archive_path(
+            self.spec, fetcher, resource.name)
         stage = ResourceStage(resource.fetcher,
                               root=root_stage,
                               resource=resource,
@@ -702,8 +703,12 @@ class PackageBase(object):
         # Construct a path where the stage should build..
         s = self.spec
         stage_name = "%s-%s-%s" % (s.name, s.version, s.dag_hash())
-        # Build the composite stage
-        stage = Stage(fetcher, mirror_path=mp, name=stage_name, path=self.path)
+
+        # Check list_url alternative archive URLs
+        dynamic_fetcher = fs.from_list_url(self)
+        alternate_fetchers = [dynamic_fetcher] if dynamic_fetcher else None
+        stage = Stage(fetcher, mirror_path=mp, name=stage_name, path=self.path,
+                      alternate_fetchers=alternate_fetchers)
         return stage
 
     def _make_stage(self):
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index ff10a38ca8..8377f9651f 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -161,7 +161,8 @@ class Stage(object):
 
     def __init__(
             self, url_or_fetch_strategy,
-            name=None, mirror_path=None, keep=False, path=None, lock=True):
+            name=None, mirror_path=None, keep=False, path=None, lock=True,
+            alternate_fetchers=None):
         """Create a stage object.
            Parameters:
              url_or_fetch_strategy
@@ -197,6 +198,7 @@ class Stage(object):
         self.fetcher.set_stage(self)
         # self.fetcher can change with mirrors.
         self.default_fetcher = self.fetcher
+        self.alternate_fetchers = alternate_fetchers
         # used for mirrored archives of repositories.
         self.skip_checksum_for_mirror = True
 
@@ -408,28 +410,14 @@ class Stage(object):
                 fetchers.insert(
                     0, fs.URLFetchStrategy(
                         url, digest, expand=expand, extension=extension))
-            fetchers.insert(
-                0, spack.fetch_cache.fetcher(
-                    self.mirror_path, digest, expand=expand,
-                    extension=extension))
-
-            # Look for the archive in list_url
-            package_name = os.path.dirname(self.mirror_path)
-            pkg = spack.repo.get(package_name)
-            if pkg.list_url is not None and pkg.url is not None:
-                try:
-                    archive_version = spack.url.parse_version(
-                        self.default_fetcher.url)
-                    versions = pkg.fetch_remote_versions()
-                    try:
-                        url_from_list = versions[Version(archive_version)]
-                        fetchers.append(fs.URLFetchStrategy(
-                            url_from_list, digest))
-                    except KeyError:
-                        tty.msg("Can not find version %s in url_list" %
-                                archive_version)
-                except:
-                    tty.msg("Could not determine url from list_url.")
+            if self.default_fetcher.cachable:
+                fetchers.insert(
+                    0, spack.fetch_cache.fetcher(
+                        self.mirror_path, digest, expand=expand,
+                        extension=extension))
+
+        if self.alternate_fetchers:
+            fetchers.extend(self.alternate_fetchers)
 
         for fetcher in fetchers:
             try: