From 22798d75407189ae639355474f0b97cd4fc8f195 Mon Sep 17 00:00:00 2001 From: psakievich Date: Wed, 20 Jul 2022 14:55:16 -0600 Subject: Don't restage develop specs when a patch fails (#31593) * make develop specs not restage when a patch fails * add a unit test --- lib/spack/spack/package_base.py | 12 +++++++++-- lib/spack/spack/test/patch.py | 46 ++++++++++++++++++++++++++++++++++++++++- 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') -- cgit v1.2.3-60-g2f50