Add testproject with C/C++ interdependent targets, fix native backend…

… design mistake (#6628)

### Problem

*This probably blocks #6486 and #6492.*

As @CMLivingston and I realized sometime this week (to my *immense* dismay), we had no tests of native library targets depending on each other. When making these examples, I realized two things:

1. `--strict-deps` doesn't really make sense as two separate options for C and C++ targets which can depend on each other.
2. The implementation of `--strict-deps` in the native backend ("caching" native target interdependencies in a product from the compiler tasks) was absurd and unnecessary.

I think I vaguely recall that the fact that `LinkSharedLibraries` previously did not have a subsystem dependency on the `CCompileSettings` or `CppCompileSettings` subsystems may have led to a subtle caching bug in the link task when options in those subsystems were modified, but I don't remember all the details right now. Regardless, that bug would have been fixed with this PR -- see the below:

### Solution

- Move dependency calculation into a single `NativeBuildSettings` subsystem which is consumed by
compile and link tasks, and use it to calculate the dependencies in both tasks.
  - Drop the very unnecessary `NativeTargetDependencies` product.
- Move a ton of logic into `NativeTask` that really should have been there instead of `NativeCompile`, but couldn't because of the hacked together dependency calculation.
- Add a testproject with C and C++ targets depending on each other, and an integration test showing the above all works.

### Result

We have a working example of C and C++ targets depending on each other, resulting in idiomatic enough C and C++, which also work extremely effectively in a `python_dist()` target.

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

@@ -0,0 +1,32 @@
+# coding=utf-8
+# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+from pants.backend.native.subsystems.utils.mirrored_target_option_mixin import \
+  MirroredTargetOptionMixin
+from pants.subsystem.subsystem import Subsystem
+
+
+class NativeBuildSettings(Subsystem, MirroredTargetOptionMixin):
+  """Any settings relevant to a compiler and/or linker invocation."""
+  options_scope = 'native-build-settings'
+
+  mirrored_option_to_kwarg_map = {
+    'strict_deps': 'strict_deps',
+  }
+
+  @classmethod
+  def register_options(cls, register):
+    super(NativeBuildSettings, cls).register_options(register)
+
+    # TODO: rename this so it's clear it is not the same option as JVM strict deps!
+    register('--strict-deps', type=bool, default=True, fingerprint=True, advanced=True,
+             help="Whether to include only dependencies directly declared in the BUILD file "
+                  "for C and C++ targets by default. If this is False, all transitive dependencies "
+                  "are used when compiling and linking native code. C and C++ targets may override "
+                  "this behavior with the strict_deps keyword argument as well.")
+
+  def get_strict_deps_value_for_target(self, target):
+    return self.get_target_mirrored_option('strict_deps', target)

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

@@ -0,0 +1,41 @@
+# coding=utf-8
+# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+from pants.backend.native.subsystems.utils.mirrored_target_option_mixin import \
+  MirroredTargetOptionMixin
+from pants.subsystem.subsystem import Subsystem
+
+
+class NativeBuildStepSettingsBase(Subsystem, MirroredTargetOptionMixin):
+
+  mirrored_option_to_kwarg_map = {
+    'fatal_warnings': 'fatal_warnings',
+  }
+
+  @classmethod
+  def register_options(cls, register):
+    super(NativeBuildStepSettingsBase, cls).register_options(register)
+
+    # TODO: implement compiler_option_sets as an interface to platform/host-specific optimization
+    # flags!
+    register('--fatal-warnings', type=bool, default=True, fingerprint=True, advanced=True,
+             help='The default for the "fatal_warnings" argument for targets of this language.')
+
+  def get_fatal_warnings_value_for_target(self, target):
+    return self.get_target_mirrored_option('fatal_warnings', target)
+
+
+class CCompileSettings(NativeBuildStepSettingsBase):
+  options_scope = 'c-compile-settings'
+
+
+class CppCompileSettings(NativeBuildStepSettingsBase):
+  options_scope = 'cpp-compile-settings'
+
+
+# TODO: add a fatal_warnings kwarg to NativeArtifact and make a LinkSharedLibrariesSettings subclass
+# of NativeBuildStepSettingsBase here! The method should work even though NativeArtifact is not a
+# Target.

src/python/pants/backend/native/subsystems/utils/mirrored_target_option_mixin.py

@@ -0,0 +1,35 @@
+# coding=utf-8
+# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+from builtins import object
+
+from pants.util.meta import classproperty
+
+
+# TODO: consider coalescing existing methods of mirroring options between a target and a subsystem
+# -- see pants.backend.jvm.subsystems.dependency_context.DependencyContext#defaulted_property()!
+class MirroredTargetOptionMixin(object):
+  """Get option values which may be set in this subsystem or in a Target's keyword argument."""
+
+  @classproperty
+  def mirrored_option_to_kwarg_map(cls):
+    """Subclasses should override and return a dict of (subsystem option name) -> (target kwarg).
+
+    This classproperty should return a dict mapping this subsystem's options attribute name (with
+    underscores) to the corresponding target's keyword argument name.
+    """
+    raise NotImplementedError()
+
+  def get_target_mirrored_option(self, option_name, target):
+    field_name = self.mirrored_option_to_kwarg_map[option_name]
+    return self._get_subsystem_target_mirrored_field_value(option_name, field_name, target)
+
+  def _get_subsystem_target_mirrored_field_value(self, option_name, field_name, target):
+    """Get the attribute `field_name` from `target` if set, else from this subsystem's options."""
+    tgt_setting = getattr(target, field_name)
+    if tgt_setting is None:
+      return getattr(self.get_options(), option_name)
+    return tgt_setting

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

@@ -5,11 +5,9 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 from pants.backend.native.config.environment import LLVMCToolchain
-from pants.backend.native.subsystems.native_compile_settings import CCompileSettings
-from pants.backend.native.subsystems.native_toolchain import NativeToolchain
+from pants.backend.native.subsystems.native_build_step_settings_base import CCompileSettings
 from pants.backend.native.targets.native_library import CLibrary
 from pants.backend.native.tasks.native_compile import NativeCompile
-from pants.util.memo import memoized_property
 from pants.util.objects import SubclassesOf
 
 
@@ -28,21 +26,10 @@ def implementation_version(cls):
 
   @classmethod
   def subsystem_dependencies(cls):
-    return super(CCompile, cls).subsystem_dependencies() + (
-      CCompileSettings.scoped(cls),
-      NativeToolchain.scoped(cls),
-    )
-
-  @memoized_property
-  def _native_toolchain(self):
-    return NativeToolchain.scoped_instance(self)
+    return super(CCompile, cls).subsystem_dependencies() + (CCompileSettings.scoped(cls),)
 
   def get_compile_settings(self):
     return CCompileSettings.scoped_instance(self)
 
-  @memoized_property
-  def _c_toolchain(self):
-    return self._request_single(LLVMCToolchain, self._native_toolchain).c_toolchain
-
   def get_compiler(self):
-    return self._c_toolchain.c_compiler
+    return self._request_single(LLVMCToolchain, self._native_toolchain).c_toolchain.c_compiler

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

@@ -5,11 +5,9 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 from pants.backend.native.config.environment import LLVMCppToolchain
-from pants.backend.native.subsystems.native_compile_settings import CppCompileSettings
-from pants.backend.native.subsystems.native_toolchain import NativeToolchain
+from pants.backend.native.subsystems.native_build_step_settings_base import CppCompileSettings
 from pants.backend.native.targets.native_library import CppLibrary
 from pants.backend.native.tasks.native_compile import NativeCompile
-from pants.util.memo import memoized_property
 from pants.util.objects import SubclassesOf
 
 
@@ -28,21 +26,10 @@ def implementation_version(cls):
 
   @classmethod
   def subsystem_dependencies(cls):
-    return super(CppCompile, cls).subsystem_dependencies() + (
-      CppCompileSettings.scoped(cls),
-      NativeToolchain.scoped(cls),
-    )
-
-  @memoized_property
-  def _native_toolchain(self):
-    return NativeToolchain.scoped_instance(self)
+    return super(CppCompile, cls).subsystem_dependencies() + (CppCompileSettings.scoped(cls),)
 
   def get_compile_settings(self):
     return CppCompileSettings.scoped_instance(self)
 
-  @memoized_property
-  def _cpp_toolchain(self):
-    return self._request_single(LLVMCppToolchain, self._native_toolchain).cpp_toolchain
-
   def get_compiler(self):
-    return self._cpp_toolchain.cpp_compiler
+    return self._request_single(LLVMCppToolchain, self._native_toolchain).cpp_toolchain.cpp_compiler

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

@@ -7,17 +7,15 @@
 import os
 
 from pants.backend.native.config.environment import Linker, LLVMCppToolchain, Platform
-from pants.backend.native.subsystems.native_toolchain import NativeToolchain
 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 NativeTargetDependencies, ObjectFiles
+from pants.backend.native.tasks.native_compile import ObjectFiles
 from pants.backend.native.tasks.native_external_library_fetch import NativeExternalLibraryFiles
 from pants.backend.native.tasks.native_task import NativeTask
 from pants.base.exceptions import TaskError
 from pants.base.workunit import WorkUnit, WorkUnitLabel
-from pants.util.collections import assert_single_element
 from pants.util.memo import memoized_property
-from pants.util.objects import datatype
+from pants.util.objects import SubclassesOf, datatype
 from pants.util.process_handler import subprocess
 
 
@@ -39,14 +37,17 @@ class LinkSharedLibraries(NativeTask):
 
   options_scope = 'link-shared-libraries'
 
+  # TODO(#6486): change this to include ExternalNativeLibrary, then add a test that strict-deps
+  # works on external libs.
+  source_target_constraint = SubclassesOf(NativeLibrary)
+
   @classmethod
   def product_types(cls):
     return [SharedLibrary]
 
   @classmethod
   def prepare(cls, options, round_manager):
     super(LinkSharedLibraries, cls).prepare(options, round_manager)
-    round_manager.require(NativeTargetDependencies)
     round_manager.require(ObjectFiles)
     round_manager.optional_product(NativeExternalLibraryFiles)
 
@@ -56,18 +57,10 @@ def cache_target_dirs(self):
 
   @classmethod
   def implementation_version(cls):
-    return super(LinkSharedLibraries, cls).implementation_version() + [('LinkSharedLibraries', 0)]
+    return super(LinkSharedLibraries, cls).implementation_version() + [('LinkSharedLibraries', 1)]
 
   class LinkSharedLibrariesError(TaskError): pass
 
-  @classmethod
-  def subsystem_dependencies(cls):
-    return super(LinkSharedLibraries, cls).subsystem_dependencies() + (NativeToolchain.scoped(cls),)
-
-  @memoized_property
-  def _native_toolchain(self):
-    return NativeToolchain.scoped_instance(self)
-
   @memoized_property
   def _cpp_toolchain(self):
     return self._request_single(LLVMCppToolchain, self._native_toolchain).cpp_toolchain
@@ -81,17 +74,8 @@ def platform(self):
     # TODO: convert this to a v2 engine dependency injection.
     return Platform.create()
 
-  def _retrieve_single_product_at_target_base(self, product_mapping, target):
-    self.context.log.debug("product_mapping: {}".format(product_mapping))
-    self.context.log.debug("target: {}".format(target))
-    product = product_mapping.get(target)
-    single_base_dir = assert_single_element(product.keys())
-    single_product = assert_single_element(product[single_base_dir])
-    return single_product
-
   def execute(self):
     targets_providing_artifacts = self.context.targets(NativeLibrary.produces_ctypes_native_library)
-    native_target_deps_product = self.context.products.get(NativeTargetDependencies)
     compiled_objects_product = self.context.products.get(ObjectFiles)
     shared_libs_product = self.context.products.get(SharedLibrary)
     external_libs_product = self.context.products.get_data(NativeExternalLibraryFiles)
@@ -108,7 +92,7 @@ def execute(self):
           # perform a link to every native_external_library for all targets in the closure.
           # https://github.com/pantsbuild/pants/issues/6178
           link_request = self._make_link_request(
-            vt, compiled_objects_product, native_target_deps_product, external_libs_product)
+            vt, compiled_objects_product, external_libs_product)
           self.context.log.debug("link_request: {}".format(link_request))
           shared_library = self._execute_link_request(link_request)
 
