refactor libc resolution and make tests pass on osx

src/python/pants/backend/native/config/environment.py

@@ -4,7 +4,6 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
-import os
 from abc import abstractproperty
 from builtins import object
 
@@ -189,10 +188,7 @@ class GCCCppToolchain(datatype([('cpp_toolchain', CppToolchain)])): pass
 
 # TODO: make this an @rule, after we can automatically produce LibcDev and other subsystems in the
 # v2 engine (see #5788).
-class HostLibcDev(datatype(['crti_object', 'fingerprint'])):
-
-  def get_lib_dir(self):
-    return os.path.dirname(self.crti_object)
+class HostLibcDev(datatype(['crti_object', 'fingerprint'])): pass
 
 
 def create_native_environment_rules():

src/python/pants/backend/native/subsystems/binaries/gcc.py

@@ -75,6 +75,8 @@ def _cpp_include_dirs(self):
       ('include/c++', self.version()),
     ])
 
+    # TODO: determine whether there is any manual explaining when any of these file paths are
+    # necessary.
     # This file is needed for C++ compilation.
     cpp_config_header_path = self._file_mapper.assert_single_path_by_glob(
       # NB: There are multiple paths matching this glob unless we provide the full path to

src/python/pants/backend/native/subsystems/libc_dev.py

@@ -100,7 +100,7 @@ def _host_libc(self):
     return self._get_host_libc_from_host_compiler()
 
   @memoized_method
-  def get_libc(self):
+  def get_libc_objects(self):
     if not self.get_options().enable_libc_search:
-      return None
-    return self._host_libc
+      return []
+    return [self._host_libc.crti_object]

src/python/pants/backend/native/subsystems/native_toolchain.py

@@ -118,8 +118,6 @@ def select_gcc_install_location(gcc):
 def select_llvm_c_toolchain(platform, native_toolchain):
   provided_clang = yield Get(CCompiler, LLVM, native_toolchain._llvm)
 
-  libc_dev = yield Get(LibcDev, NativeToolchain, native_toolchain)
-
   # These arguments are shared across platforms.
   llvm_c_compiler_args = [
     '-x', 'c', '-std=c11',
@@ -133,7 +131,6 @@ def select_llvm_c_toolchain(platform, native_toolchain):
       library_dirs=(provided_clang.library_dirs + xcode_clang.library_dirs),
       include_dirs=(provided_clang.include_dirs + xcode_clang.include_dirs),
       extra_args=(provided_clang.extra_args + llvm_c_compiler_args + xcode_clang.extra_args))
-    linker_extra_args = []
   else:
     gcc_install = yield Get(GCCInstallLocationForLLVM, GCC, native_toolchain._gcc)
     provided_gcc = yield Get(CCompiler, GCC, native_toolchain._gcc)
@@ -142,7 +139,6 @@ def select_llvm_c_toolchain(platform, native_toolchain):
       library_dirs=(provided_gcc.library_dirs + provided_clang.library_dirs),
       include_dirs=provided_gcc.include_dirs,
       extra_args=(llvm_c_compiler_args + provided_clang.extra_args + gcc_install.as_clang_argv))
-    linker_extra_args = [libc_dev.get_libc().crti_object]
 
   base_linker_wrapper = yield Get(BaseLinker, NativeToolchain, native_toolchain)
   base_linker = base_linker_wrapper.linker
@@ -151,7 +147,6 @@ def select_llvm_c_toolchain(platform, native_toolchain):
     path_entries=(base_linker.path_entries + working_c_compiler.path_entries),
     exe_filename=working_c_compiler.exe_filename,
     library_dirs=(base_linker.library_dirs + working_c_compiler.library_dirs),
-    extra_args=(base_linker.extra_args + linker_extra_args),
   )
 
   yield LLVMCToolchain(CToolchain(working_c_compiler, working_linker))
@@ -161,8 +156,6 @@ def select_llvm_c_toolchain(platform, native_toolchain):
 def select_llvm_cpp_toolchain(platform, native_toolchain):
   provided_clangpp = yield Get(CppCompiler, LLVM, native_toolchain._llvm)
 
