refactor the compilation and linking pipeline to use subsystems

- also add `PythonNativeCode` subsystem to bridge the native and python backends

refactor the compilation and linking pipeline to use subsystems

add some notes

fix rebase issues

add link to pantsbuild#5788 -- maybe use variants for args for static libs

move `native_source_extensions` to a new `PythonNativeCode` subsystem

update native toolchain docs and remove bad old tests

move tgt_closure_platforms into the new `PythonNativeCode` subsystem

remove unnecessary logging

remove compile_settings_class in favor of another abstractmethod

refactor `NativeCompile` and add documentation

improve debug logging in NativeCompile

document NativeCompileSettings

refactor and add docstrings

convert provides= to ctypes_dylib= and add many more docstrings

remove or improve TODOs

improve or remove FIXMEs

improve some docstrings, demote a FIXME, and add a TODO

link FIXMEs to a ticket

add notes to the ctypes testproject

update mock object for strict deps -- test passes

fix failing integration test on osx

add hack to let travis pass

fix the system_id key in llvm and add a shameful hack to pass travis

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

@@ -13,10 +13,7 @@
 
 
 class UnsupportedPlatformError(Exception):
-  """Thrown if the native toolchain is invoked on an unrecognized platform.
-
-    Note that the native toolchain should work on all of Pants's supported
-    platforms."""
+  """Thrown if pants is running on an unrecognized platform."""
 
 
 class Platform(datatype(['normalized_os_name'])):
@@ -48,11 +45,14 @@ class Executable(object):
 
   @abstractproperty
   def path_entries(self):
-    """???"""
+    """A list of directory paths containing this executable, to be used in a subprocess's PATH.
+
+    This may be multiple directories, e.g. if the main executable program invokes any subprocesses.
+    """
 
   @abstractproperty
   def exe_filename(self):
