Don't restage develop specs when a patch fails (#31593)

* make develop specs not restage when a patch fails
* add a unit test

2 files changed, 55 insertions, 3 deletions

diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index ae1998e241..95aaa6319a 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -1556,8 +1556,16 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
         # If we encounter an archive that failed to patch, restage it
         # so that we can apply all the patches again.
         if os.path.isfile(bad_file):
-            tty.debug('Patching failed last time. Restaging.')
-            self.stage.restage()
+            if self.stage.managed_by_spack:
+                tty.debug('Patching failed last time. Restaging.')
+                self.stage.restage()
+            else:
+                # develop specs/ DIYStages may have patch failures but
+                # should never be restaged
+                msg = ('A patch failure was detected in %s.' % self.name +
+                       ' Build errors may occur due to this.')
+                tty.warn(msg)
+                return
 
         # If this file exists, then we already applied all the patches.
         if os.path.isfile(good_file):
diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py
index 596811eb99..33b49f2506 100644
--- a/lib/spack/spack/test/patch.py
+++ b/lib/spack/spack/test/patch.py
@@ -10,7 +10,7 @@ import sys
 
 import pytest
 
-from llnl.util.filesystem import mkdirp, working_dir
+from llnl.util.filesystem import mkdirp, touch, working_dir
 
 import spack.patch
 import spack.paths
@@ -213,6 +213,50 @@ def test_patched_dependency(
                 assert 'Patched!' in mf.read()
 
 
+def trigger_bad_patch(pkg):
+    if not os.path.isdir(pkg.stage.source_path):
+        os.makedirs(pkg.stage.source_path)
+    bad_file = os.path.join(pkg.stage.source_path, '.spack_patch_failed')
+    touch(bad_file)
+    return bad_file
+
+
+def test_patch_failure_develop_spec_exits_gracefully(
+        mock_packages, config, install_mockery, mock_fetch, tmpdir):
+    """
+    ensure that a failing patch does not trigger exceptions
+    for develop specs
+    """
+
+    spec = Spec('patch-a-dependency '
+                '^libelf dev_path=%s' % str(tmpdir))
+    spec.concretize()
+    libelf = spec['libelf']
+    assert 'patches' in list(libelf.variants.keys())
+    pkg = libelf.package
+    with pkg.stage:
+        bad_patch_indicator = trigger_bad_patch(pkg)
+        assert os.path.isfile(bad_patch_indicator)
+        pkg.do_patch()
+    # success if no exceptions raised
+
+
+def test_patch_failure_restages(
+        mock_packages, config, install_mockery, mock_fetch):
+    """
+    ensure that a failing patch does not trigger exceptions
+    for non-develop specs and the source gets restaged
+    """
+    spec = Spec('patch-a-dependency')
+    spec.concretize()
+    pkg = spec['libelf'].package
+    with pkg.stage:
+        bad_patch_indicator = trigger_bad_patch(pkg)
+        assert os.path.isfile(bad_patch_indicator)
+        pkg.do_patch()
+        assert not os.path.isfile(bad_patch_indicator)
+
+
 def test_multiple_patched_dependencies(mock_packages, config):
     """Test whether multiple patched dependencies work."""
     spec = Spec('patch-several-dependencies')