Add toolchain_variant on native target level (pantsbuild#7179)

Resolves pantsbuild#7122

pants.ini

@@ -415,7 +415,7 @@ compiler_option_sets_enabled_args: {
 
 # TODO(#6848, #7122): A gcc toolchain is currently required to build the tensorflow_custom_op example. This
 # should be removed when we can fully support both toolchains, and select between them with constraints.
-[native-build-settings]
+[native-build-step]
 toolchain_variant: gnu
 
 [libc]

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

@@ -7,7 +7,6 @@
 from pants.backend.native.subsystems.utils.mirrored_target_option_mixin import \
   MirroredTargetOptionMixin
 from pants.subsystem.subsystem import Subsystem
-from pants.util.memo import memoized_property
 from pants.util.objects import enum
 
 
@@ -37,15 +36,5 @@ def register_options(cls, register):
                   "are used when compiling and linking native code. C and C++ targets may override "
                   "this behavior with the strict_deps keyword argument as well.")
 
-    register('--toolchain-variant', type=str, fingerprint=True, advanced=True,
-             choices=ToolchainVariant.allowed_values,
-             default=ToolchainVariant.default_value,
-             help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
-                  "linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")
-
   def get_strict_deps_value_for_target(self, target):
     return self.get_target_mirrored_option('strict_deps', target)
-
-  @memoized_property
-  def toolchain_variant(self):
-    return ToolchainVariant.create(self.get_options().toolchain_variant)

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

@@ -4,6 +4,7 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+from pants.backend.native.subsystems.native_build_settings import ToolchainVariant
 from pants.backend.native.subsystems.utils.mirrored_target_option_mixin import \
   MirroredTargetOptionMixin
 from pants.option.compiler_option_sets_mixin import CompilerOptionSetsMixin
@@ -19,6 +20,7 @@ class NativeBuildStepSettings(CompilerOptionSetsMixin, MirroredTargetOptionMixin
 
   mirrored_option_to_kwarg_map = {
     'compiler_option_sets': 'compiler_option_sets',
+    'toolchain_variant': 'toolchain_variant'
   }
 
   @classmethod
@@ -30,9 +32,18 @@ def register_options(cls, register):
              help='The default for the "compiler_option_sets" argument '
                   'for targets of this language.')
 
+    register('--toolchain-variant', type=str, fingerprint=True, advanced=True,
+             choices=ToolchainVariant.allowed_values,
+             default=ToolchainVariant.default_value,
+             help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
+                  "linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")
+
   def get_compiler_option_sets_for_target(self, target):
     return self.get_target_mirrored_option('compiler_option_sets', target)
 
+  def get_toolchain_variant_for_target(self, target):
+    return ToolchainVariant.create(self.get_target_mirrored_option('toolchain_variant', target))
+
   @classproperty
   def get_compiler_option_sets_enabled_default_value(cls):
     return {"fatal_warnings": ["-Werror"]}

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

@@ -4,6 +4,7 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+from pants.backend.native.subsystems.native_build_settings import ToolchainVariant
 from pants.backend.native.targets.native_artifact import NativeArtifact
 from pants.base.exceptions import TargetDefinitionException
 from pants.base.payload import Payload
@@ -22,6 +23,7 @@ def produces_ctypes_native_library(cls, target):
 
   def __init__(self, address, payload=None, sources=None, ctypes_native_library=None,
                strict_deps=None, fatal_warnings=None, compiler_option_sets=None,
+               toolchain_variant=None,
                **kwargs):
 
     if not payload:
@@ -33,6 +35,7 @@ def __init__(self, address, payload=None, sources=None, ctypes_native_library=No
       'strict_deps': PrimitiveField(strict_deps),
       'fatal_warnings': PrimitiveField(fatal_warnings),
       'compiler_option_sets': PrimitivesSetField(compiler_option_sets),
+      'toolchain_variant': PrimitiveField(toolchain_variant)
     })
 
     if ctypes_native_library and not isinstance(ctypes_native_library, NativeArtifact):
