Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some trivial fixes and Xcode avoidance (preferring our tools if found, in build prefix or base env) also warn about Apple's stub exes #3808

Merged
merged 9 commits into from
Nov 14, 2019
18 changes: 16 additions & 2 deletions conda_build/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def regex_files_rg(files, prefix, tag, rg, regex_rg, replacement_re,
pu = prefix.encode('utf-8')
prefix_files = [os.path.join(pu, f.replace('/', os.sep).encode('utf-8')) for f in files]
args_len = len(b' '.join(args_base))
file_lists = list(chunks(prefix_files, 131071 - args_len))
file_lists = list(chunks(prefix_files, (32760 if utils.on_win else 131071) - args_len))
for file_list in file_lists:
args = args_base[:] + file_list
# This will not work now our args are binary strings:
Expand Down Expand Up @@ -907,7 +907,10 @@ def get_files_with_prefix(m, files_in, prefix):
debug=m.config.debug)
perform_replacements(all_matches, prefix)
end = time.time()
print("INFO :: Time taken to do replacements (prefix pkg-config, CMake, qmake) was: {}".format(end - start))
total_replacements = sum(map(lambda i: len(i['submatches']), all_matches))
print("INFO :: Time taken to mark (prefix) and mark+peform (pkg-config, CMake, qmake)\n"
" {} replacements in {} files was {} seconds".format(
total_replacements, len(all_matches), end - start))
'''
# Keeping this around just for a while.
files_with_prefix2 = sorted(have_prefix_files(files_in, prefix))
Expand Down Expand Up @@ -1408,6 +1411,11 @@ def bundle_conda(output, metadata, env, stats, **kw):
if not interpreter_and_args[0]:
log.error("Did not find an interpreter to run {}, looked for {}".format(
output['script'], interpreter_and_args[0]))
if 'system32' in interpreter_and_args[0] and 'bash' in interpreter_and_args[0]:
print("ERROR :: WSL bash.exe detected, this will not work (PRs welcome!). Please\n"
" use MSYS2 packages. Add `m2-base` and more (depending on what your"
" script needs) to `requirements/build` instead.")
sys.exit(1)
else:
interpreter_and_args = interpreter.split(' ')

Expand All @@ -1417,6 +1425,7 @@ def bundle_conda(output, metadata, env, stats, **kw):
env_output['TOP_PKG_VERSION'] = env['PKG_VERSION']
env_output['PKG_VERSION'] = metadata.version()
env_output['PKG_NAME'] = metadata.get_value('package/name')
env_output['RECIPE_DIR'] = metadata.path
env_output['MSYS2_PATH_TYPE'] = 'inherit'
env_output['CHERE_INVOKING'] = '1'
for var in utils.ensure_list(metadata.get_value('build/script_env')):
Expand Down Expand Up @@ -2054,6 +2063,8 @@ def build(m, stats, post=None, need_source_download=True, need_reparse_in_env=Fa
if not (m.is_output or
(os.path.isdir(m.config.host_prefix) and
len(os.listdir(m.config.host_prefix)) <= 1)):
# This log message contradicts both the not (m.is_output or ..) check above
# and also the comment "For more than one output, ..."
log.debug('Not creating new env for output - already exists from top-level')
else:
m.config._merge_build_host = m.build_is_host
Expand Down Expand Up @@ -2105,6 +2116,9 @@ def build(m, stats, post=None, need_source_download=True, need_reparse_in_env=Fa
if f.startswith('conda-meta'):
to_remove.add(f)

# This is wrong, files has not been expanded at this time and could contain
# wildcards. Also well, I just do not understand this, because when this
# does contain wildcards, the files in to_remove will slip back in.
if 'files' in output_d:
output_d['files'] = set(output_d['files']) - to_remove

Expand Down
2 changes: 1 addition & 1 deletion conda_build/inspect_pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def inspect_objects(packages, prefix=sys.prefix, groupby='package'):
path = join(prefix, f)
filetype = codefile_type(path)
if filetype == 'machofile':
f_info['filetype'] = human_filetype(path)
f_info['filetype'] = human_filetype(path, None)
f_info['rpath'] = ':'.join(get_rpaths(path))
f_info['filename'] = f
info.append(f_info)
Expand Down
2 changes: 1 addition & 1 deletion conda_build/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ def _str_version(package_meta):
# conda-docs/docs/source/build.rst
FIELDS = {
'package': {'name', 'version'},
'source': {'fn', 'url', 'md5', 'sha1', 'sha256', 'path', 'path_via_symlinks',
'source': {'fn', 'url', 'md5', 'sha1', 'sha256', 'path', 'path_via_symlink',
'git_url', 'git_tag', 'git_branch', 'git_rev', 'git_depth',
'hg_url', 'hg_tag',
'svn_url', 'svn_rev', 'svn_ignore_externals',
Expand Down
4 changes: 3 additions & 1 deletion conda_build/os_utils/external.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,7 @@ def find_executable(executable, prefix=None):
def find_preferably_prefixed_executable(executable, build_prefix=None):
found = find_executable('*' + executable, build_prefix)
if not found:
found = find_executable(executable, build_prefix)
# It is possible to force non-prefixed exes by passing os.sep as the
# first character in executable. basename makes this work.
found = find_executable(os.path.basename(executable), build_prefix)
return found
31 changes: 25 additions & 6 deletions conda_build/os_utils/macho.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ def is_macho(path):
return bool(head in MAGIC)


def is_dylib(path):
def is_dylib(path, build_prefix):
return human_filetype(path) == 'DYLIB'


def human_filetype(path):
ot = find_preferably_prefixed_executable('otool')
output = check_output((ot, '-h', path)).decode('utf-8')
def human_filetype(path, build_prefix):
otool = find_apple_cctools_executable('otool', build_prefix)
output = check_output((otool, '-h', path)).decode('utf-8')
lines = output.splitlines()
if not lines[0].startswith((path, 'Mach header')):
raise ValueError(
Expand Down Expand Up @@ -146,6 +146,24 @@ def _get_matching_load_commands(lines, cb_filter):
return result


def find_apple_cctools_executable(name, build_prefix, nofail=False):
tool = find_preferably_prefixed_executable(name, build_prefix)
try:
if '/usr/bin' in tool:
with open(tool, 'rb') as f:
s = f.read()
if s.find(b'usr/lib/libxcselect.dylib') != -1:
print("WARNING :: Found `{}` but is is an Apple Xcode stub executable.".format(tool))
# This is not the real tool, but Apple's irritating GUI dialog launcher.
raise
except Exception as _: # noqa
print("ERROR :: Failed to run `{}`. Please use `conda` to install `cctools` into your base environment.\n"
" An alternative option for users of macOS is to install `Xcode` or `Command Line Tools for Xcode`."
.format(tool))
sys.exit(1)
return tool


def otool(path, build_prefix=None, cb_filter=is_dylib_info):
"""A wrapper around otool -l

Expand All @@ -163,7 +181,8 @@ def otool(path, build_prefix=None, cb_filter=is_dylib_info):
Any key values that can be converted to integers are converted
to integers, the rest are strings.
"""
lines = check_output([find_preferably_prefixed_executable('otool', build_prefix), '-l', path],
otool = find_apple_cctools_executable('otool', build_prefix)
lines = check_output([otool, '-l', path],
stderr=STDOUT).decode('utf-8')
# llvm-objdump returns 0 for some things that are anything but successful completion.
lines_split = lines.splitlines()
Expand Down Expand Up @@ -205,7 +224,7 @@ def _chmod(filename, mode):


def install_name_tool(args, build_prefix=None, verbose=False):
args_full = [find_preferably_prefixed_executable('install_name_tool', build_prefix)]
args_full = [find_apple_cctools_executable('install_name_tool', build_prefix)]
args_full.extend(args)
if verbose:
print(' '.join(args_full))
Expand Down
69 changes: 42 additions & 27 deletions conda_build/post.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,9 @@ def osx_ch_link(path, link_dict, host_prefix, build_prefix, files):

def mk_relative_osx(path, host_prefix, build_prefix, files, rpaths=('lib',)):
assert sys.platform == 'darwin'

names = macho.otool(path, build_prefix)
s = macho.install_name_change(path, build_prefix,
prefix = build_prefix if os.path.exists(build_prefix) else host_prefix
names = macho.otool(path, prefix)
s = macho.install_name_change(path, prefix,
partial(osx_ch_link,
host_prefix=host_prefix,
build_prefix=build_prefix,
Expand All @@ -415,7 +415,7 @@ def mk_relative_osx(path, host_prefix, build_prefix, files, rpaths=('lib',)):
rpath_new = os.path.join('@loader_path',
os.path.relpath(os.path.join(host_prefix, rpath), os.path.dirname(path)),
'').replace('/./', '/')
macho.add_rpath(path, rpath_new, build_prefix=build_prefix, verbose=True)
macho.add_rpath(path, rpath_new, build_prefix=prefix, verbose=True)
if s:
# Skip for stub files, which have to use binary_has_prefix_files to be
# made relocatable.
Expand Down Expand Up @@ -479,37 +479,52 @@ def check_binary_patchers(elf, prefix, rpath):
'''


