diff options
| -rw-r--r-- | lib/spack/spack/patch.py | 36 | ||||
| -rw-r--r-- | lib/spack/spack/test/patch.py | 97 | ||||
| -rw-r--r-- | lib/spack/spack/util/crypto.py | 5 | ||||
| -rw-r--r-- | lib/spack/spack/util/executable.py | 24 |
4 files changed, 137 insertions, 25 deletions
diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py index a273514636..cdd9ad0387 100644 --- a/lib/spack/spack/patch.py +++ b/lib/spack/spack/patch.py @@ -7,6 +7,7 @@ import hashlib import inspect import os import os.path +import sys import llnl.util.filesystem import llnl.util.lang @@ -25,6 +26,12 @@ 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 @@ -34,10 +41,31 @@ def apply_patch(stage, patch_path, level=1, working_dir='.'): """ patch = which("patch", required=True) with llnl.util.filesystem.working_dir(stage.source_path): - patch('-s', - '-p', str(level), - '-i', patch_path, - '-d', working_dir) + 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 class Patch(object): diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py index 8383100440..5285be8ba9 100644 --- a/lib/spack/spack/test/patch.py +++ b/lib/spack/spack/test/patch.py @@ -15,8 +15,9 @@ 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 Stage +from spack.stage import DIYStage, Stage from spack.util.executable import Executable # various sha256 sums (using variables for legibility) @@ -33,6 +34,43 @@ 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. @@ -67,19 +105,9 @@ def test_url_patch(mock_patch_stage, filename, sha256, archive_sha256): mkdirp(stage.source_path) with working_dir(stage.source_path): - # 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 -""") + write_file("foo.txt", file_to_patch) + write_file("foo-expected.txt", expected_patch_result) + # apply the patch and compare files patch.fetch() patch.apply(stage) @@ -89,6 +117,47 @@ third line 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 549216a4c6..8e055126fe 100644 --- a/lib/spack/spack/util/crypto.py +++ b/lib/spack/spack/util/crypto.py @@ -92,6 +92,11 @@ 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 e615ccdcfd..ded429c2e6 100644 --- a/lib/spack/spack/util/executable.py +++ b/lib/spack/spack/util/executable.py @@ -27,6 +27,7 @@ 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) @@ -90,7 +91,8 @@ 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`` + ``exe.returncode``, and a saved ``ProcessError`` that would + have been raised is in ``exe.error``. 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`` @@ -213,7 +215,7 @@ class Executable(object): sys.stderr.write(errstr) rc = self.returncode = proc.returncode - if fail_on_error and rc != 0 and (rc not in ignore_errors): + if rc != 0: long_msg = cmd_line_string if result: # If the output is not captured in the result, it will have @@ -222,8 +224,11 @@ class Executable(object): # stdout/stderr (e.g. if 'output' is not specified) long_msg += '\n' + result - raise ProcessError('Command exited with status %d:' % - proc.returncode, long_msg) + 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 return result @@ -232,10 +237,15 @@ 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 ProcessError( - str(e), '\nExit status %d when invoking command: %s' % - (proc.returncode, cmd_line_string)) + raise self.error finally: if close_ostream: |