spack.relocate: add unit test (#15610)

* relocate: removed import from statements
* relocate: renamed *Exception to *Error

This aims at consistency in naming with both
the standard library (ValueError, AttributeError,
etc.) and other errors in 'spack.error'.

Improved existing docstrings

* relocate: simplified search function by un-nesting conditionals

The search function that searches for patchelf has been
refactored to remove deeply nested conditionals.

Extended docstring.

* relocate: removed a condition specific to unit tests
* relocate: added test for _patchelf
This commit is contained in:
Massimiliano Culpo 2020-03-24 09:02:54 +01:00 committed by GitHub
parent 7e0ec0ec09
commit 1b4de7813a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 130 additions and 67 deletions

View file

@ -2,55 +2,63 @@
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import os
import platform
import re
import shutil
import platform
import spack.repo
import spack.cmd
import llnl.util.lang
from spack.util.executable import Executable, ProcessError
import llnl.util.tty as tty
from macholib.MachO import MachO
from spack.spec import Spec
import macholib.MachO
import macholib.mach_o
import spack.cmd
import spack.repo
import spack.spec
import spack.util.executable as executable
class InstallRootStringException(spack.error.SpackError):
"""
Raised when the relocated binary still has the install root string.
"""
class InstallRootStringError(spack.error.SpackError):
def __init__(self, file_path, root_path):
super(InstallRootStringException, self).__init__(
"""Signal that the relocated binary still has the original
Spack's store root string
Args:
file_path (str): path of the binary
root_path (str): original Spack's store root string
"""
super(InstallRootStringError, self).__init__(
"\n %s \ncontains string\n %s \n"
"after replacing it in rpaths.\n"
"Package should not be relocated.\n Use -a to override." %
(file_path, root_path))
class BinaryStringReplacementException(spack.error.SpackError):
"""
Raised when the size of the file changes after binary path substitution.
"""
class BinaryStringReplacementError(spack.error.SpackError):
def __init__(self, file_path, old_len, new_len):
super(BinaryStringReplacementException, self).__init__(
"""The size of the file changed after binary path substitution
Args:
file_path (str): file with changing size
old_len (str): original length of the file
new_len (str): length of the file after substitution
"""
super(BinaryStringReplacementError, self).__init__(
"Doing a binary string replacement in %s failed.\n"
"The size of the file changed from %s to %s\n"
"when it should have remanined the same." %
(file_path, old_len, new_len))
class BinaryTextReplaceException(spack.error.SpackError):
"""
Raised when the new install path is shorter than the old install path
so binary text replacement cannot occur.
"""
class BinaryTextReplaceError(spack.error.SpackError):
def __init__(self, old_path, new_path):
"""Raised when the new install path is longer than the
old one, so binary text replacement cannot occur.
Args:
old_path (str): original path to be substituted
new_path (str): candidate path for substitution
"""
msg = "New path longer than old path: binary text"
msg += " replacement not possible."
err_msg = "The new path %s" % new_path
@ -58,33 +66,35 @@ def __init__(self, old_path, new_path):
err_msg += "Text replacement in binaries will not work.\n"
err_msg += "Create buildcache from an install path "
err_msg += "longer than new path."
super(BinaryTextReplaceException, self).__init__(msg, err_msg)
super(BinaryTextReplaceError, self).__init__(msg, err_msg)
def get_patchelf():
def _patchelf():
"""Return the full path to the patchelf binary, if available, else None.
Search first the current PATH for patchelf. If not found, try to look
if the default patchelf spec is installed and if not install it.
Return None on Darwin or if patchelf cannot be found.
"""
Returns the full patchelf binary path if available in $PATH.
Builds and installs spack patchelf package on linux platforms
using the first concretized spec if it is not installed and
returns the full patchelf binary path.
"""
# as we may need patchelf, find out where it is
# Check if patchelf is already in the PATH
patchelf = spack.util.executable.which('patchelf')
if patchelf is not None:
return patchelf.path
patchelf_spec = Spec('patchelf').concretized()
patchelf = patchelf_spec.package
if patchelf.installed:
patchelf_executable = os.path.join(patchelf.prefix.bin, "patchelf")
return patchelf_executable
else:
if (str(spack.architecture.platform()) == 'test' or
str(spack.architecture.platform()) == 'darwin'):
return None
else:
patchelf.do_install()
patchelf_executable = os.path.join(patchelf.prefix.bin, "patchelf")
return patchelf_executable
# Check if patchelf spec is installed
spec = spack.spec.Spec('patchelf').concretized()
exe_path = os.path.join(spec.prefix.bin, "patchelf")
if spec.package.installed and os.path.exists(exe_path):
return exe_path
# Skip darwin
if str(spack.architecture.platform()) == 'darwin':
return None
# Install the spec and return its path
spec.package.do_install()
return exe_path if os.path.exists(exe_path) else None
def get_existing_elf_rpaths(path_name):
@ -95,17 +105,17 @@ def get_existing_elf_rpaths(path_name):
# if we're relocating patchelf itself, use it
if path_name[-13:] == "/bin/patchelf":
patchelf = Executable(path_name)
if path_name.endswith("/bin/patchelf"):
patchelf = executable.Executable(path_name)
else:
patchelf = Executable(get_patchelf())
patchelf = executable.Executable(_patchelf())
rpaths = list()
try:
output = patchelf('--print-rpath', '%s' %
path_name, output=str, error=str)
rpaths = output.rstrip('\n').split(':')
except ProcessError as e:
except executable.ProcessError as e:
msg = 'patchelf --print-rpath %s produced an error %s' % (path_name, e)
tty.warn(msg)
return rpaths
@ -266,7 +276,7 @@ def modify_macho_object(cur_path, rpaths, deps, idpath,
# avoid error message for libgcc_s
if 'libgcc_' in cur_path:
return
install_name_tool = Executable('install_name_tool')
install_name_tool = executable.Executable('install_name_tool')
if idpath:
new_idpath = paths_to_paths.get(idpath, None)
@ -295,7 +305,7 @@ def modify_object_macholib(cur_path, paths_to_paths):
dictionary mapping paths in old install layout to new install layout
"""
dll = MachO(cur_path)
dll = macholib.MachO.MachO(cur_path)
changedict = paths_to_paths
@ -324,7 +334,7 @@ def macholib_get_paths(cur_path):
Get rpaths, dependencies and id of mach-o objects
using python macholib package
"""
dll = MachO(cur_path)
dll = macholib.MachO.MachO(cur_path)
ident = None
rpaths = list()
@ -359,14 +369,14 @@ def modify_elf_object(path_name, new_rpaths):
if path_name[-13:] == "/bin/patchelf":
shutil.copy(path_name, bak_path)
patchelf = Executable(bak_path)
patchelf = executable.Executable(bak_path)
else:
patchelf = Executable(get_patchelf())
patchelf = executable.Executable(_patchelf())
try:
patchelf('--force-rpath', '--set-rpath', '%s' % new_joined,
'%s' % path_name, output=str, error=str)
except ProcessError as e:
except executable.ProcessError as e:
msg = 'patchelf --force-rpath --set-rpath %s failed with error %s' % (
path_name, e)
tty.warn(msg)
@ -443,7 +453,7 @@ def replace(match):
return
ndata = pat.sub(replace, data)
if not len(ndata) == original_data_len:
raise BinaryStringReplacementException(
raise BinaryStringReplacementError(
path_name, original_data_len, len(ndata))
f.write(ndata)
f.truncate()
@ -468,7 +478,7 @@ def replace(match):
new_dir.encode('utf-8')) + b'\0' * padding
if len(new_dir) > len(old_dir):
raise BinaryTextReplaceException(old_dir, new_dir)
raise BinaryTextReplaceError(old_dir, new_dir)
with open(path_name, 'rb+') as f:
data = f.read()
@ -479,7 +489,7 @@ def replace(match):
return
ndata = pat.sub(replace, data)
if not len(ndata) == original_data_len:
raise BinaryStringReplacementException(
raise BinaryStringReplacementError(
path_name, original_data_len, len(ndata))
f.write(ndata)
f.truncate()
@ -656,7 +666,7 @@ def check_files_relocatable(cur_path_names, allow_root):
for cur_path in cur_path_names:
if (not allow_root and
not file_is_relocatable(cur_path)):
raise InstallRootStringException(
raise InstallRootStringError(
cur_path, spack.store.layout.root)
@ -725,7 +735,7 @@ def relocate_text_bin(path_names, old_layout_root, new_layout_root,
replace_prefix_bin(path_name, old_spack_prefix, new_spack_prefix)
else:
if len(path_names) > 0:
raise BinaryTextReplaceException(
raise BinaryTextReplaceError(
old_install_prefix, new_install_prefix)
@ -789,7 +799,7 @@ def file_is_relocatable(file, paths_to_relocate=None):
if not os.path.isabs(file):
raise ValueError('{0} is not an absolute path'.format(file))
strings = Executable('strings')
strings = executable.Executable('strings')
# Remove the RPATHS from the strings in the executable
set_of_strings = set(strings(file, output=str).split())
@ -851,7 +861,7 @@ def mime_type(file):
Returns:
Tuple containing the MIME type and subtype
"""
file_cmd = Executable('file')
file_cmd = executable.Executable('file')
output = file_cmd('-b', '-h', '--mime-type', file, output=str, error=str)
tty.debug('[MIME_TYPE] {0} -> {1}'.format(file, output.strip()))
# In corner cases the output does not contain a subtype prefixed with a /

View file

@ -3,15 +3,18 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import collections
import os.path
import platform
import shutil
import pytest
import llnl.util.filesystem
import pytest
import spack.architecture
import spack.concretize
import spack.paths
import spack.relocate
import spack.spec
import spack.store
import spack.tengine
import spack.util.executable
@ -45,6 +48,47 @@ def source_file(tmpdir, is_relocatable):
return src
@pytest.fixture(params=['which_found', 'installed', 'to_be_installed'])
def expected_patchelf_path(request, mutable_database, monkeypatch):
"""Prepare the stage to tests different cases that can occur
when searching for patchelf.
"""
case = request.param
# Mock the which function
which_fn = {
'which_found': lambda x: collections.namedtuple(
'_', ['path']
)('/usr/bin/patchelf')
}
monkeypatch.setattr(
spack.util.executable, 'which',
which_fn.setdefault(case, lambda x: None)
)
if case == 'which_found':
return '/usr/bin/patchelf'
# TODO: Mock a case for Darwin architecture
spec = spack.spec.Spec('patchelf')
spec.concretize()
patchelf_cls = type(spec.package)
do_install = patchelf_cls.do_install
expected_path = os.path.join(spec.prefix.bin, 'patchelf')
def do_install_mock(self, **kwargs):
do_install(self, fake=True)
with open(expected_path):
pass
monkeypatch.setattr(patchelf_cls, 'do_install', do_install_mock)
if case == 'installed':
spec.package.do_install()
return expected_path
@pytest.mark.requires_executables(
'/usr/bin/gcc', 'patchelf', 'strings', 'file'
)
@ -64,7 +108,7 @@ def test_file_is_relocatable(source_file, is_relocatable):
'patchelf', 'strings', 'file'
)
def test_patchelf_is_relocatable():
patchelf = spack.relocate.get_patchelf()
patchelf = spack.relocate._patchelf()
assert llnl.util.filesystem.is_exe(patchelf)
assert spack.relocate.file_is_relocatable(patchelf)
@ -87,3 +131,12 @@ def test_file_is_relocatable_errors(tmpdir):
with pytest.raises(ValueError) as exc_info:
spack.relocate.file_is_relocatable('delete.me')
assert 'is not an absolute path' in str(exc_info.value)
@pytest.mark.skipif(
platform.system().lower() != 'linux',
reason='implementation for MacOS still missing'
)
def test_search_patchelf(expected_patchelf_path):
current = spack.relocate._patchelf()
assert current == expected_patchelf_path