def mk_relative_linux(f, prefix, rpaths=('lib',), method='LIEF'):
def mk_relative_linux(f, prefix, rpaths=('lib',), method=None):
'Respects the original values and converts abs to $ORIGIN-relative'

elf = os.path.join(prefix, f)
origin = os.path.dirname(elf)

existing_pe = None
patchelf = external.find_executable('patchelf', prefix)
try:
existing = check_output([patchelf, '--print-rpath', elf]).decode('utf-8').splitlines()[0]
except CalledProcessError:
print('patchelf: --print-rpath failed for %s\n' % (elf))
return
if not patchelf:
print("ERROR :: You should install patchelf, will proceed with LIEF for {} (was {})".format(elf, method))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it actually an error? If not, we should change ERROR to WARNING

Suggested change
print("ERROR :: You should install patchelf, will proceed with LIEF for {} (was {})".format(elf, method))
print("WARNING :: You should install patchelf, will proceed with LIEF for {} (was {})".format(elf, method))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if you explicitly ask for patchelf (rather than leaving it as default/None), which you'd do because LIEF is known to be failing, then I think we probably should error out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. My main point is: if you show a message and continue, that's a warning. If you show a warning and stop, that's an error.

method = 'LIEF'
else:
try:
existing_pe = check_output([patchelf, '--print-rpath', elf]).decode('utf-8').splitlines()[0]
except CalledProcessError:
if method == 'patchelf':
print("ERROR :: `patchelf --print-rpath` failed for {}, but patchelf was specified".format(
elf))
elif method != 'LIEF':
print("WARNING :: `patchelf --print-rpath` failed for {}, will proceed with LIEF (was {})".format(
elf, method))
method = 'LIEF'
else:
existing_pe = existing_pe.split(os.pathsep)
existing = existing_pe
if have_lief:
existing2, _, _ = get_rpaths_raw(elf)
if [existing] != existing2:
print('ERROR :: get_rpaths_raw()={} and patchelf={} disagree for {} :: '.format(
existing2, [existing], elf))
existing = existing.split(os.pathsep)
if existing_pe and [existing_pe] != existing2:
print('WARNING :: get_rpaths_raw()={} and patchelf={} disagree for {} :: '.format(
existing2, [existing_pe], elf))
# Use LIEF if method is LIEF to get the initial value?
if method == 'LIEF':
existing = existing2
new = []
for old in existing:
if old.startswith('$ORIGIN'):
new.append(old)
elif old.startswith('/'):
# Test if this absolute path is outside of prefix. That is fatal.
relpath = os.path.relpath(old, prefix)
if relpath.startswith('..' + os.sep):
rp = os.path.relpath(old, prefix)
if rp.startswith('..' + os.sep):
print('Warning: rpath {0} is outside prefix {1} (removing it)'.format(old, prefix))
else:
relpath = '$ORIGIN/' + os.path.relpath(old, origin)
if relpath not in new:
new.append(relpath)
rp = '$ORIGIN/' + os.path.relpath(old, origin)
if rp not in new:
new.append(rp)
# Ensure that the asked-for paths are also in new.
for rpath in rpaths:
if rpath != '':
Expand All @@ -528,15 +543,15 @@ def mk_relative_linux(f, prefix, rpaths=('lib',), method='LIEF'):
rpath = ':'.join(new)