-  libc_dev = yield Get(LibcDev, NativeToolchain, native_toolchain)
-
   # These arguments are shared across platforms.
   llvm_cpp_compiler_args = [
     '-x', 'c++', '-std=c++11',
@@ -174,14 +167,14 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
   ]
 
   if platform.normalized_os_name == 'darwin':
-    xcode_clang = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
+    xcode_clangpp = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
     working_cpp_compiler = provided_clangpp.copy(
-      path_entries=(provided_clangpp.path_entries + xcode_clang.path_entries),
-      library_dirs=(provided_clangpp.library_dirs + xcode_clang.library_dirs),
-      include_dirs=(provided_clangpp.include_dirs + xcode_clang.include_dirs),
+      path_entries=(provided_clangpp.path_entries + xcode_clangpp.path_entries),
+      library_dirs=(provided_clangpp.library_dirs + xcode_clangpp.library_dirs),
+      include_dirs=(provided_clangpp.include_dirs + xcode_clangpp.include_dirs),
       # On OSX, this uses the libc++ (LLVM) C++ standard library implementation. This is
       # feature-complete for OSX, but not for Linux (see https://libcxx.llvm.org/ for more info).
-      extra_args=(llvm_cpp_compiler_args + provided_clangpp.extra_args + xcode_clang.extra_args))
+      extra_args=(llvm_cpp_compiler_args + provided_clangpp.extra_args + xcode_clangpp.extra_args))
     extra_linking_library_dirs = []
     linker_extra_args = []
   else:
@@ -196,7 +189,7 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
     # TODO: 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++', libc_dev.get_libc().crti_object]
+    linker_extra_args=['-stdlib=libstdc++']
 
   base_linker_wrapper = yield Get(BaseLinker, NativeToolchain, native_toolchain)
   base_linker = base_linker_wrapper.linker
@@ -206,7 +199,8 @@ def select_llvm_cpp_toolchain(platform, native_toolchain):
     library_dirs=(base_linker.library_dirs + working_cpp_compiler.library_dirs),
     linking_library_dirs=(base_linker.linking_library_dirs +
                           extra_linking_library_dirs),
-    extra_args=(base_linker.extra_args + linker_extra_args))
+    extra_args=(base_linker.extra_args + linker_extra_args),
+  )
 
   yield LLVMCppToolchain(CppToolchain(working_cpp_compiler, working_linker))
 
@@ -217,7 +211,6 @@ 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)
-  libc_dev = yield Get(LibcDev, NativeToolchain, native_toolchain)
 
   if platform.normalized_os_name == 'darwin':
     # GCC needs access to some headers that are only provided by the XCode toolchain
@@ -226,11 +219,9 @@ def select_gcc_c_toolchain(platform, native_toolchain):
     # 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 = xcode_clang.include_dirs + provided_gcc.include_dirs
-    linker_extra_args = []
+    new_include_dirs = provided_gcc.include_dirs + xcode_clang.include_dirs
   else:
     new_include_dirs = provided_gcc.include_dirs
-    linker_extra_args = [libc_dev.get_libc().crti_object]
 
   working_c_compiler = provided_gcc.copy(
     path_entries=(provided_gcc.path_entries + assembler.path_entries),
@@ -244,7 +235,6 @@ def select_gcc_c_toolchain(platform, native_toolchain):
     path_entries=(working_c_compiler.path_entries + base_linker.path_entries),
     exe_filename=working_c_compiler.exe_filename,
     library_dirs=(base_linker.library_dirs + working_c_compiler.library_dirs),
-    extra_args=(base_linker.extra_args + linker_extra_args),
   )
 
   yield GCCCToolchain(CToolchain(working_c_compiler, working_linker))
@@ -256,7 +246,11 @@ def select_gcc_cpp_toolchain(platform, native_toolchain):
 
   # GCC needs an assembler, so we provide that (platform-specific) tool here.
   assembler = yield Get(Assembler, NativeToolchain, native_toolchain)
