Add deps attribute to flatbuffer_cc_library (#7107)

I'm not seeing the reason why we didn't attempt to support transitive
dependencies for flatbuffer_cc_library, and the current setup makes
having to propagate new dependencies to all of their recursive
dependents obnoxious.

build_defs.bzl

@@ -37,6 +37,7 @@ def flatbuffer_library_public(
         reflection_visibility = None,
         compatible_with = None,
         restricted_to = None,
+        target_compatible_with = None,
         output_to_bindir = False):
     """Generates code files for reading/writing the given flatbuffers in the requested language using the public compiler.
 
@@ -58,6 +59,8 @@ def flatbuffer_library_public(
         built for, in addition to default-supported environments.
       restricted_to: Optional, The list of environments this rule can be built
         for, instead of default-supported environments.
+      target_compatible_with: Optional, The list of target platform constraints
+        to use.
       output_to_bindir: Passed to genrule for output to bin directory.
 
 
@@ -92,6 +95,7 @@ def flatbuffer_library_public(
         tools = [flatc_path],
         cmd = genrule_cmd,
         compatible_with = compatible_with,
+        target_compatible_with = target_compatible_with,
         restricted_to = restricted_to,
         message = "Generating flatbuffer files for %s:" % (name),
     )
@@ -120,8 +124,10 @@ def flatbuffer_library_public(
             tools = [flatc_path],
             compatible_with = compatible_with,
             restricted_to = restricted_to,
+            target_compatible_with = target_compatible_with,
             cmd = reflection_genrule_cmd,
             message = "Generating flatbuffer reflection binary for %s:" % (name),
+            visibility = reflection_visibility,
         )
         native.filegroup(
             name = "%s_out" % reflection_name,
@@ -136,15 +142,18 @@ def flatbuffer_cc_library(
         srcs,
         srcs_filegroup_name = "",
         out_prefix = "",
+        deps = [],
         includes = [],
         include_paths = DEFAULT_INCLUDE_PATHS,
+        cc_include_paths = [],
         flatc_args = DEFAULT_FLATC_ARGS,
         visibility = None,
         compatible_with = None,
         restricted_to = None,
+        target_compatible_with = None,
         srcs_filegroup_visibility = None,
         gen_reflections = False):
-    '''A cc_library with the generated reader/writers for the given flatbuffer definitions.
+    """A cc_library with the generated reader/writers for the given flatbuffer definitions.
 
     Args:
       name: Rule name.
@@ -154,9 +163,12 @@ def flatbuffer_cc_library(
           flatbuffer_cc_library that depends on this one's schemas.
       out_prefix: Prepend this path to the front of all generated files. Usually
           is a directory name.
+      deps: Optional, list of other flatbuffer_cc_library's to depend on. Cannot be specified
+          alongside includes.
       includes: Optional, list of filegroups of schemas that the srcs depend on.
-          ** SEE REMARKS BELOW **
+          Use of this is discouraged, and may be deprecated.
       include_paths: Optional, list of paths the includes files can be found in.
+      cc_include_paths: Optional, list of paths to add to the cc_library includes attribute.
       flatc_args: Optional list of additional arguments to pass to flatc
           (e.g. --gen-mutable).
       visibility: The visibility of the generated cc_library. By default, use the
@@ -169,6 +181,8 @@ def flatbuffer_cc_library(
         for, in addition to default-supported environments.
       restricted_to: Optional, The list of environments this rule can be built
         for, instead of default-supported environments.
+      target_compatible_with: Optional, The list of target platform constraints
+        to use.
 
     This produces:
       filegroup([name]_srcs): all generated .h files.
@@ -177,41 +191,17 @@ def flatbuffer_cc_library(
           parameter, if they depend on the schemas in this library.
       Fileset([name]_reflection): (Optional) all generated reflection binaries.
       cc_library([name]): library with sources and flatbuffers deps.
-
+    """
-    Remarks:
-      ** Because the genrule used to call flatc does not have any trivial way of
-        computing the output list of files transitively generated by includes and
-        --gen-includes (the default) being defined for flatc, the --gen-includes
-        flag will not work as expected. The way around this is to add a dependency
-        to the flatbuffer_cc_library defined alongside the flatc included Fileset.
-        For example you might define:
-
-        flatbuffer_cc_library(
-            name = "my_fbs",
-            srcs = [ "schemas/foo.fbs" ],
-            includes = [ "//third_party/bazz:bazz_fbs_includes" ],
-        )
-
-        In which foo.fbs includes a few files from the Fileset defined at
-        //third_party/bazz:bazz_fbs_includes. When compiling the library that
-        includes foo_generated.h, and therefore has my_fbs as a dependency, it
-        will fail to find any of the bazz *_generated.h files unless you also
-        add bazz's flatbuffer_cc_library to your own dependency list, e.g.:
-
-        cc_library(
-            name = "my_lib",
-            deps = [
-                ":my_fbs",
-                "//third_party/bazz:bazz_fbs"
-            ],
-        )
-
-        Happy dependent Flatbuffering!
-    '''
     output_headers = [
         (out_prefix + "%s_generated.h") % (s.replace(".fbs", "").split("/")[-1].split(":")[-1])
         for s in srcs
     ]
+    if deps and includes:
+        # There is no inherent reason we couldn't support both, but this discourages
+        # use of includes without good reason.
+        fail("Cannot specify both deps and include in flatbuffer_cc_library.")
+    if deps:
+        includes = [d + "_includes" for d in deps]
     reflection_name = "%s_reflection" % name if gen_reflections else ""
 
     srcs_lib = "%s_srcs" % (name)
@@ -226,6 +216,7 @@ def flatbuffer_cc_library(
         flatc_args = flatc_args,
         compatible_with = compatible_with,
         restricted_to = restricted_to,
+        target_compatible_with = target_compatible_with,
         reflection_name = reflection_name,
         reflection_visibility = visibility,
     )
@@ -242,10 +233,12 @@ def flatbuffer_cc_library(
         ],
         deps = [
             "@com_github_google_flatbuffers//:runtime_cc",
-        ],
+            "@com_github_google_flatbuffers//:flatbuffers",
-        includes = [],
+        ] + deps,
+        includes = cc_include_paths,
         compatible_with = compatible_with,
         restricted_to = restricted_to,
+        target_compatible_with = target_compatible_with,
         linkstatic = 1,
         visibility = visibility,
     )
@@ -254,7 +247,7 @@ def flatbuffer_cc_library(
     # Flatbuffer set.
     native.filegroup(
         name = srcs_filegroup_name if srcs_filegroup_name else "%s_includes" % (name),
-        srcs = srcs,
+        srcs = srcs + includes,
         compatible_with = compatible_with,
         restricted_to = restricted_to,
         visibility = srcs_filegroup_visibility if srcs_filegroup_visibility != None else visibility,

tests/BUILD.bazel

@@ -116,14 +116,50 @@ cc_library(
 )
 
 flatbuffer_cc_library(
-    name = "monster_test_cc_fbs",
+    name = "include_test_fbs",
-    srcs = ["monster_test.fbs"],
+    srcs = [
-    include_paths = ["tests/include_test"],
-    includes = [
         "include_test/include_test1.fbs",
         "include_test/sub/include_test2.fbs",
     ],
+    include_paths = ["tests/include_test"],
+)
+
+flatbuffer_cc_library(
+    name = "monster_test_cc_fbs",
+    srcs = ["monster_test.fbs"],
+    include_paths = ["tests/include_test"],
     visibility = ["//grpc/tests:__subpackages__"],
+    deps = [":include_test_fbs"],
+)
+
+# Test that running without --no-includes works properly (monster_test doesn't
+# work cleanly due to the circular dependency in the include_tests/ files).
+include_test_args = [
+    "--gen-object-api",
+    "--gen-compare",
+    "--gen-mutable",
+    "--reflect-names",
+    "--cpp-ptr-type flatbuffers::unique_ptr",
+    "--force-empty",
+]
+
+flatbuffer_cc_library(
+    name = "included_test_fbs",
+    srcs = ["included_test.fbs"],
+    flatc_args = include_test_args,
+)
+
+flatbuffer_cc_library(
+    name = "includer_test_fbs",
+    srcs = ["includer_test.fbs"],
+    flatc_args = include_test_args,
+    deps = [":included_test_fbs"],
+)
+
+cc_library(
+    name = "include_build_test",
+    srcs = ["include_build_test.cc"],
+    deps = [":includer_test_fbs"],
 )
 
 flatbuffer_cc_library(

tests/include_build_test.cc

@@ -0,0 +1 @@
+#include "tests/includer_test_generated.h"

tests/included_test.fbs

@@ -0,0 +1,3 @@
+table Wrapped {
+  a:double;
+}

tests/includer_test.fbs

@@ -0,0 +1,5 @@
+include "tests/included_test.fbs";
+
+table Wrapper {
+  a:Wrapped;
+}