@@ -123,7 +107,7 @@ def execute(self):
         else:
           all_shared_libs_by_name[shared_library.name] = shared_library
 
-        shared_libs_product.add(vt.target, vt.target.target_base).append(shared_library)
+        self._add_product_at_target_base(shared_libs_product, vt.target, shared_library)
 
   def _retrieve_shared_lib_from_cache(self, vt):
     native_artifact = vt.target.ctypes_native_library
@@ -134,14 +118,10 @@ def _retrieve_shared_lib_from_cache(self, vt):
                                           .format(path_to_cached_lib))
     return SharedLibrary(name=native_artifact.lib_name, path=path_to_cached_lib)
 
-  def _make_link_request(self,
-                         vt,
-                         compiled_objects_product,
-                         native_target_deps_product,
-                         external_libs_product):
+  def _make_link_request(self, vt, compiled_objects_product, external_libs_product):
     self.context.log.debug("link target: {}".format(vt.target))
 
-    deps = self._retrieve_single_product_at_target_base(native_target_deps_product, vt.target)
+    deps = self.native_deps(vt.target)
 
     all_compiled_object_files = []
     for dep_tgt in deps:
@@ -174,13 +154,6 @@ def _make_link_request(self,
     'linux': lambda: ['-shared'],
   }
 
-  def _get_third_party_lib_args(self, link_request):
-    ext_libs = link_request.external_libs_info
-    if not ext_libs:
-      return []
-
-    return ext_libs.get_third_party_lib_args()
-
   def _execute_link_request(self, link_request):
     object_files = link_request.object_files
 
@@ -199,6 +172,8 @@ def _execute_link_request(self, link_request):
            self.platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS) +
            linker.extra_args +
            ['-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])
     self.context.log.debug("linker command: {}".format(cmd))