# check_binary_patchers(elf, prefix, rpath)

if method == 'LIEF' or not patchelf:
if not method or not patchelf or method.upper() == 'LIEF':
set_rpath(old_matching='*', new_rpath=rpath, file=elf)
else:
call([patchelf, '--force-rpath', '--set-rpath', rpath, elf])


def assert_relative_osx(path, host_prefix, build_prefix):
for name in macho.get_dylibs(path, build_prefix):
tools_prefix = build_prefix if os.path.exists(build_prefix) else host_prefix
for name in macho.get_dylibs(path, tools_prefix):
for prefix in (host_prefix, build_prefix):
if prefix and name.startswith(prefix):
raise RuntimeError("library at %s appears to have an absolute path embedded" % path)
Expand Down Expand Up @@ -1096,14 +1111,14 @@ def post_process_shared_lib(m, f, files, host_prefix=None):
host_prefix = m.config.host_prefix
path = os.path.join(host_prefix, f)
codefile_t = codefile_type(path)
if not codefile_t:
if not codefile_t or path.endswith('.debug'):
return
rpaths = m.get_value('build/rpaths', ['lib'])
if sys.platform.startswith('linux') and codefile_t == 'elffile':
if codefile_t == 'elffile':
mk_relative_linux(f, m.config.host_prefix, rpaths=rpaths,
method=m.get_value('build/rpaths_patcher', 'patchelf'))
elif sys.platform == 'darwin' and codefile_t == 'machofile':
mk_relative_osx(path, m.config.host_prefix, m.config.build_prefix, files=files, rpaths=rpaths)
method=m.get_value('build/rpaths_patcher', None))
elif codefile_t == 'machofile':
mk_relative_osx(path, host_prefix, m.config.build_prefix, files=files, rpaths=rpaths)


def fix_permissions(files, prefix):
Expand Down