summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorHarmen Stoppels <harmenstoppels@gmail.com>2021-12-15 17:56:03 +0100
committerMassimiliano Culpo <massimiliano.culpo@gmail.com>2021-12-23 16:02:09 +0100
commitb2694013d4f3a564a0d1e4a9b02feaab15a5abef (patch)
tree468f8e42a20ffdb458b537f479ff1e7c5a49c3a4 /lib
parent8f3b025b5502120c99c483b619ae83d2ead07c7f (diff)
downloadspack-b2694013d4f3a564a0d1e4a9b02feaab15a5abef.tar.gz
spack-b2694013d4f3a564a0d1e4a9b02feaab15a5abef.tar.bz2
spack-b2694013d4f3a564a0d1e4a9b02feaab15a5abef.tar.xz
spack-b2694013d4f3a564a0d1e4a9b02feaab15a5abef.zip
Revert "patches: make re-applied patches idempotent (#26784)" (#27625)
This reverts commit c5ca0db27fce5d772dc8a4fcffec3b62bb0bf1f3.
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/patch.py36
-rw-r--r--lib/spack/spack/test/patch.py97
-rw-r--r--lib/spack/spack/util/crypto.py5
-rw-r--r--lib/spack/spack/util/executable.py24
4 files changed, 25 insertions, 137 deletions
diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py
index cdd9ad0387..a273514636 100644
--- a/lib/spack/spack/patch.py
+++ b/lib/spack/spack/patch.py
@@ -7,7 +7,6 @@ import hashlib
import inspect
import os
import os.path
-import sys
import llnl.util.filesystem
import llnl.util.lang
@@ -26,12 +25,6 @@ from spack.util.executable import which
def apply_patch(stage, patch_path, level=1, working_dir='.'):
"""Apply the patch at patch_path to code in the stage.
- Spack runs ``patch`` with ``-N`` so that it does not reject already-applied
- patches. This is useful for develop specs, so that the build does not fail
- due to repeated application of patches, and for easing requirements on patch
- specifications in packages -- packages won't stop working when patches we
- previously had to apply land in upstream.
-
Args:
stage (spack.stage.Stage): stage with code that will be patched
patch_path (str): filesystem location for the patch to apply
@@ -41,31 +34,10 @@ def apply_patch(stage, patch_path, level=1, working_dir='.'):
"""
patch = which("patch", required=True)
with llnl.util.filesystem.working_dir(stage.source_path):
- output = patch(
- '-N', # don't reject already-applied patches
- '-p', str(level), # patch level (directory depth)
- '-i', patch_path, # input source is the patch file
- '-d', working_dir, # patch chdir's to here before patching
- output=str,
- fail_on_error=False,
- )
-
- if patch.returncode != 0:
- # `patch` returns 1 both:
- # a) when an error applying a patch, and
- # b) when -N is supplied and the patch has already been applied
- #
- # It returns > 1 if there's something more serious wrong.
- #
- # So, the best we can do is to look for return code 1, look for output
- # indicating that the patch was already applied, and ignore the error
- # if we see it. Most implementations (BSD and GNU) seem to have the
- # same messages, so we expect these checks to be reliable.
- if patch.returncode > 1 or not any(
- s in output for s in ("Skipping patch", "ignored")
- ):
- sys.stderr.write(output)
- raise patch.error
+ patch('-s',
+ '-p', str(level),
+ '-i', patch_path,
+ '-d', working_dir)
class Patch(object):
diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py
index 5285be8ba9..8383100440 100644
--- a/lib/spack/spack/test/patch.py
+++ b/lib/spack/spack/test/patch.py
@@ -15,9 +15,8 @@ import spack.patch
import spack.paths
import spack.repo
import spack.util.compression
-import spack.util.crypto
from spack.spec import Spec
-from spack.stage import DIYStage, Stage
+from spack.stage import Stage
from spack.util.executable import Executable
# various sha256 sums (using variables for legibility)
@@ -34,43 +33,6 @@ url2_sha256 = '1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd'
url2_archive_sha256 = 'abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd'
-# some simple files for patch tests
-file_to_patch = """\
-first line
-second line
-"""
-
-patch_file = """\
-diff a/foo.txt b/foo-expected.txt
---- a/foo.txt
-+++ b/foo-expected.txt
-@@ -1,2 +1,3 @@
-+zeroth line
- first line
--second line
-+third line
-"""
-
-expected_patch_result = """\
-zeroth line
-first line
-third line
-"""
-
-file_patch_cant_apply_to = """\
-this file
-is completely different
-from anything in the files
-or patch above
-"""
-
-
-def write_file(filename, contents):
- """Helper function for setting up tests."""
- with open(filename, 'w') as f:
- f.write(contents)
-
-
@pytest.fixture()
def mock_patch_stage(tmpdir_factory, monkeypatch):
# Don't disrupt the spack install directory with tests.
@@ -105,9 +67,19 @@ def test_url_patch(mock_patch_stage, filename, sha256, archive_sha256):
mkdirp(stage.source_path)
with working_dir(stage.source_path):
- write_file("foo.txt", file_to_patch)
- write_file("foo-expected.txt", expected_patch_result)
-
+ # write a file to be patched
+ with open('foo.txt', 'w') as f:
+ f.write("""\
+first line
+second line
+""")
+ # write the expected result of patching.
+ with open('foo-expected.txt', 'w') as f:
+ f.write("""\
+zeroth line
+first line
+third line
+""")
# apply the patch and compare files
patch.fetch()
patch.apply(stage)
@@ -117,47 +89,6 @@ def test_url_patch(mock_patch_stage, filename, sha256, archive_sha256):
assert filecmp.cmp('foo.txt', 'foo-expected.txt')
-def test_apply_patch_twice(mock_patch_stage, tmpdir):
- """Ensure that patch doesn't fail if applied twice."""
-
- stage = DIYStage(str(tmpdir))
- with tmpdir.as_cwd():
- write_file("foo.txt", file_to_patch)
- write_file("foo-expected.txt", expected_patch_result)
- write_file("foo.patch", patch_file)
-
- FakePackage = collections.namedtuple(
- 'FakePackage', ['name', 'namespace', 'fullname'])
- fake_pkg = FakePackage('fake-package', 'test', 'fake-package')
-
- def make_patch(filename):
- path = os.path.realpath(str(tmpdir.join(filename)))
- url = 'file://' + path
- sha256 = spack.util.crypto.checksum("sha256", path)
- return spack.patch.UrlPatch(fake_pkg, url, sha256=sha256)
-
- # apply the first time
- patch = make_patch('foo.patch')
- patch.fetch()
-
- patch.apply(stage)
- with working_dir(stage.source_path):
- assert filecmp.cmp('foo.txt', 'foo-expected.txt')
-
- # ensure apply() is idempotent
- patch.apply(stage)
- with working_dir(stage.source_path):
- assert filecmp.cmp('foo.txt', 'foo-expected.txt')
-
- # now write a file that can't be patched
- with working_dir(stage.source_path):
- write_file("foo.txt", file_patch_cant_apply_to)
-
- # this application should fail with a real error
- with pytest.raises(spack.util.executable.ProcessError):
- patch.apply(stage)
-
-
def test_patch_in_spec(mock_packages, config):
"""Test whether patches in a package appear in the spec."""
spec = Spec('patch')
diff --git a/lib/spack/spack/util/crypto.py b/lib/spack/spack/util/crypto.py
index 8e055126fe..549216a4c6 100644
--- a/lib/spack/spack/util/crypto.py
+++ b/lib/spack/spack/util/crypto.py
@@ -92,11 +92,6 @@ def checksum(hashlib_algo, filename, **kwargs):
"""Returns a hex digest of the filename generated using an
algorithm from hashlib.
"""
- if isinstance(hashlib_algo, str):
- if hashlib_algo not in hashes:
- raise ValueError("Invalid hash algorithm: ", hashlib_algo)
- hashlib_algo = hash_fun_for_algo(hashlib_algo)
-
block_size = kwargs.get('block_size', 2**20)
hasher = hashlib_algo()
with open(filename, 'rb') as file:
diff --git a/lib/spack/spack/util/executable.py b/lib/spack/spack/util/executable.py
index ded429c2e6..e615ccdcfd 100644
--- a/lib/spack/spack/util/executable.py
+++ b/lib/spack/spack/util/executable.py
@@ -27,7 +27,6 @@ class Executable(object):
from spack.util.environment import EnvironmentModifications # no cycle
self.default_envmod = EnvironmentModifications()
self.returncode = None
- self.error = None # saved ProcessError when fail_on_error
if not self.exe:
raise ProcessError("Cannot construct executable for '%s'" % name)
@@ -91,8 +90,7 @@ class Executable(object):
the environment (neither requires nor precludes env)
fail_on_error (bool): Raise an exception if the subprocess returns
an error. Default is True. The return code is available as
- ``exe.returncode``, and a saved ``ProcessError`` that would
- have been raised is in ``exe.error``.
+ ``exe.returncode``
ignore_errors (int or list): A list of error codes to ignore.
If these codes are returned, this process will not raise
an exception even if ``fail_on_error`` is set to ``True``
@@ -215,7 +213,7 @@ class Executable(object):
sys.stderr.write(errstr)
rc = self.returncode = proc.returncode
- if rc != 0:
+ if fail_on_error and rc != 0 and (rc not in ignore_errors):
long_msg = cmd_line_string
if result:
# If the output is not captured in the result, it will have
@@ -224,11 +222,8 @@ class Executable(object):
# stdout/stderr (e.g. if 'output' is not specified)
long_msg += '\n' + result
- self.error = ProcessError(
- 'Command exited with status %d:' % proc.returncode, long_msg
- )
- if fail_on_error and (rc not in ignore_errors):
- raise self.error
+ raise ProcessError('Command exited with status %d:' %
+ proc.returncode, long_msg)
return result
@@ -237,15 +232,10 @@ class Executable(object):
'%s: %s' % (self.exe[0], e.strerror), 'Command: ' + cmd_line_string)
except subprocess.CalledProcessError as e:
- self.error = ProcessError(
- str(e),
- '\nExit status %d when invoking command: %s' % (
- proc.returncode,
- cmd_line_string,
- ),
- )
if fail_on_error:
- raise self.error
+ raise ProcessError(
+ str(e), '\nExit status %d when invoking command: %s' %
+ (proc.returncode, cmd_line_string))
finally:
if close_ostream: