From 2c2392318567ef69b2988a89405b198eaead2b65 Mon Sep 17 00:00:00 2001 From: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 27 Jun 2018 01:26:34 -0700 Subject: [PATCH] refactor XCodeCLITools's method of finding files --- .../backend/native/config/environment.py | 37 ++++- .../native/subsystems/binaries/llvm.py | 6 +- .../native/subsystems/native_toolchain.py | 3 + .../native/subsystems/xcode_cli_tools.py | 150 ++++++++---------- .../pants/backend/python/tasks/setup_py.py | 28 ++-- 5 files changed, 126 insertions(+), 98 deletions(-) diff --git a/src/python/pants/backend/native/config/environment.py b/src/python/pants/backend/native/config/environment.py index a858c72445d5..96351700e9f2 100644 --- a/src/python/pants/backend/native/config/environment.py +++ b/src/python/pants/backend/native/config/environment.py @@ -11,7 +11,7 @@ from pants.engine.rules import SingletonRule from pants.util.objects import datatype from pants.util.osutil import all_normalized_os_names, get_normalized_os_name -from pants.util.strutil import create_path_env_var +from pants.util.strutil import create_path_env_var, safe_shlex_join class Platform(datatype(['normalized_os_name'])): @@ -78,18 +78,30 @@ class Assembler(datatype([ pass +# FIXME: rename this to SharedLibLinker or something! class Linker(datatype([ 'path_entries', 'exe_filename', 'library_dirs', ]), Executable): + # FIXME(???): we need a way to compose executables hygienically -- this will work because we use + # safe shlex methods, but we should really be composing each datatype's members, and only + # creating an environment at the very end. This could be done declaratively -- something like: + # { 'LIBRARY_PATH': DelimitedPathDirectoryEnvVar(...) }. + # We could also just use safe_shlex_join() and create_path_env_var() and keep all the state in the + # environment -- but then we have to remember to use those each time we specialize. def get_invocation_environment_dict(self, platform): ret = super(Linker, self).get_invocation_environment_dict(platform).copy() + all_ldflags_for_platform = platform.resolve_platform_specific({ + 'darwin': lambda: ['-mmacosx-version-min=10.11', '-Wl,-dylib'], + 'linux': lambda: ['-shared'], + }) ret.update({ 'LDSHARED': self.exe_filename, 'LIBRARY_PATH': create_path_env_var(self.library_dirs), + 'LDFLAGS': safe_shlex_join(all_ldflags_for_platform), }) return ret @@ -109,6 +121,12 @@ def get_invocation_environment_dict(self, platform): if self.include_dirs: ret['CPATH'] = create_path_env_var(self.include_dirs) + all_cflags_for_platform = platform.resolve_platform_specific({ + 'darwin': lambda: ['-mmacosx-version-min=10.11'], + 'linux': lambda: [], + }) + ret['CFLAGS'] = safe_shlex_join(all_cflags_for_platform) + return ret @@ -118,8 +136,13 @@ class CCompiler(datatype([ 'library_dirs', 'include_dirs', ]), CompilerMixin): - # TODO: set CC in this datatype's env dict, and CXX in CppCompiler! - pass + + def get_invocation_environment_dict(self, platform): + ret = super(CCompiler, self).get_invocation_environment_dict(platform).copy() + + ret['CC'] = self.exe_filename + + return ret class CppCompiler(datatype([ @@ -128,7 +151,13 @@ class CppCompiler(datatype([ 'library_dirs', 'include_dirs', ]), CompilerMixin): - pass + + def get_invocation_environment_dict(self, platform): + ret = super(CppCompiler, self).get_invocation_environment_dict(platform).copy() + + ret['CXX'] = self.exe_filename + + return ret # FIXME: make this an @rule, after we can automatically produce LibcDev (see #5788). diff --git a/src/python/pants/backend/native/subsystems/binaries/llvm.py b/src/python/pants/backend/native/subsystems/binaries/llvm.py index d27ceefdcf83..d21bbddebda9 100644 --- a/src/python/pants/backend/native/subsystems/binaries/llvm.py +++ b/src/python/pants/backend/native/subsystems/binaries/llvm.py @@ -100,7 +100,8 @@ def c_compiler(self, platform): return CCompiler( path_entries=path_entries, exe_filename=exe_filename, - library_dirs=lib_search_dirs) + library_dirs=lib_search_dirs, + include_dirs=[]) def cpp_compiler(self, platform): exe_filename = 'clang++' @@ -113,7 +114,8 @@ def cpp_compiler(self, platform): return CppCompiler( path_entries=path_entries, exe_filename=exe_filename, - library_dirs=lib_search_dirs) + library_dirs=lib_search_dirs, + include_dirs=[]) @rule(Linker, [Select(Platform), Select(LLVM)]) diff --git a/src/python/pants/backend/native/subsystems/native_toolchain.py b/src/python/pants/backend/native/subsystems/native_toolchain.py index 71f5ddb0e4af..66ac8cdf7edb 100644 --- a/src/python/pants/backend/native/subsystems/native_toolchain.py +++ b/src/python/pants/backend/native/subsystems/native_toolchain.py @@ -90,6 +90,7 @@ def select_linker(platform, native_toolchain): # or a linker (e.g. when we compile native code from `python_dist()`s), use the environment from # the linker object (in addition to any further customizations), which has the paths from the C # and C++ compilers baked in. + # FIXME(???): we need a way to compose executables hygienically. linker = Linker( path_entries=( c_compiler.path_entries + @@ -107,6 +108,8 @@ def select_linker(platform, native_toolchain): @rule(CCompiler, [Select(Platform), Select(NativeToolchain)]) def select_c_compiler(platform, native_toolchain): + # FIXME(???): we should be using our clang, not the provided one, in all cases, and just using + # XCodeCLITools to get the lib and include dirs we need from the host. if platform.normalized_os_name == 'darwin': c_compiler = yield Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools) else: diff --git a/src/python/pants/backend/native/subsystems/xcode_cli_tools.py b/src/python/pants/backend/native/subsystems/xcode_cli_tools.py index 765f1a80452c..8a01870d6b63 100644 --- a/src/python/pants/backend/native/subsystems/xcode_cli_tools.py +++ b/src/python/pants/backend/native/subsystems/xcode_cli_tools.py @@ -10,9 +10,7 @@ from pants.backend.native.config.environment import Assembler, CCompiler, CppCompiler, Linker from pants.engine.rules import RootRule, rule from pants.engine.selectors import Select -from pants.option.custom_types import dir_option from pants.subsystem.subsystem import Subsystem -from pants.util.dirutil import is_executable from pants.util.memo import memoized_method, memoized_property @@ -26,7 +24,7 @@ class XCodeCLITools(Subsystem): options_scope = 'xcode-cli-tools' - REQUIRED_FILES_DEFAULT = { + REQUIRED_FILES = { 'bin': [ 'as', 'cc', @@ -36,19 +34,23 @@ class XCodeCLITools(Subsystem): 'ld', 'lipo', ], - 'include': ['_stdio.h'], + 'include': [ + '_stdio.h', + ], 'lib': [], } - _OSX_BASE_PREFIX = '/usr' - _XCODE_TOOLCHAIN_BASE = '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr' - - # This comes from running clang -###. - INCLUDE_SEARCH_DIRS_DEFAULT = [ - os.path.join(_OSX_BASE_PREFIX, 'local/include'), - os.path.join(_XCODE_TOOLCHAIN_BASE, 'lib/clang/9.1.0/include'), - os.path.join(_XCODE_TOOLCHAIN_BASE, 'include'), - os.path.join(_OSX_BASE_PREFIX, 'include'), + INSTALL_PREFIXES_DEFAULT = [ + # Prefer files from this installation directory, if available. This doesn't appear to be + # populated with e.g. header files on travis. + '/usr', + # Populated by the XCode CLI tools. + '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr', + # Populated by the XCode app. + # TODO: ???/where do all these directories come from? + '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr', + '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.1.0', + '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr', ] class XCodeToolsUnavailable(Exception): @@ -61,102 +63,90 @@ class XCodeToolsInvalid(Exception): def register_options(cls, register): super(XCodeCLITools, cls).register_options(register) - register('--install-prefix', type=dir_option, default=cls._OSX_BASE_PREFIX, advanced=True, - help='Location where the XCode command-line developer tools have been installed. ' - 'Under this directory should be at least {} subdirectories.' - .format(cls.REQUIRED_FILES_DEFAULT.keys())) - - register('--required-files', type=dict, default=cls.REQUIRED_FILES_DEFAULT, advanced=True, - help='Files that should exist within the XCode CLI tools installation.') + register('--install-prefixes', type=list, default=cls.INSTALL_PREFIXES_DEFAULT, advanced=True, + help='Locations to search for resources from the XCode CLI tools, including a ' + 'compiler, linker, header files, and some libraries. ' + 'Under this directory should be some selection of these subdirectories: {}.' + .format(cls.REQUIRED_FILES.keys())) - register('--include-search-dirs', type=list, default=cls.INCLUDE_SEARCH_DIRS_DEFAULT, - advanced=True, - help='Directories to search, in order, for files in #include directives.') - - # TODO: Obtaining options values from a subsystem should be made ergonomic as we move to complete - # #5788. @memoized_property - def _install_prefix(self): - return self.get_options().install_prefix + def _all_existing_install_prefixes(self): + return [pfx for pfx in self.get_options().install_prefixes if os.path.isdir(pfx)] - @memoized_property - def _required_files(self): - return self.get_options().required_files + # NB: We use @memoized_method in this file for methods which may raise. + @memoized_method + def _get_existing_subdirs(self, subdir_name): + maybe_subdirs = [os.path.join(pfx, subdir_name) for pfx in self._all_existing_install_prefixes] + existing_dirs = [existing_dir for existing_dir in maybe_subdirs if os.path.isdir(existing_dir)] + + required_files_for_dir = self.REQUIRED_FILES.get(subdir_name) + if required_files_for_dir: + for fname in required_files_for_dir: + found = False + for subdir in existing_dirs: + full_path = os.path.join(subdir, fname) + if os.path.isfile(full_path): + found = True + continue + + if not found: + raise self.XCodeToolsUnavailable( + "File '{fname}' in subdirectory '{subdir_name}' does not exist at any of the specified " + "prefixes. This file is required to build native code on this platform. You may need " + "to install the XCode command line developer tools from the Mac App Store.\n\n" + "If the XCode tools are installed and you are still seeing this message, please file " + "an issue at https://github.com/pantsbuild/pants/issues/new describing your " + "OSX environment and which file could not be found.\n" + "The existing install prefixes were: {pfxs}. These can be extended with " + "--{scope}-install-prefixes." + .format(fname=fname, + subdir_name=subdir_name, + pfxs=self._all_existing_install_prefixes, + scope=self.get_options_scope_equivalent_flag_component())) + + return existing_dirs - @memoized_property - def _include_search_dirs(self): - return self.get_options().include_search_dirs + @memoized_method + def path_entries(self): + return self._get_existing_subdirs('bin') - @memoized_property - def _lib_dir(self): - return os.path.join(self._install_prefix, 'lib') - - def _check_executables_exist(self): - for subdir, items in self._required_files.items(): - for fname in items: - file_path = os.path.join(self._install_prefix, subdir, fname) - if subdir == 'bin': - if not is_executable(file_path): - raise self.XCodeToolsUnavailable( - "'{exe}' is not an executable file, but it is required to build " - "native code on this platform. You may need to install the XCode " - "command line developer tools from the Mac App Store. " - "(with --required-files={req}, --install-prefix={pfx})" - .format(exe=file_path, req=self._required_files, pfx=self._install_prefix)) - else: - if not os.path.isfile(file_path): - raise self.XCodeToolsUnavailable( - "The file at path '{path}' does not exist, but it is required to build " - "native code on this platform. You may need to install the XCode " - "command line developer tools from the Mac App Store. " - "(with --required-files={req}, --install-prefix={pfx})" - .format(path=file_path, req=self._required_files, pfx=self._install_prefix)) + @memoized_method + def lib_dirs(self): + return self._get_existing_subdirs('lib') @memoized_method - def path_entries(self): - self._check_executables_exist() - return [os.path.join(self._install_prefix, 'bin')] - - def _verify_tool_name(self, name): - # TODO: introduce some more generic way to do this that can be applied to - # provided tools as well, and actually checks whether the file exists within - # some set of PATH entries and is executable. - known_tools = self._required_files['bin'] - if name in known_tools: - return name - raise self.XCodeToolsInvalid( - "Internal error: {!r} is not a valid tool. Known tools are: {!r}." - .format(name, known_tools)) + def include_dirs(self): + return self._get_existing_subdirs('include') @memoized_method def assembler(self): return Assembler( path_entries=self.path_entries(), - exe_filename=self._verify_tool_name('as'), + exe_filename='as', library_dirs=[]) @memoized_method def linker(self): return Linker( path_entries=self.path_entries(), - exe_filename=self._verify_tool_name('ld'), + exe_filename='ld', library_dirs=[]) @memoized_method def c_compiler(self): return CCompiler( path_entries=self.path_entries(), - exe_filename=self._verify_tool_name('clang'), - library_dirs=[self._lib_dir], - include_dirs=self._include_search_dirs) + exe_filename='clang', + library_dirs=self.lib_dirs(), + include_dirs=self.include_dirs()) @memoized_method def cpp_compiler(self): return CppCompiler( path_entries=self.path_entries(), - exe_filename=self._verify_tool_name('clang++'), - library_dirs=[self._lib_dir], - include_dirs=self._include_search_dirs) + exe_filename='clang++', + library_dirs=self.lib_dirs(), + include_dirs=self.include_dirs()) @rule(Assembler, [Select(XCodeCLITools)]) diff --git a/src/python/pants/backend/python/tasks/setup_py.py b/src/python/pants/backend/python/tasks/setup_py.py index ae0e6928cf4c..3115ce4098c4 100644 --- a/src/python/pants/backend/python/tasks/setup_py.py +++ b/src/python/pants/backend/python/tasks/setup_py.py @@ -151,11 +151,6 @@ class SetupPyExecutionEnvironment(datatype([ 'setup_py_native_tools', ])): - _SHARED_CMDLINE_ARGS = { - 'darwin': lambda: ['-mmacosx-version-min=10.11', '-Wl,-dylib'], - 'linux': lambda: ['-shared'], - } - def as_environment(self): ret = {} @@ -164,13 +159,22 @@ def as_environment(self): native_tools = self.setup_py_native_tools if native_tools: - ret['CC'] = native_tools.c_compiler.exe_filename - ret['CXX'] = native_tools.cpp_compiler.exe_filename - - all_ldflags = native_tools.platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS) - ret['LDFLAGS'] = safe_shlex_join(all_ldflags) - - ret.update(native_tools.linker.get_invocation_environment_dict(native_tools.platform)) + plat = native_tools.platform + + # FIXME: add an as_tuple() method to datatype so we can do this with a for loop! + ret.update(native_tools.c_compiler.get_invocation_environment_dict(plat)) + ret.update(native_tools.cpp_compiler.get_invocation_environment_dict(plat)) + ret.update(native_tools.linker.get_invocation_environment_dict(plat)) + + # FIXME(???): we need a way to compose executables hygienically -- this will work because we + # use safe shlex methods, but we should probably be composing each datatype's members, and + # only creating an environment at the very end. + prev_ldflags = safe_shlex_split(ret.get('LDFLAGS', '')) + all_new_ldflags = plat.resolve_platform_specific({ + 'darwin': lambda: prev_ldflags + ['-undefined', 'dynamic_lookup'], + 'linux': lambda: prev_ldflags, + }) + ret['LDFLAGS'] = safe_shlex_join(all_new_ldflags) return ret