Header file extensions as options for C/C++ targets (#6802)

### Problem

The file extensions we consider "header files" for C/C++ targets are hardcoded as `('.h', '.hpp')`. Additionally, we will error out on header-only `ctypes_compatible_cpp_library()` targets (as for C targets).

### Solution

- Create the `--header-file-extensions` option in the CCompileSettings and CppCompileSettings subsystems.
- Make a debug-level log if the library undergoing compile is a header-only library before early returning.
- Add some tests.

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

@@ -37,32 +37,44 @@ def get_compiler_option_sets_enabled_default_value(cls):
     return {"fatal_warnings": ["-Werror"]}
 
 
-class CCompileSettings(Subsystem):
-  options_scope = 'c-compile-settings'
+class CompileSettingsBase(Subsystem):
 
   @classmethod
   def subsystem_dependencies(cls):
-    return super(CCompileSettings, cls).subsystem_dependencies() + (
+    return super(CompileSettingsBase, cls).subsystem_dependencies() + (
       NativeBuildStepSettings.scoped(cls),
     )
 
+  @classproperty
+  def header_file_extensions_default(cls):
+    raise NotImplementedError('header_file_extensions_default() must be overridden!')
+
+  @classmethod
+  def register_options(cls, register):
+    super(CompileSettingsBase, cls).register_options(register)
+    register('--header-file-extensions', advanced=True, default=cls.header_file_extensions_default,
+             type=list, fingerprint=True,
+             help="The file extensions which should not be provided to the compiler command line.")
+
   @memoized_property
   def native_build_step_settings(self):
     return NativeBuildStepSettings.scoped_instance(self)
 
+  @memoized_property
+  def header_file_extensions(self):
+    return self.get_options().header_file_extensions
 
-class CppCompileSettings(Subsystem):
-  options_scope = 'cpp-compile-settings'
 
-  @classmethod
-  def subsystem_dependencies(cls):
-    return super(CppCompileSettings, cls).subsystem_dependencies() + (
-      NativeBuildStepSettings.scoped(cls),
-    )
+class CCompileSettings(CompileSettingsBase):
+  options_scope = 'c-compile-settings'
 
-  @memoized_property
-  def native_build_step_settings(self):
-    return NativeBuildStepSettings.scoped_instance(self)
+  header_file_extensions_default = ['.h']
+
+
+class CppCompileSettings(CompileSettingsBase):
+  options_scope = 'cpp-compile-settings'
+
+  header_file_extensions_default = ['.h', '.hpp', '.hxx', '.tpp']
 
 
 # TODO: add a fatal_warnings kwarg to NativeArtifact and make a LinkSharedLibrariesSettings subclass

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

@@ -29,6 +29,7 @@ class NativeCompileRequest(datatype([
     'sources',
     'compiler_options',
     'output_dir',
+    'header_file_extensions',
 ])): pass
 
 
@@ -47,8 +48,6 @@ class NativeCompile(NativeTask, AbstractClass):
   source_target_constraint = None
   dependent_target_constraint = SubclassesOf(ExternalNativeLibrary, NativeLibrary)
 
-  HEADER_EXTENSIONS = ('.h', '.hpp')
-
   # `NativeCompile` will use `workunit_label` as the name of the workunit when executing the
   # compiler process. `workunit_label` must be set to a string.
   @classproperty
@@ -181,10 +180,23 @@ def _make_compile_request(self, versioned_target, dependencies, external_libs_pr
       compiler_options=(self._compile_settings
                             .native_build_step_settings
                             .get_merged_args_for_compiler_option_sets(compiler_option_sets)),
-      output_dir=versioned_target.results_dir)
+      output_dir=versioned_target.results_dir,
+      header_file_extensions=self._compile_settings.header_file_extensions)
+
+  def _iter_sources_minus_headers(self, compile_request):
+    for s in compile_request.sources:
+      if not s.endswith(tuple(compile_request.header_file_extensions)):
+        yield s
+
+  class _HeaderOnlyLibrary(Exception): pass
 
   def _make_compile_argv(self, compile_request):
     """Return a list of arguments to use to compile sources. Subclasses can override and append."""
+
+    sources_minus_headers = list(self._iter_sources_minus_headers(compile_request))
+    if len(sources_minus_headers) == 0:
+      raise self._HeaderOnlyLibrary()
+
     compiler = compile_request.compiler
     compiler_options = compile_request.compiler_options
     # We are going to execute in the target output, so get absolute paths for everything.
@@ -199,7 +211,7 @@ def _make_compile_argv(self, compile_request):
         '-I{}'.format(os.path.join(buildroot, inc_dir))
         for inc_dir in compile_request.include_dirs
       ] +
-      [os.path.join(buildroot, src) for src in compile_request.sources])
+      [os.path.join(buildroot, src) for src in sources_minus_headers])
 
     self.context.log.debug("compile argv: {}".format(argv))
 