-    """???"""
+    """The "entry point" -- which file to invoke when PATH is set to `path_entries()`."""
 
 
 class Linker(datatype([

src/python/pants/backend/native/register.py

@@ -11,9 +11,8 @@
 from pants.backend.native.subsystems.binaries.llvm import create_llvm_rules
 from pants.backend.native.subsystems.native_toolchain import create_native_toolchain_rules
 from pants.backend.native.subsystems.xcode_cli_tools import create_xcode_cli_tools_rules
-from pants.backend.native.targets.c_library import CLibrary
-from pants.backend.native.targets.cpp_library import CppLibrary
 from pants.backend.native.targets.native_artifact import NativeArtifact
+from pants.backend.native.targets.native_library import CLibrary, CppLibrary
 from pants.backend.native.tasks.c_compile import CCompile
 from pants.backend.native.tasks.cpp_compile import CppCompile
 from pants.backend.native.tasks.link_shared_libraries import LinkSharedLibraries

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

@@ -24,7 +24,7 @@ class LLVMReleaseUrlGenerator(BinaryToolUrlGenerator):
   # TODO(cosmicexplorer): Give a more useful error message than KeyError if the host platform was
   # not recognized (and make it easy for other BinaryTool subclasses to do this as well).
   _SYSTEM_ID = {
-    'darwin': 'apple-darwin',
+    'mac': 'apple-darwin',
     'linux': 'linux-gnu-ubuntu-16.04',
   }
 

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

@@ -0,0 +1,54 @@
+# 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, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+from pants.subsystem.subsystem import Subsystem
+
+
+class NativeCompileSettings(Subsystem):
+  """Any settings relevant to a compiler invocation."""
+
+  default_header_file_extensions = None
+  default_source_file_extensions = None
+
+  @classmethod
+  def register_options(cls, register):
+    super(NativeCompileSettings, cls).register_options(register)
+
+    # TODO: have some more formal method of mirroring options between a target and a subsystem?
+    register('--strict-deps', type=bool, default=True, fingerprint=True, advanced=True,
+             help='The default for the "strict_deps" argument for targets of this language.')
+    register('--fatal-warnings', type=bool, default=True, fingerprint=True, advanced=True,
+             help='The default for the "fatal_warnings" argument for targets of this language.')
+
+    # TODO: make a list of file extension option type?
+    register('--header-file-extensions', type=list, default=cls.default_header_file_extensions,
+             fingerprint=True, advanced=True,
+             help='The allowed extensions for header files, as a list of strings.')
+    register('--source-file-extensions', type=list, default=cls.default_source_file_extensions,
+             fingerprint=True, advanced=True,
+             help='The allowed extensions for source files, as a list of strings.')
+
+  def get_subsystem_target_mirrored_field_value(self, 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(), field_name)
+    return tgt_setting
+
+
+class CCompileSettings(NativeCompileSettings):
+  options_scope = 'c-compile-settings'
+
+  default_header_file_extensions = ['.h']
+  default_source_file_extensions = ['.c']
+
+
+class CppCompileSettings(NativeCompileSettings):
+  options_scope = 'cpp-compile-settings'
+
+  default_header_file_extensions = ['.h', '.hpp', '.tpp']
+  default_source_file_extensions = ['.cpp', '.cxx', '.cc']

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

@@ -19,31 +19,17 @@
 class NativeToolchain(Subsystem):
   """Abstraction over platform-specific tools to compile and link native code.
 
-  This "native toolchain" subsystem is an abstraction that exposes directories
-  containing executables to compile and link "native" code (for now, C and C++
-  are supported). Consumers of this subsystem can add these directories to their
-  PATH to invoke subprocesses which use these tools.
-
-  This abstraction is necessary for two reasons. First, because there are
-  multiple binaries involved in compilation and linking, which often invoke
-  other binaries that must also be available on the PATH. Second, because unlike
-  other binary tools in Pants, we can't provide the same package built for both
-  OSX and Linux, because there is no open-source linker for OSX with a
-  compatible license.
-
-  So when this subsystem is consumed, Pants will download and unpack archives
-  (if necessary) which together provide an appropriate "native toolchain" for
-  the host platform. On OSX, Pants will also find and provide path entries for
-  the XCode command-line tools, or error out with installation instructions if
-  the XCode tools could not be found.
+  When this subsystem is consumed, Pants will download and unpack archives (if necessary) which
+  together provide an appropriate "native toolchain" for the host platform: a compiler and linker,
+  usually. This subsystem exposes the toolchain through `@rule`s, which tasks then request during
+  setup or execution (synchronously, for now).
+
+  NB: Currently, on OSX, Pants will find and invoke the XCode command-line tools, or error out with
+  installation instructions if the XCode tools could not be found.
   """
 
   options_scope = 'native-toolchain'
 
-  # This is a list of subsystems which implement `ExecutablePathProvider` and
-  # can be provided for all supported platforms.
-  _CROSS_PLATFORM_SUBSYSTEMS = [LLVM]
-
   @classmethod
   def subsystem_dependencies(cls):
     return super(NativeToolchain, cls).subsystem_dependencies() + (
@@ -72,7 +58,7 @@ def _xcode_cli_tools(self):
 
 @rule(Linker, [Select(Platform), Select(NativeToolchain)])
 def select_linker(platform, native_toolchain):
-  # TODO: make it possible to yield Get with a non-static
+  # TODO(#5933): make it possible to yield Get with a non-static
   # subject type and use `platform.resolve_platform_specific()`, something like:
   # linker = platform.resolve_platform_specific({
   #   'darwin': lambda: Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools),

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

@@ -54,7 +54,7 @@ def _check_executables_exist(self):
 
   def path_entries(self):
     self._check_executables_exist()
-    return [self._INSTALL_LOCATION]
+    return [self._install_location]
 
   def linker(self, platform):
     return Linker(

src/python/pants/backend/native/targets/native_artifact.py

@@ -12,15 +12,15 @@
 
 
 class NativeArtifact(datatype(['lib_name']), PayloadField):
-  """???"""
+  """A BUILD file object declaring a target can be exported to other languages with a native ABI."""
 
-  # TODO: why do we need this to be a method and not e.g. a field?
+  # TODO: This should probably be made into an @classproperty (see PR #5901).
   @classmethod
   def alias(cls):
     return 'native_artifact'
 
-  def as_filename(self, platform):
-    # TODO: check that the name conforms to some format (e.g. no dots?)
+  def as_shared_lib(self, platform):
+    # TODO: check that the name conforms to some format in the constructor (e.g. no dots?).
     return platform.resolve_platform_specific({
       'darwin': lambda: 'lib{}.dylib'.format(self.lib_name),
       'linux': lambda: 'lib{}.so'.format(self.lib_name),

src/python/pants/backend/native/targets/native_library.py

@@ -5,47 +5,38 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-import logging
-
 from pants.backend.native.targets.native_artifact import NativeArtifact
 from pants.base.exceptions import TargetDefinitionException
 from pants.base.payload import Payload
 from pants.base.payload_field import PrimitiveField
 from pants.build_graph.target import Target
 
 
-logger = logging.getLogger(__name__)
-
-
 class NativeLibrary(Target):
-  """???"""
+  """A class wrapping targets containing sources for C-family languages and related code."""
 
   @classmethod
-  def provides_native_artifact(cls, target):
-    return isinstance(target, cls) and bool(target.provides)
+  def produces_ctypes_dylib(cls, target):
+    return isinstance(target, cls) and bool(target.ctypes_dylib)
 
-  def __init__(self, address, payload=None, sources=None, provides=None,
+  def __init__(self, address, payload=None, sources=None, ctypes_dylib=None,
                strict_deps=None, fatal_warnings=None, **kwargs):
-    logger.debug("address: {}".format(address))
-    logger.debug("sources: {}".format(sources))
 
     if not payload:
       payload = Payload()
     sources_field = self.create_sources_field(sources, address.spec_path, key_arg='sources')
     payload.add_fields({
       'sources': sources_field,
-      'provides': provides,
+      'ctypes_dylib': ctypes_dylib,
       'strict_deps': PrimitiveField(strict_deps),
       'fatal_warnings': PrimitiveField(fatal_warnings),
     })
 
-    logger.debug("sources_field.sources: {}".format(sources_field.sources))
-
-    if provides and not isinstance(provides, NativeArtifact):
+    if ctypes_dylib and not isinstance(ctypes_dylib, NativeArtifact):
       raise TargetDefinitionException(
         "Target must provide a valid pants '{}' object. Received an object with type '{}' "
         "and value: {}."
-        .format(NativeArtifact.alias(), type(provides).__name__, provides))
+        .format(NativeArtifact.alias(), type(ctypes_dylib).__name__, ctypes_dylib))
 
     super(NativeLibrary, self).__init__(address=address, payload=payload, **kwargs)
 
@@ -58,5 +49,30 @@ def fatal_warnings(self):
     return self.payload.fatal_warnings
 
   @property
-  def provides(self):
-    return self.payload.provides
+  def ctypes_dylib(self):
+    return self.payload.ctypes_dylib
+
+
+class CLibrary(NativeLibrary):
+
+  default_sources_globs = [
+    '*.h',
+    '*.c',
+  ]
+
+  @classmethod
+  def alias(cls):
+    return 'ctypes_compatible_c_library'
+
+
+class CppLibrary(NativeLibrary):
+
+  default_sources_globs = [
+    '*.h',
+    '*.hpp',
+    '*.cpp',
+  ]
+
+  @classmethod
+  def alias(cls):
+    return 'ctypes_compatible_cpp_library'

src/python/pants/backend/native/tasks/BUILD

@@ -12,6 +12,7 @@ python_library(
     'src/python/pants/base:workunit',
     'src/python/pants/build_graph',
     'src/python/pants/task',
+    'src/python/pants/util:collections',
     'src/python/pants/util:contextutil',
     'src/python/pants/util:memo',
     'src/python/pants/util:objects',