From 5aeb9e55bcce23ebe9188aa3edc019fb20b9ec30 Mon Sep 17 00:00:00 2001
From: Chris Green <artists@fnal.gov>
Date: Fri, 2 Aug 2019 13:04:23 -0500
Subject: [PATCH] Address @tldalhgren review comments.

---
 lib/spack/docs/packaging_guide.rst | 11 ++++++-----
 lib/spack/spack/fetch_strategy.py  | 16 ++++++++--------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index ce0dbd81215ce3..0b6642ba9599d5 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -882,11 +882,12 @@ Git fetching supports the following parameters to ``version``:
 * ``tag``: Name of a tag to fetch.
 * ``commit``: SHA hash (or prefix) of a commit to fetch.
 * ``submodules``: Also fetch submodules recursively when checking out this repository.
-* ``full_depth``: If the version of git and the specified transport
-  protocol support it, `--depth 1` will be used unless this option is
-  `True`.
-* ``all_branches``: If the version of git supports it, `--single-branch`
-  will be used unless this option is `True`.
+* ``full_depth``: Ensure the full git history is checked out. Normally
+  (`full_depth=False` ), the git option `--depth 1` will be used if the
+  version of git and the specified transport protocol support it.
+* ``all_branches``: Retrieve at least the HEAD commit (see `full_depth`
+  of all branches). Normally (`all_branches=False`), the git option
+  `--single-branch` will be used if the version of git supports it.
 
 Only one of ``tag``, ``branch``, or ``commit`` can be used at a time.
 
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index 2237bd2ded655e..d2764efbed2af7 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -95,8 +95,8 @@ def __init__(self, **kwargs):
         # constructed at package construction time.  This is where things
         # will be fetched.
         self.stage = None
-        # Possibly disable caching for this strategy.
-        self._no_cache_opt = kwargs.pop('no_cache', False)
+        # Enable or disable caching for this strategy?
+        self._cache_enabled = not kwargs.pop('no_cache', False)
 
     def set_stage(self, stage):
         """This is called by Stage before any of the fetching
@@ -104,8 +104,8 @@ def set_stage(self, stage):
         self.stage = stage
 
     @property
-    def no_cache_opt(self):
-        return self._no_cache_opt
+    def cache_enabled(self):
+        return self._cache_enabled
 
     # Subclasses need to implement these methods
     def fetch(self):
@@ -326,7 +326,7 @@ def archive_file(self):
 
     @property
     def cachable(self):
-        return (not self.no_cache_opt) and bool(self.digest)
+        return self._cache_enabled and bool(self.digest)
 
     @_needs_stage
     def expand(self):
@@ -676,7 +676,7 @@ def git(self):
 
     @property
     def cachable(self):
-        return (not self.no_cache_opt) and bool(self.commit or self.tag)
+        return self._cache_enabled and bool(self.commit or self.tag)
 
     def source_id(self):
         return self.commit or self.tag
@@ -845,7 +845,7 @@ def svn(self):
 
     @property
     def cachable(self):
-        return (not self.no_cache_opt) and bool(self.revision)
+        return self._cache_enabled and bool(self.revision)
 
     def source_id(self):
         return self.revision
@@ -953,7 +953,7 @@ def hg(self):
 
     @property
     def cachable(self):
-        return (not self.no_cache_opt) and bool(self.revision)
+        return self._cache_enabled and bool(self.revision)
 
     def source_id(self):
         return self.revision