@@ -43,6 +46,13 @@ def __init__(self, address, payload=None, sources=None, ctypes_native_library=No
 
     super(NativeLibrary, self).__init__(address=address, payload=payload, **kwargs)
 
+  @property
+  def toolchain_variant(self):
+    if not self.payload.toolchain_variant:
+      return None
+
+    return ToolchainVariant.create(self.payload.toolchain_variant)
+
   @property
   def strict_deps(self):
     return self.payload.strict_deps

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

@@ -30,5 +30,5 @@ def subsystem_dependencies(cls):
   def get_compile_settings(self):
     return CCompileSettings.scoped_instance(self)
 
-  def get_compiler(self):
-    return self.get_c_toolchain_variant().c_compiler
+  def get_compiler(self, native_library_target):
+    return self.get_c_toolchain_variant(native_library_target).c_compiler

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

@@ -30,5 +30,5 @@ def subsystem_dependencies(cls):
   def get_compile_settings(self):
     return CppCompileSettings.scoped_instance(self)
 
-  def get_compiler(self):
-    return self.get_cpp_toolchain_variant().cpp_compiler
+  def get_compiler(self, native_library_target):
+    return self.get_cpp_toolchain_variant(native_library_target).cpp_compiler

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

@@ -56,11 +56,10 @@ def implementation_version(cls):
 
   class LinkSharedLibrariesError(TaskError): pass
 
-  @memoized_property
-  def linker(self):
+  def linker(self, native_library_target):
     # NB: we are using the C++ toolchain here for linking every type of input, including compiled C
     # source files.
-    return self.get_cpp_toolchain_variant().cpp_linker
+    return self.get_cpp_toolchain_variant(native_library_target).cpp_linker
 
   @memoized_property
   def platform(self):
@@ -132,7 +131,7 @@ def _make_link_request(self, vt, compiled_objects_product):
       external_lib_names.extend(ext_dep.native_lib_names)
 
     link_request = LinkSharedLibraryRequest(
-      linker=self.linker,
+      linker=self.linker(vt.target),
       object_files=tuple(all_compiled_object_files),
       native_artifact=vt.target.ctypes_native_library,
       output_dir=vt.results_dir,

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

@@ -133,17 +133,16 @@ def _compile_settings(self):
     return self.get_compile_settings()
 
   @abstractmethod
-  def get_compiler(self):
+  def get_compiler(self, native_library_target):
     """An instance of `Executable` which can be invoked to compile files.
 
     NB: Subclasses will be queried for the compiler instance once and the result cached.
 
     :return: :class:`pants.backend.native.config.environment.Executable`
     """
 
-  @memoized_property
-  def _compiler(self):
-    return self.get_compiler()
+  def _compiler(self, native_library_target):
+    return self.get_compiler(native_library_target)
 
   def _make_compile_request(self, versioned_target):
     target = versioned_target.target
@@ -166,7 +165,7 @@ def _make_compile_request(self, versioned_target):
                                 .get_compiler_option_sets_for_target(target))
 
     compile_request = NativeCompileRequest(
-      compiler=self._compiler,
+      compiler=self._compiler(target),
       include_dirs=include_dirs,
       sources=sources_and_headers,
       compiler_options=(self._compile_settings

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

@@ -8,6 +8,7 @@
 
 from pants.backend.native.config.environment import CppToolchain, CToolchain
 from pants.backend.native.subsystems.native_build_settings import NativeBuildSettings
+from pants.backend.native.subsystems.native_build_step_settings import NativeBuildStepSettings
 from pants.backend.native.subsystems.native_toolchain import (NativeToolchain,
                                                               ToolchainVariantRequest)
 from pants.backend.native.targets.native_library import NativeLibrary
@@ -78,23 +79,30 @@ def implementation_version(cls):
   def _native_build_settings(self):
     return NativeBuildSettings.global_instance()
 
+  # TODO(#7183): remove this global subsystem dependency!
+  @memoized_property
+  def _native_build_step_settings(self):
+    return NativeBuildStepSettings.global_instance()
+
   @memoized_property
   def _native_toolchain(self):
     return NativeToolchain.scoped_instance(self)
 
-  @memoized_property
-  def _toolchain_variant_request(self):
+  def _toolchain_variant_request(self, variant):
     return ToolchainVariantRequest(
       toolchain=self._native_toolchain,
-      variant=self._native_build_settings.toolchain_variant)
+      variant=variant)
 
-  @memoized_method
-  def get_c_toolchain_variant(self):
-    return self._request_single(CToolchain, self._toolchain_variant_request)
+  def get_c_toolchain_variant(self, native_library_target):
+    return self._get_toolchain_variant(CToolchain, native_library_target)
 
-  @memoized_method
-  def get_cpp_toolchain_variant(self):
-    return self._request_single(CppToolchain, self._toolchain_variant_request)
+  def get_cpp_toolchain_variant(self, native_library_target):
+    return self._get_toolchain_variant(CppToolchain, native_library_target)
+
+  def _get_toolchain_variant(self, toolchain_type, native_library_target):
+    selected_variant = self._native_build_step_settings.get_toolchain_variant_for_target(
+      native_library_target)
+    return self._request_single(toolchain_type, self._toolchain_variant_request(selected_variant))
 
   @memoized_method
   def native_deps(self, target):

tests/python/pants_test/backend/native/tasks/test_cpp_compile.py

@@ -6,7 +6,7 @@
 
 from textwrap import dedent
 
-from pants.backend.native.targets.native_library import CppLibrary
+from pants.backend.native.targets.native_library import CppLibrary, NativeLibrary
 from pants.backend.native.tasks.cpp_compile import CppCompile
 from pants.backend.native.tasks.native_compile import ObjectFiles
 from pants_test.backend.native.tasks.native_task_test_base import (NativeCompileTestMixin,
@@ -59,3 +59,37 @@ def test_caching(self):
     # TODO: what is this testing?
     cpp_compile.execute()
     cpp_compile.execute()
+
+  def test_target_level_toolchain_variant_llvm(self):
+    self.set_options_for_scope('native-build-step', toolchain_variant='gnu')
+    cpp_lib_target = self.make_target(
+      '//:cpp_library',
+      NativeLibrary,
+      toolchain_variant='llvm'
+    )
+
+    task = self.create_task(self.context(target_roots=[cpp_lib_target]))
+    compiler = task.get_compiler(cpp_lib_target)
+    self.assertIn('llvm', compiler.path_entries[0])
+
+  def test_target_level_toolchain_variant_default_llvm(self):
+    self.set_options_for_scope('native-build-step', toolchain_variant='llvm')
+    cpp_lib_target = self.make_target(
+      '//:cpp_library',
+      NativeLibrary,
+    )
+
+    task = self.create_task(self.context(target_roots=[cpp_lib_target]))
+    compiler = task.get_compiler(cpp_lib_target)
+    self.assertIn('llvm', compiler.path_entries[0])
+
+  def test_target_level_toolchain_variant_default_gnu(self):
+    self.set_options_for_scope('native-build-step', toolchain_variant='gnu')
+    cpp_lib_target = self.make_target(
+      '//:cpp_library',
+      NativeLibrary,
+    )
+
+    task = self.create_task(self.context(target_roots=[cpp_lib_target]))
+    compiler = task.get_compiler(cpp_lib_target)
+    self.assertIn('gcc', compiler.path_entries[0])

tests/python/pants_test/backend/python/tasks/native/test_ctypes_integration.py

@@ -62,7 +62,7 @@ def _assert_ctypes_binary_creation(self, toolchain_variant):
         GLOBAL_SCOPE_CONFIG_SECTION: {
           'pants_distdir': tmp_dir,
         },
-        'native-build-settings': {
+        'native-build-step': {
           'toolchain_variant': toolchain_variant,
         },
       })
@@ -137,9 +137,11 @@ def _assert_ctypes_interop_with_mock_buildroot(self, toolchain_variant):
         command=['binary', self._binary_target_with_interop],
         # Explicitly set to True (although this is the default).
         config={
+          'native-build-step': {
+            'toolchain_variant': toolchain_variant,
+          },
           # TODO(#6848): don't make it possible to forget to add the toolchain_variant option!
           'native-build-settings': {
-            'toolchain_variant': toolchain_variant,
             'strict_deps': True,
           },
         },
@@ -157,9 +159,11 @@ def _assert_ctypes_interop_with_mock_buildroot(self, toolchain_variant):
     })
     if attempt_pants_run:
       pants_run_interop = self.run_pants(['-q', 'run', self._binary_target_with_interop], config={
-        'native-build-settings': {
+        'native-build-step': {
           'toolchain_variant': toolchain_variant,
-          'strict_deps': False,
+        },
+        'native-build-settings': {
+          'strict_deps': True,
         },
       })
       self.assert_success(pants_run_interop)
@@ -171,7 +175,7 @@ def test_ctypes_third_party_integration(self):
 
   def _assert_ctypes_third_party_integration(self, toolchain_variant):
     pants_binary = self.run_pants(['binary', self._binary_target_with_third_party], config={
-      'native-build-settings': {
+      'native-build-step': {
         'toolchain_variant': toolchain_variant,
       },
     })
@@ -185,7 +189,7 @@ def _assert_ctypes_third_party_integration(self, toolchain_variant):
     })
     if attempt_pants_run:
       pants_run = self.run_pants(['-q', 'run', self._binary_target_with_third_party], config={
-        'native-build-settings': {
+        'native-build-step': {
           'toolchain_variant': toolchain_variant,
         },
       })
@@ -206,7 +210,7 @@ def test_pants_native_source_detection_for_local_ctypes_dists_for_current_platfo
     # TODO(#6848): we need to provide the libstdc++.so.6.dylib which comes with gcc on osx in the
     # DYLD_LIBRARY_PATH during the 'run' goal somehow.
     pants_run = self.run_pants(command=command, config={
-      'native-build-settings': {
+      'native-build-step': {
         'toolchain_variant': 'llvm',
       },
       'python-setup': {
@@ -231,10 +235,10 @@ def _assert_native_compiler_option_sets_integration(self, toolchain_variant):
       self._binary_target_with_compiler_option_sets
     ]
     pants_run = self.run_pants(command=command, config={
-      'native-build-settings': {
+      'native-build-step': {
         'toolchain_variant': toolchain_variant,
       },
-      'native-build-step.cpp-compile-settings': {
+      'native-build-settings.cpp-compile-settings': {
         'compiler_option_sets_enabled_args': {
           'asdf': ['-D_ASDF=1'],
         },

tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py

@@ -149,7 +149,7 @@ def _create_distribution_synthetic_target(self, python_dist_target, extra_target
       for_subsystems=[PythonRepos, LibcDev],
       # TODO(#6848): we should be testing all of these with both of our toolchains.
       options={
-        'native-build-settings': {
+        'native-build-step': {
           'toolchain_variant': 'llvm',
         },
       })