-  libc_dev = yield Get(LibcDev, NativeToolchain, native_toolchain)
+
+  gcc_cpp_compiler_args = [
+    '-x', 'c++', '-std=c++11',
+    '-nostdinc++',
+  ]
 
   if platform.normalized_os_name == 'darwin':
     # GCC needs access to some headers that are only provided by the XCode toolchain
@@ -265,33 +259,28 @@ def select_gcc_cpp_toolchain(platform, native_toolchain):
     # TODO: we should be providing all of these (so we can eventually phase out XCodeCLITools
     # entirely).
     xcode_clangpp = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
-    new_include_dirs = xcode_clangpp.include_dirs + provided_gpp.include_dirs
+    working_cpp_compiler = provided_gpp.copy(
+      path_entries=(provided_gpp.path_entries + assembler.path_entries),
+      include_dirs=(provided_gpp.include_dirs + xcode_clangpp.include_dirs),
+      extra_args=(gcc_cpp_compiler_args + provided_gpp.extra_args + xcode_clangpp.extra_args),
+    )
     extra_linking_library_dirs = []
-    linker_extra_args = []
   else:
     provided_clangpp = yield Get(CppCompiler, LLVM, native_toolchain._llvm)
-    new_include_dirs = provided_gpp.include_dirs
+    working_cpp_compiler = provided_gpp.copy(
+      path_entries=(provided_gpp.path_entries + assembler.path_entries),
+      extra_args=(gcc_cpp_compiler_args + provided_gpp.extra_args),
+    )
     extra_linking_library_dirs = provided_gpp.library_dirs + provided_clangpp.library_dirs
-    linker_extra_args = [libc_dev.get_libc().crti_object]
-
-  working_cpp_compiler = provided_gpp.copy(
-    path_entries=(provided_gpp.path_entries + assembler.path_entries),
-    include_dirs=new_include_dirs,
-    extra_args=([
-      '-x', 'c++', '-std=c++11',
-      '-nostdinc++',
-    ]))
 
   base_linker_wrapper = yield Get(BaseLinker, NativeToolchain, native_toolchain)
   base_linker = base_linker_wrapper.linker
   working_linker = base_linker.copy(
     path_entries=(working_cpp_compiler.path_entries + base_linker.path_entries),
     exe_filename=working_cpp_compiler.exe_filename,
     library_dirs=(base_linker.library_dirs + working_cpp_compiler.library_dirs),
-    linking_library_dirs=(base_linker.linking_library_dirs +
-                          extra_linking_library_dirs),
-    extra_args=(base_linker.extra_args +
-                linker_extra_args))
+    linking_library_dirs=(base_linker.linking_library_dirs + extra_linking_library_dirs),
+  )
 
   yield GCCCppToolchain(CppToolchain(working_cpp_compiler, working_linker))
 

src/python/pants/backend/native/subsystems/xcode_cli_tools.py

@@ -84,6 +84,7 @@ 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.
     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)]
 
@@ -123,7 +124,7 @@ def lib_dirs(self):
     return self._get_existing_subdirs('lib')
 
   @memoized_method
-  def include_dirs(self):
+  def include_dirs(self, include_cpp_inc=False):
     base_inc_dirs = self._get_existing_subdirs('include')
 
     all_inc_dirs = base_inc_dirs
@@ -133,6 +134,12 @@ def include_dirs(self):
       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
 
   @memoized_method
@@ -166,7 +173,7 @@ def cpp_compiler(self):
       path_entries=self.path_entries(),
       exe_filename='clang++',
       library_dirs=self.lib_dirs(),
-      include_dirs=self.include_dirs(),
+      include_dirs=self.include_dirs(include_cpp_inc=True),
       extra_args=[MIN_OSX_VERSION_ARG])
 
 

src/python/pants/backend/native/tasks/link_shared_libraries.py

@@ -7,6 +7,7 @@
 import os
 
 from pants.backend.native.config.environment import Linker, Platform
