Procedure to deprecate old versions of software (#20767)

* Procedure to deprecate old versions of software

* Add documentation

* Fix bug in logic

* Update tab completion

* Deprecate legacy packages

* Deprecate old mxnet as well

* More explicit docs

12 files changed, 124 insertions, 11 deletions

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index c989a48f92..321d636b8f 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -612,6 +612,62 @@ it executable, then runs it with some arguments.
        installer = Executable(self.stage.archive_file)
        installer('--prefix=%s' % prefix, 'arg1', 'arg2', 'etc.')
 
+
+^^^^^^^^^^^^^^^^^^^^^^^^
+Deprecating old versions
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+There are many reasons to remove old versions of software:
+
+#. Security vulnerabilities (most serious reason)
+#. Changing build systems that increase package complexity
+#. Changing dependencies/patches/resources/flags that increase package complexity
+#. Maintainer/developer inability/unwillingness to support old versions
+#. No longer available for download (right to be forgotten)
+#. Package or version rename
+
+At the same time, there are many reasons to keep old versions of software:
+
+#. Reproducibility
+#. Requirements for older packages (e.g. some packages still rely on Qt 3)
+
+In general, you should not remove old versions from a ``package.py``. Instead,
+you should first deprecate them using the following syntax:
+
+.. code-block:: python
+
+   version('1.2.3', sha256='...', deprecated=True)
+
+
+This has two effects. First, ``spack info`` will no longer advertise that
+version. Second, commands like ``spack install`` that fetch the package will
+require user approval:
+
+.. code-block:: console
+
+   $ spack install openssl@1.0.1e
+   ==> Warning: openssl@1.0.1e is deprecated and may be removed in a future Spack release.
+   ==>   Fetch anyway? [y/N]
+
+
+If you use ``spack install --deprecated``, this check can be skipped.
+
+This also applies to package recipes that are renamed or removed. You should
+first deprecate all versions before removing a package. If you need to rename
+it, you can deprecate the old package and create a new package at the same
+time.
+
+Version deprecations should always last at least one Spack minor release cycle
+before the version is completely removed. For example, if a version is
+deprecated in Spack 0.16.0, it should not be removed until Spack 0.17.0. No
+version should be removed without such a deprecation process. This gives users
+a chance to complain about the deprecation in case the old version is needed
+for some application. If you require a deprecated version of a package, simply
+submit a PR to remove ``deprecated=True`` from the package. However, you may be
+asked to help maintain this version of the package if the current maintainers
+are unwilling to support this older version.
+
+
 ^^^^^^^^^^^^^^^^
 Download caching
 ^^^^^^^^^^^^^^^^
diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py
index f83708a7dd..e6a0edd599 100644
--- a/lib/spack/spack/cmd/common/arguments.py
+++ b/lib/spack/spack/cmd/common/arguments.py
@@ -277,6 +277,13 @@ def no_checksum():
         help="do not use checksums to verify downloaded files (unsafe)")
 
 
+@arg
+def deprecated():
+    return Args(
+        '--deprecated', action='store_true', default=False,
+        help='fetch deprecated versions without warning')
+
+
 def add_cdash_args(subparser, add_help):
     cdash_help = {}
     if add_help:
diff --git a/lib/spack/spack/cmd/dev_build.py b/lib/spack/spack/cmd/dev_build.py
index a681e2c2d7..2e7c51da42 100644
--- a/lib/spack/spack/cmd/dev_build.py
+++ b/lib/spack/spack/cmd/dev_build.py
@@ -26,7 +26,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-i', '--ignore-dependencies', action='store_true', dest='ignore_deps',
         help="don't try to install dependencies of requested packages")
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'deprecated'])
     subparser.add_argument(
         '--keep-prefix', action='store_true',
         help="do not remove the install prefix if installation fails")
@@ -98,6 +98,9 @@ def dev_build(self, args):
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
+    if args.deprecated:
+        spack.config.set('config:deprecated', True, scope='command_line')
+
     tests = False
     if args.test == 'all':
         tests = True
diff --git a/lib/spack/spack/cmd/fetch.py b/lib/spack/spack/cmd/fetch.py
index 4f3e0ad594..962bcefce6 100644
--- a/lib/spack/spack/cmd/fetch.py
+++ b/lib/spack/spack/cmd/fetch.py
@@ -16,7 +16,7 @@ level = "long"
 
 
 def setup_parser(subparser):
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'deprecated'])
     subparser.add_argument(
         '-m', '--missing', action='store_true',
         help="fetch only missing (not yet installed) dependencies")
@@ -33,6 +33,9 @@ def fetch(parser, args):
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
+    if args.deprecated:
+        spack.config.set('config:deprecated', True, scope='command_line')
+
     specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         if args.missing or args.dependencies:
diff --git a/lib/spack/spack/cmd/info.py b/lib/spack/spack/cmd/info.py
index 4762150c77..6b316f3cc4 100644
--- a/lib/spack/spack/cmd/info.py
+++ b/lib/spack/spack/cmd/info.py
@@ -189,10 +189,11 @@ def print_text_info(pkg):
         color.cprint(section_title('Safe versions:  '))
 
         for v in reversed(sorted(pkg.versions)):
