Merge pull request RobotLocomotion#7478 from jwnimmer-tri/6996-privat…

…e-library

Partition private headers into their own library

tools/skylark/drake_cc.bzl

@@ -219,29 +219,23 @@ def drake_transitive_installed_hdrs_filegroup(name, deps = [], **kwargs):
         **kwargs
     )
 
-def drake_cc_library(
+def _raw_drake_cc_library(
         name,
         hdrs = [],
-        srcs = [],
+        srcs = [],  # Cannot list any headers here.
         deps = [],
-        copts = [],
-        gcc_copts = [],
-        linkstatic = 1,
+        declare_installed_headers = 0,
         install_hdrs_exclude = [],
         **kwargs):
-    """Creates a rule to declare a C++ library.
-
-    By default, we produce only static libraries, to reduce compilation time
-    on all platforms, and to avoid mysterious dyld errors on OS X. This default
-    could be revisited if binary size becomes a concern.
-
-    The deps= of a drake_cc_library must either be another drake_cc_library, or
-    be named like "@something//etc..." (i.e., come from the workspace, not part
-    of Drake).  In other words, all of Drake's C++ libraries must be declared
-    using the drake_cc_library macro.
+    """Creates a rule to declare a C++ library.  Uses Drake's include_prefix and
+    checks the deps blacklist.  If declare_installed_headers is true, also adds
+    a drake_installed_headers() target.  (This should be set if and only if the
+    caller is drake_cc_library.)
     """
-    _, private_hdrs = _prune_private_hdrs(srcs)
     _check_library_deps_blacklist(name, deps)
+    _, private_hdrs = _prune_private_hdrs(srcs)
+    if private_hdrs:
+        fail("private_hdrs = " + private_hdrs)
     if native.package_name().startswith("drake"):
         strip_include_prefix = None
         include_prefix = None
@@ -254,25 +248,96 @@ def drake_cc_library(
         hdrs = hdrs,
         srcs = srcs,
         deps = deps,
-        copts = _platform_copts(copts, gcc_copts),
-        linkstatic = linkstatic,
         strip_include_prefix = strip_include_prefix,
         include_prefix = include_prefix,
         **kwargs)
+    if declare_installed_headers:
+        drake_installed_headers(
+            name = name + ".installed_headers",
+            hdrs = hdrs,
+            hdrs_exclude = install_hdrs_exclude,
+            deps = installed_headers_for_drake_deps(deps),
+            tags = ["nolint"],
+            visibility = ["//visibility:public"],
+        )
+
+def _maybe_add_pruned_private_hdrs_dep(
+        base_name,
+        srcs,
+        deps,
+        **kwargs):
+    """Given some srcs, prunes any header files into a separate cc_library, and
+    appends that new library to deps, returning new_srcs (sans headers) and
+    new_deps.  The separate cc_library is private with linkstatic = 1.
+
+    We use this helper in all drake_cc_{library,binary,test) because when we
+    want to fiddle with include paths, we *must* have all header files listed
+    as hdrs; the include_prefix does not apply to srcs.
+    """
+    new_srcs, private_hdrs = _prune_private_hdrs(srcs)
+    if private_hdrs:
+        name = "_" + base_name + "_private_headers_impl"
+        kwargs.pop('linkshared', '')
+        kwargs.pop('linkstatic', '')
+        kwargs.pop('visibility', '')
+        _raw_drake_cc_library(
+            name = name,
+            hdrs = private_hdrs,
+            srcs = [],
+            deps = deps,
+            linkstatic = 1,
+            visibility = ["//visibility:private"],
+            **kwargs)
+        new_deps = deps + [":" + name]
+    else:
+        new_deps = deps
+    return new_srcs, new_deps
+
+def drake_cc_library(
+        name,
+        hdrs = [],
+        srcs = [],
+        deps = [],
+        copts = [],
+        gcc_copts = [],
+        linkstatic = 1,
+        install_hdrs_exclude = [],
+        **kwargs):
+    """Creates a rule to declare a C++ library.
+
+    By default, we produce only static libraries, to reduce compilation time
+    on all platforms, and to avoid mysterious dyld errors on OS X. This default
+    could be revisited if binary size becomes a concern.
+
+    The deps= of a drake_cc_library must either be another drake_cc_library, or
+    be named like "@something//etc..." (i.e., come from the workspace, not part
+    of Drake).  In other words, all of Drake's C++ libraries must be declared
+    using the drake_cc_library macro.
+    """
+    new_copts = _platform_copts(copts, gcc_copts)
     # We install private_hdrs by default, because Bazel's visibility denotes
     # whether headers can be *directly* included when using cc_library; it does
     # not precisely relate to which headers should appear in the install tree.
     # For example, common/symbolic.h is the only public-visibility header for
     # its cc_library, but we also need to install all of its child headers that
     # it includes, such as common/symbolic_expression.h.
-    drake_installed_headers(
-        name = name + ".installed_headers",
-        hdrs = hdrs + private_hdrs,
-        hdrs_exclude = install_hdrs_exclude,
-        deps = installed_headers_for_drake_deps(deps),
-        tags = ["nolint"],
-        visibility = ["//visibility:public"],
-    )
+    new_srcs, new_deps = _maybe_add_pruned_private_hdrs_dep(
+        base_name = name,
+        srcs = srcs,
+        deps = deps,
+        copts = new_copts,
+        declare_installed_headers = 1,
+        **kwargs)
+    _raw_drake_cc_library(
+        name = name,
+        hdrs = hdrs,
+        srcs = new_srcs,
+        deps = new_deps,
+        copts = new_copts,
+        linkstatic = linkstatic,
+        declare_installed_headers = 1,
+        install_hdrs_exclude = install_hdrs_exclude,
+        **kwargs)
 
 def drake_cc_binary(
         name,
@@ -300,12 +365,20 @@ def drake_cc_binary(
     tests. The smoke-test will be named <name>_test. You may override cc_test
     defaults using test_rule_args=["-f", "--bar=42"] or test_rule_size="baz".
     """
+    new_copts = _platform_copts(copts, gcc_copts)
+    new_srcs, new_deps = _maybe_add_pruned_private_hdrs_dep(
+        base_name = name,
+        srcs = srcs,
+        deps = deps,
+        copts = new_copts,
+        testonly = testonly,
+        **kwargs)
     native.cc_binary(
         name = name,
-        srcs = srcs,
+        srcs = new_srcs,
         data = data,
-        deps = deps,
-        copts = _platform_copts(copts, gcc_copts),
+        deps = new_deps,
+        copts = new_copts,
         testonly = testonly,
         linkstatic = linkstatic,
         **kwargs)
@@ -328,13 +401,13 @@ def drake_cc_binary(
     if add_test_rule:
         drake_cc_test(
             name = name + "_test",
-            srcs = srcs,
+            srcs = new_srcs,
             data = data + test_rule_data,
-            deps = deps,
+            deps = new_deps,
             copts = copts,
+            gcc_copts = gcc_copts,
             size = test_rule_size,
             flaky = test_rule_flaky,
-            testonly = testonly,
             linkstatic = linkstatic,
             args = test_rule_args,
             **kwargs)
@@ -343,6 +416,7 @@ def drake_cc_test(
         name,
         size = None,
         srcs = [],
+        deps = [],
         copts = [],
         gcc_copts = [],
         disable_in_compilation_mode_dbg = False,
@@ -352,6 +426,7 @@ def drake_cc_test(
 
     By default, sets size="small" because that indicates a unit test.
     By default, sets name="test/${name}.cc" per Drake's filename convention.
+    Unconditionally forces testonly=1.
 
     If disable_in_compilation_mode_dbg is True, the srcs will be suppressed
     in debug-mode builds, so the test will trivially pass. This option should
@@ -361,18 +436,27 @@ def drake_cc_test(
         size = "small"
     if not srcs:
         srcs = ["test/%s.cc" % name]
+    kwargs['testonly'] = 1
+    new_copts = _platform_copts(copts, gcc_copts, cc_test = 1)
+    new_srcs, new_deps = _maybe_add_pruned_private_hdrs_dep(
+        base_name = name,
+        srcs = srcs,
+        deps = deps,
+        copts = new_copts,
+        **kwargs)
     if disable_in_compilation_mode_dbg:
         # Remove the test declarations from the test in debug mode.
         # TODO(david-german-tri): Actually suppress the test rule.
-        srcs = select({
+        new_srcs = select({
             "//tools/cc_toolchain:debug": [],
-            "//conditions:default": srcs,
+            "//conditions:default": new_srcs,
         })
     native.cc_test(
         name = name,
         size = size,
-        srcs = srcs,
-        copts = _platform_copts(copts, gcc_copts, cc_test = 1),
+        srcs = new_srcs,
+        deps = new_deps,
+        copts = new_copts,
         **kwargs)
 
     # Also generate the OS X debug symbol file for this test.
@@ -381,7 +465,7 @@ def drake_cc_test(
         srcs = [":" + name],
         outs = [name + ".dSYM"],
         output_to_bindir = 1,
-        testonly = 1,
+        testonly = kwargs['testonly'],
         tags = ["dsym"],
         visibility = ["//visibility:private"],
         cmd = _dsym_command(name),