Skip to content

Commit

Permalink
remove some of the include dir dark magic by standardizing options
Browse files Browse the repository at this point in the history
specifically remove the hardcoded extra include paths in the xcode clang wrapper
  • Loading branch information
cosmicexplorer committed Dec 5, 2018
1 parent 64e3e54 commit 5473093
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 22 deletions.
18 changes: 13 additions & 5 deletions src/python/pants/backend/native/subsystems/native_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,12 @@ class GCCInstallLocationForLLVM(datatype(['toolchain_dir'])):
"""This class is convertible into a list of command line arguments for clang and clang++.
This is only used on Linux. The option --gcc-toolchain stops clang from searching for another gcc
on the host system. The option appears to only exist on Linux clang and clang++."""
on the host system. The option appears to only exist on Linux clang and clang++.
"""

@property
def as_clang_argv(self):
# TODO(#6143): describe exactly what this argument does to the clang/clang++ invocation!
return ['--gcc-toolchain={}'.format(self.toolchain_dir)]


Expand All @@ -161,6 +163,7 @@ def select_llvm_c_toolchain(platform, native_toolchain):
llvm_c_compiler_args = [
'-x', 'c', '-std=c11',
'-nobuiltininc',
'-nostdinc',
]

if platform.normalized_os_name == 'darwin':
Expand Down Expand Up @@ -203,6 +206,7 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
# implementation, or any from the host system. Instead, we use include dirs from the
# XCodeCLITools or GCC.
'-nobuiltininc',
'-nostdinc',
'-nostdinc++',
]

Expand All @@ -226,7 +230,7 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
# NB: we use g++'s headers on Linux, and therefore their C++ standard library.
include_dirs=provided_gpp.include_dirs,
extra_args=(llvm_cpp_compiler_args + provided_clangpp.extra_args + gcc_install.as_clang_argv))
# TODO: why are these necessary? this is very mysterious.
# TODO(#6855): why are these necessary? this is very mysterious.
extra_linking_library_dirs = provided_gpp.library_dirs + provided_clangpp.library_dirs
# Ensure we use libstdc++, provided by g++, during the linking stage.
linker_extra_args=['-stdlib=libstdc++']
Expand All @@ -253,12 +257,15 @@ def select_gcc_c_toolchain(platform, native_toolchain):
# GCC needs an assembler, so we provide that (platform-specific) tool here.
assembler = yield Get(Assembler, NativeToolchain, native_toolchain)

gcc_c_compiler_args = [
'-x', 'c', '-std=c11',
'-nostdinc',
]

if platform.normalized_os_name == 'darwin':
# GCC needs access to some headers that are only provided by the XCode toolchain
# currently (e.g. "_stdio.h"). These headers are unlikely to change across versions, so this is
# probably safe.
# TODO: we should be providing all of these (so we can eventually phase out XCodeCLITools
# entirely).
xcode_clang = yield Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
new_include_dirs = provided_gcc.include_dirs + xcode_clang.include_dirs
else:
Expand All @@ -267,7 +274,7 @@ def select_gcc_c_toolchain(platform, native_toolchain):
working_c_compiler = provided_gcc.copy(
path_entries=(provided_gcc.path_entries + assembler.path_entries),
include_dirs=new_include_dirs,
extra_args=['-x', 'c', '-std=c11'])
extra_args=gcc_c_compiler_args)

gcc_linker_wrapper = yield Get(GCCLinker, NativeToolchain, native_toolchain)
gcc_linker = gcc_linker_wrapper.linker
Expand All @@ -290,6 +297,7 @@ def select_gcc_cpp_toolchain(platform, native_toolchain):

gcc_cpp_compiler_args = [
'-x', 'c++', '-std=c++11',
'-nostdinc',
'-nostdinc++',
]

Expand Down
21 changes: 4 additions & 17 deletions src/python/pants/backend/native/subsystems/xcode_cli_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ def _all_existing_install_prefixes(self):
# NB: We use @memoized_method in this file for methods which may raise.
@memoized_method
def _get_existing_subdirs(self, subdir_name):
# TODO: consume ArchiveFileMapper instead of doing all that logic over again in this file.
# TODO(#6143): We should attempt to use ParseSearchDirs here or find some documentation on which
# directories we should be adding to the include path, and why. If we do need to manually
# specify paths, use ArchiveFileMapper instead of doing all that logic over again in this file.
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 is_readable_dir(existing_dir)]

Expand Down Expand Up @@ -125,22 +127,7 @@ def lib_dirs(self):

@memoized_method
def include_dirs(self, include_cpp_inc=False):
base_inc_dirs = self._get_existing_subdirs('include')

all_inc_dirs = base_inc_dirs
for d in base_inc_dirs:
# TODO: figure out what this directory does and why it's not already found by this compiler.
secure_inc_dir = os.path.join(d, 'secure')
if is_readable_dir(secure_inc_dir):
all_inc_dirs.append(secure_inc_dir)

# TODO: figure out why we need to specify this directory separately.
if include_cpp_inc:
cpp_inc_dir = os.path.join(d, 'c++/v1')
if is_readable_dir(cpp_inc_dir):
all_inc_dirs.append(cpp_inc_dir)

return all_inc_dirs
return self._get_existing_subdirs('include')

@memoized_method
def assembler(self):
Expand Down

0 comments on commit 5473093

Please sign in to comment.