-            if pkg.has_code:
-                url = fs.for_package_version(pkg, v)
-            line = version('    {0}'.format(pad(v))) + color.cescape(url)
-            color.cprint(line)
+            if not pkg.versions[v].get('deprecated', False):
+                if pkg.has_code:
+                    url = fs.for_package_version(pkg, v)
+                line = version('    {0}'.format(pad(v))) + color.cescape(url)
+                color.cprint(line)
 
     color.cprint('')
     color.cprint(section_title('Variants:'))
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index eb7dc93f98..5e572f82f6 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -126,7 +126,7 @@ remote spec matches that of the local spec""")
     subparser.add_argument(
         '--source', action='store_true', dest='install_source',
         help="install source files in prefix")
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'deprecated'])
     subparser.add_argument(
         '-v', '--verbose', action='store_true',
         help="display verbose build output while installing")
@@ -288,6 +288,9 @@ environment variables:
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
+    if args.deprecated:
+        spack.config.set('config:deprecated', True, scope='command_line')
+
     # Parse cli arguments and construct a dictionary
     # that will be passed to the package installer
     update_kwargs_from_args(args, kwargs)
diff --git a/lib/spack/spack/cmd/mirror.py b/lib/spack/spack/cmd/mirror.py
index 4552cce400..2f0611a2c4 100644
--- a/lib/spack/spack/cmd/mirror.py
+++ b/lib/spack/spack/cmd/mirror.py
@@ -28,7 +28,7 @@ level = "long"
 
 
 def setup_parser(subparser):
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'deprecated'])
 
     sp = subparser.add_subparsers(
         metavar='SUBCOMMAND', dest='mirror_command')
@@ -371,4 +371,7 @@ def mirror(parser, args):
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
+    if args.deprecated:
+        spack.config.set('config:deprecated', True, scope='command_line')
+
     action[args.mirror_command](args)
diff --git a/lib/spack/spack/cmd/patch.py b/lib/spack/spack/cmd/patch.py
index e85541a535..14739b8d7d 100644
--- a/lib/spack/spack/cmd/patch.py
+++ b/lib/spack/spack/cmd/patch.py
@@ -16,7 +16,8 @@ level = "long"
 
 
 def setup_parser(subparser):
-    arguments.add_common_arguments(subparser, ['no_checksum', 'specs'])
+    arguments.add_common_arguments(
+        subparser, ['no_checksum', 'deprecated', 'specs'])
 
 
 def patch(parser, args):
@@ -26,6 +27,9 @@ def patch(parser, args):
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
+    if args.deprecated:
+        spack.config.set('config:deprecated', True, scope='command_line')
+
     specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
diff --git a/lib/spack/spack/cmd/stage.py b/lib/spack/spack/cmd/stage.py
index d6832b2f09..62073f10bd 100644
--- a/lib/spack/spack/cmd/stage.py
+++ b/lib/spack/spack/cmd/stage.py
@@ -16,7 +16,8 @@ level = "long"
 
 
 def setup_parser(subparser):
-    arguments.add_common_arguments(subparser, ['no_checksum', 'specs'])
+    arguments.add_common_arguments(
+        subparser, ['no_checksum', 'deprecated', 'specs'])
     subparser.add_argument(
         '-p', '--path', dest='path',
         help="path to stage package, does not add to spack tree")
@@ -37,6 +38,9 @@ def stage(parser, args):
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
+    if args.deprecated:
+        spack.config.set('config:deprecated', True, scope='command_line')
+
     specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index 360448b851..30a45f1b06 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -278,6 +278,9 @@ def version(ver, checksum=None, **kwargs):
 
     The ``dict`` of arguments is turned into a valid fetch strategy for
     code packages later. See ``spack.fetch_strategy.for_package_version()``.
+
+    Keyword Arguments:
+        deprecated (bool): whether or not this version is deprecated
     """
     def _execute_version(pkg):
         if checksum is not None:
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 3e19264e24..0c09410c6d 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1309,7 +1309,6 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
             tty.debug('No fetch required for {0}: package has no code.'
                       .format(self.name))
 
-        start_time = time.time()
         checksum = spack.config.get('config:checksum')
         fetch = self.stage.managed_by_spack
         if checksum and fetch and self.version not in self.versions:
@@ -1331,8 +1330,34 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
                 raise FetchError("Will not fetch %s" %
                                  self.spec.format('{name}{@version}'), ck_msg)
 
+        deprecated = spack.config.get('config:deprecated')
+        if not deprecated and self.versions.get(
+                self.version, {}).get('deprecated', False):
+            tty.warn("{0} is deprecated and may be removed in a future Spack "
+                     "release.".format(
+                         self.spec.format('{name}{@version}')))
+
+            # Ask the user whether to install deprecated version if we're
+            # interactive, but just fail if non-interactive.
+            dp_msg = ("If you are willing to be a maintainer for this version "
+                      "of the package, submit a PR to remove `deprecated=False"
+                      "`, or use `--deprecated` to skip this check.")
+            ignore_deprecation = False
+            if sys.stdout.isatty():
+                ignore_deprecation = tty.get_yes_or_no("  Fetch anyway?",
+                                                       default=False)
+
+                if ignore_deprecation:
+                    tty.debug("Fetching deprecated version. {0}".format(
+                        dp_msg))
+
+            if not ignore_deprecation:
+                raise FetchError("Will not fetch {0}".format(
+                    self.spec.format('{name}{@version}')), dp_msg)
+
         self.stage.create()
         err_msg = None if not self.manual_download else self.download_instr
+        start_time = time.time()
         self.stage.fetch(mirror_only, err_msg=err_msg)
         self._fetch_time = time.time() - start_time
 
diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py
index 3a97d79675..f0669600e1 100644
--- a/lib/spack/spack/schema/config.py
+++ b/lib/spack/spack/schema/config.py
@@ -78,6 +78,7 @@ properties = {
             'install_missing_compilers': {'type': 'boolean'},
             'debug': {'type': 'boolean'},
             'checksum': {'type': 'boolean'},
+            'deprecated': {'type': 'boolean'},
             'locks': {'type': 'boolean'},
             'dirty': {'type': 'boolean'},
             'build_language': {'type': 'string'},