+from pants.backend.native.subsystems.libc_dev import LibcDev
 from pants.backend.native.targets.native_artifact import NativeArtifact
 from pants.backend.native.targets.native_library import NativeLibrary
 from pants.backend.native.tasks.native_compile import ObjectFiles
@@ -72,6 +73,17 @@ def platform(self):
     # TODO: convert this to a v2 engine dependency injection.
     return Platform.create()
 
+  @memoized_property
+  def _libc(self):
+    return self._request_single(LibcDev, self._native_toolchain)
+
+  @memoized_property
+  def libc_objects(self):
+    return self.platform.resolve_platform_specific({
+      'darwin': lambda: [],
+      'linux': lambda: self._libc.get_libc_objects(),
+    })
+
   def execute(self):
     targets_providing_artifacts = self.context.targets(NativeLibrary.produces_ctypes_native_library)
     compiled_objects_product = self.context.products.get(ObjectFiles)
@@ -176,7 +188,8 @@ def _execute_link_request(self, link_request):
            ['-o', os.path.abspath(resulting_shared_lib_path)] +
            ['-L{}'.format(lib_dir) for lib_dir in link_request.external_lib_dirs] +
            ['-l{}'.format(lib_name) for lib_name in link_request.external_lib_names] +
-           [os.path.abspath(obj) for obj in object_files])
+           [os.path.abspath(obj) for obj in object_files] +
+           self.libc_objects)
 
     self.context.log.info("selected linker exe name: '{}'".format(linker.exe_filename))
     self.context.log.debug("linker argv: {}".format(cmd))

tests/python/pants_test/backend/native/subsystems/test_libc_resolution.py

@@ -25,18 +25,13 @@ def setUp(self):
     self.libc = global_subsystem_instance(LibcDev)
     self.platform = Platform.create()
 
-  @platform_specific('linux')
   def test_libc_search_failure(self):
     with self.assertRaises(LibcDev.HostLibcDevResolutionError) as cm:
-      self.libc.get_libc_dirs(self.platform)
+      self.libc.get_libc_objects()
     expected_msg = (
       "Could not locate crti.o in directory /does/not/exist provided by the --libc-dir option.")
     self.assertEqual(expected_msg, str(cm.exception))
 
-  @platform_specific('darwin')
-  def test_libc_search_noop_osx(self):
-    self.assertEqual([], self.libc.get_libc_dirs(self.platform))
-
 
 class TestLibcSearchDisabled(TestBase):
 
@@ -51,9 +46,8 @@ def setUp(self):
     self.libc = global_subsystem_instance(LibcDev)
     self.platform = Platform.create()
 
-  @platform_specific('linux')
   def test_libc_disabled_search(self):
-    self.assertEqual([], self.libc.get_libc_dirs(self.platform))
+    self.assertEqual([], self.libc.get_libc_objects())
 
 
 class TestLibcCompilerSearchFailure(TestBase):
@@ -72,7 +66,7 @@ def setUp(self):
   @platform_specific('linux')
   def test_libc_compiler_search_failure(self):
     with self.assertRaises(ParseSearchDirs.ParseSearchDirsError) as cm:
-      self.libc.get_libc_dirs(self.platform)
+      self.libc.get_libc_objects()
     expected_msg = (
       "Process invocation with argv "
       "'this_executable_does_not_exist -print-search-dirs' and environment None failed.")

tests/python/pants_test/backend/native/subsystems/test_native_toolchain.py

@@ -119,9 +119,11 @@ def _hello_world_source_environment(self, toolchain_type, file_name, contents):
 
   def _invoke_compiler(self, compiler, args):
     cmd = [compiler.exe_filename] + compiler.extra_args + args
-    return self._invoke_capturing_output(
-      cmd,
-      compiler.as_invocation_environment_dict)
+    env = compiler.as_invocation_environment_dict
+    # TODO: add an `extra_args`-like field to `Executable`s which allows for overriding env vars
+    # like this, but declaratively!
+    env['LC_ALL'] = 'C'
+    return self._invoke_capturing_output(cmd, env)
 
   def _invoke_linker(self, linker, args):
     cmd = [linker.exe_filename] + linker.extra_args + args