From fb3a3ba95e741ba55485affae3ca4ef4a876d69e Mon Sep 17 00:00:00 2001
From: Paul Ferrell <51765748+Paul-Ferrell@users.noreply.github.com>
Date: Wed, 20 Nov 2019 17:00:44 -0700
Subject: mirror bug fixes: symlinks, duplicate patch names, and exception
 handling (#13789)

* Some packages (e.g. mpfr at the time of this patch) can have patches
  with the same name but different contents (which apply to different
  versions of the package). This appends part of the patch hash to the
  cache file name to avoid conflicts.
* Some exceptions which occur during fetching are not a subclass of
  SpackError and therefore do not have a 'message' attribute. This
  updates the logic for mirroring a single spec (add_single_spec)
  to produce an appropriate error message in that case (where before
  it failed with an AttributeError)
* In various circumstances, a mirror can contain the universal storage
  path but not a cosmetic symlink; in this case it would not generate
  a symlink. Now "spack mirror create" will create a symlink for any
  package that doesn't have one.
---
 lib/spack/spack/caches.py | 17 ++++++++++-------
 lib/spack/spack/mirror.py |  2 +-
 lib/spack/spack/patch.py  | 10 +++++++---
 lib/spack/spack/stage.py  | 12 ++++++------
 4 files changed, 24 insertions(+), 17 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/caches.py b/lib/spack/spack/caches.py
index bdece50421..a7910905bc 100644
--- a/lib/spack/spack/caches.py
+++ b/lib/spack/spack/caches.py
@@ -53,20 +53,23 @@ class MirrorCache(object):
     def __init__(self, root):
         self.root = os.path.abspath(root)
 
-    def store(self, fetcher, relative_dest, cosmetic_path=None):
+    def store(self, fetcher, relative_dest):
+        """Fetch and relocate the fetcher's target into our mirror cache."""
+
         # Note this will archive package sources even if they would not
         # normally be cached (e.g. the current tip of an hg/git branch)
         dst = os.path.join(self.root, relative_dest)
         mkdirp(os.path.dirname(dst))
         fetcher.archive(dst)
 
-        # Add a symlink path that a human can read to understand what resource
-        # the archive path refers to
-        if not cosmetic_path:
-            return
-        cosmetic_path = os.path.join(self.root, cosmetic_path)
+    def symlink(self, mirror_ref):
+        """Symlink a human readible path in our mirror to the actual
+        storage location."""
+
+        cosmetic_path = os.path.join(self.root, mirror_ref.cosmetic_path)
         relative_dst = os.path.relpath(
-            dst, start=os.path.dirname(cosmetic_path))
+            mirror_ref.storage_path,
+            start=os.path.dirname(cosmetic_path))
         if not os.path.exists(cosmetic_path):
             mkdirp(os.path.dirname(cosmetic_path))
             os.symlink(relative_dst, cosmetic_path)
diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index bae250b97b..8575b4bb71 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -522,7 +522,7 @@ def add_single_spec(spec, mirror_root, mirror_stats):
         else:
             tty.warn(
                 "Error while fetching %s" % spec.cformat('{name}{@version}'),
-                exception.message)
+                getattr(exception, 'message', exception))
         mirror_stats.error()
 
 
diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py
index 79550538db..ba8d4c5ef8 100644
--- a/lib/spack/spack/patch.py
+++ b/lib/spack/spack/patch.py
@@ -199,11 +199,15 @@ class UrlPatch(Patch):
         fetcher = fs.URLFetchStrategy(self.url, fetch_digest,
                                       expand=bool(self.archive_sha256))
 
-        per_package_ref = os.path.join(
-            self.owner.split('.')[-1], os.path.basename(self.url))
+        # The same package can have multiple patches with the same name but
+        # with different contents, therefore apply a subset of the hash.
+        name = '{0}-{1}'.format(os.path.basename(self.url), fetch_digest[:7])
+
+        per_package_ref = os.path.join(self.owner.split('.')[-1], name)
         # Reference starting with "spack." is required to avoid cyclic imports
         mirror_ref = spack.mirror.mirror_archive_paths(
-            fetcher, per_package_ref)
+            fetcher,
+            per_package_ref)
 
         self.stage = spack.stage.Stage(fetcher, mirror_paths=mirror_ref)
         self.stage.create()
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 164ed91b2f..d730fd63b4 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -500,13 +500,13 @@ class Stage(object):
 
         if os.path.exists(absolute_storage_path):
             stats.already_existed(absolute_storage_path)
-            return
+        else:
+            self.fetch()
+            spack.caches.mirror_cache.store(
+                self.fetcher, self.mirror_paths.storage_path)
+            stats.added(absolute_storage_path)
 
-        self.fetch()
-        spack.caches.mirror_cache.store(
-            self.fetcher, self.mirror_paths.storage_path,
-            self.mirror_paths.cosmetic_path)
-        stats.added(absolute_storage_path)
+        spack.caches.mirror_cache.symlink(self.mirror_paths)
 
     def expand_archive(self):
         """Changes to the stage directory and attempt to expand the downloaded
-- 
cgit v1.2.3-70-g09d2