@@ -211,18 +223,15 @@ def _compile(self, compile_request):
     NB: This method must arrange the output files so that `collect_cached_objects()` can collect all
     of the results (or vice versa)!
     """
-    sources = compile_request.sources
 
-    if len(sources) == 0 and not any(s for s in sources if not s.endswith(self.HEADER_EXTENSIONS)):
-      # TODO: do we need this log message? Should we still have it for intentionally header-only
-      # libraries (that might be a confusing message to see)?
-      self.context.log.debug("no sources in request {}, skipping".format(compile_request))
+    try:
+      argv = self._make_compile_argv(compile_request)
+    except self._HeaderOnlyLibrary:
+      self.context.log.debug('{} is a header-only library'.format(compile_request))
       return
 
     compiler = compile_request.compiler
     output_dir = compile_request.output_dir
-
-    argv = self._make_compile_argv(compile_request)
     env = compiler.as_invocation_environment_dict
 
     with self.context.new_workunit(

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

@@ -20,6 +20,17 @@ def rules(cls):
 
 
 class NativeCompileTestMixin(object):
+
+  def _retrieve_single_product_at_target_base(self, product_mapping, target):
+    product = product_mapping.get(target)
+    base_dirs = list(product.keys())
+    self.assertEqual(1, len(base_dirs))
+    single_base_dir = base_dirs[0]
+    all_products = product[single_base_dir]
+    self.assertEqual(1, len(all_products))
+    single_product = all_products[0]
+    return single_product
+
   def create_simple_cpp_library(self, **kwargs):
     self.create_file('src/cpp/test/test.hpp', contents=dedent("""
       #ifndef __TEST_HPP__

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

@@ -8,6 +8,7 @@
 
 from pants.backend.native.targets.native_library import CLibrary
 from pants.backend.native.tasks.c_compile import CCompile
+from pants.backend.native.tasks.native_compile import ObjectFiles
 from pants_test.backend.native.tasks.native_task_test_base import (NativeCompileTestMixin,
                                                                    NativeTaskTestBase)
 
@@ -17,6 +18,21 @@ class CCompileTest(NativeTaskTestBase, NativeCompileTestMixin):
   def task_type(cls):
     return CCompile
 
+  def create_header_only_alternate_c_library(self, ext, **kwargs):
+    header_filename = 'test{}'.format(ext)
+    self.create_file('src/c/test/{}'.format(header_filename), contents=dedent("""
+      #ifndef __TEST_H__
+      #define __TEST_H__
+
+      int test(int);
+
+      #endif
+"""))
+    return self.make_target(spec='src/c/test',
+                            target_type=CLibrary,
+                            sources=[header_filename],
+                            **kwargs)
+
   def create_simple_c_library(self, **kwargs):
     self.create_file('src/c/test/test.h', contents=dedent("""
       #ifndef __TEST_H__
@@ -38,10 +54,29 @@ def create_simple_c_library(self, **kwargs):
                             sources=['test.h', 'test.c'],
                             **kwargs)
 
+  def test_header_only_noop_with_alternate_header_extension(self):
+    alternate_extension = '.asdf'
+    c = self.create_header_only_alternate_c_library(alternate_extension)
+    context = self.prepare_context_for_compile(target_roots=[c], options={
+      'c-compile-settings': {
+        'header_file_extensions': [alternate_extension],
+      },
+    })
+
+    # Test that the task runs without error if provided a header-only library.
+    c_compile = self.create_task(context)
+    c_compile.execute()
+
+    object_files_product = context.products.get(ObjectFiles)
+    object_files_for_target = self._retrieve_single_product_at_target_base(object_files_product, c)
+    # Test that no object files were produced.
+    self.assertEqual(0, len(object_files_for_target.filenames))
+
   def test_caching(self):
     c = self.create_simple_c_library()
     context = self.prepare_context_for_compile(target_roots=[c])
     c_compile = self.create_task(context)
 
+    # TODO: what is this testing?
     c_compile.execute()
     c_compile.execute()

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

@@ -4,7 +4,11 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+from textwrap import dedent
+
+from pants.backend.native.targets.native_library import CppLibrary
 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,
                                                                    NativeTaskTestBase)
 
@@ -14,10 +18,44 @@ class CppCompileTest(NativeTaskTestBase, NativeCompileTestMixin):
   def task_type(cls):
     return CppCompile
 
+  def create_simple_header_only_library(self, **kwargs):
+    # TODO: determine if there are other features that people expect from header-only libraries that
+    # we could be testing here. Currently this file's contents are just C++ which doesn't define any
+    # non-template classes or methods.
+    self.create_file('src/cpp/test/test.hpp', contents=dedent("""
+      #ifndef __TEST_HPP__
+      #define __TEST_HPP__
+
+      template <typename T>
+      T add(T a, T b) {
+        return a + b;
+      }
+
+      #endif
+"""))
+    return self.make_target(spec='src/cpp/test',
+                     target_type=CppLibrary,
+                     sources=['test.hpp'],
+                     **kwargs)
+
+  def test_header_only_target_noop(self):
+    cpp = self.create_simple_header_only_library()
+    context = self.prepare_context_for_compile(target_roots=[cpp])
+
+    # Test that the task runs without error if provided a header-only library.
+    cpp_compile = self.create_task(context)
+    cpp_compile.execute()
+
+    object_files_product = context.products.get(ObjectFiles)
+    object_files_for_target = self._retrieve_single_product_at_target_base(object_files_product, cpp)
+    # Test that no object files were produced.
+    self.assertEqual(0, len(object_files_for_target.filenames))
+
   def test_caching(self):
     cpp = self.create_simple_cpp_library()
     context = self.prepare_context_for_compile(target_roots=[cpp])
     cpp_compile = self.create_task(context)
 
+    # TODO: what is this testing?
     cpp_compile.execute()
     cpp_compile.execute()