@tgamblin this particular PR should be rebased (vs. squashed): I have formatted all the commit messages.

this particular PR should be rebased (vs. squashed): I have formatted all the commit messages.

Question related to commit messages: can you add a handle to this PR in each commit if this is not squashed? Asking because it's quite useful when you read code to annotate it and eventually read the PRs associated with the lines of interest.

@scheibelp I don't know if the other PRs fix the long timeout issue, but I tweaked this branch to get a better behavior. I added --connect-timeout to the curl arguments. It only exits when an initial connection cannot be established within a 15 second limit.

diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index 409cc334e..a6956b7d4 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -324,6 +324,7 @@ def fetch(self):
             '-D',
             '-',  # print out HTML headers
             '-L',  # resolve 3xx redirects
+            '--connect-timeout', '15',
             self.url,
         ]
 
diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index ceb53801c..3a863188a 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -499,7 +499,7 @@ def add_single_spec(spec, mirror_root, mirror_stats):
     tty.msg("Adding package {pkg} to mirror".format(
         pkg=spec.format("{name}{@version}")
     ))
-    num_retries = 3
+    num_retries = 1
     while num_retries > 0:
         try:
             with spec.package.stage as pkg_stage:

@sknigh See #13881 for a PR that adds the same flag.

(EDIT 12/12: some of this is incorrect and has been corrected by #13908 (comment))

This is failing because as part of addressing #13908 (comment) I invoke os.remove on the symlink which actually deletes the file that it points to. Oddly os.unlink appears to fail in this way also, so I'm not sure if there is a way to remove the symlink properly.

I can either:

• hack the ability to delete symlinks for existing files into Spack (e.g. by temporarily moving the symlink target to a temporary directory)
• examine the link but never delete it (and inform the user of the issue)

The latter does not seem very reasonable to me so I'll be thinking about how to do the first one.

@scheibelp I think os.unlink does what you want on linux (remove the link without deleting what it points to) but:

1. The documentation contradicts the current behavior (it says it has the same semantic a os.remove which is not always true)
2. The behavior might be OS/platform specific

Another alternative might be to return early if the link os.exists:

if os.path.exists(cosmetic_path):
    return

which means it's not broken (so basically rule out every exception to the happy code path at the beginning of the method).

Thanks for the suggestion!

I think os.unlink does what you want on linux

I encountered this problem on Mac OS so that might be why it comes up for me

Another alternative might be to return early if the link os.exists:

I agree that checking both os.path.lexists and os.path.exists should resolve the issue for all platforms. I'll be able to fix that this evening.

#13908 (comment) worked for me but to clarify:

I asserted in #13908 (comment) that os.unlink was deleting the target of a symlink. That's not true: it turns out some tests were creating a file vs. a symlink at cosmetic_path, and os.unlink will delete the provided path if it refers to a file (vs. a symlink), which was causing the error.

So at this point all requests are addressed.