Fix includes for flatbuffers |
|||
Issue descriptionThe current approach of using compiled_action_foreach in the flatbuffers library to compile the flatbuffers IDL code to C++ processes files multiple times if they are included in another file: src/tests/monster_test.fbs starts with include "include_test1.fbs"; So currently, monster_test.fbs is compiled, generating a file include_test1_generated.h, and then include_test1.fbs is compiled, generating a file include_test1_generated.h again. This confuses ninja's dependency graph (see the failed try jobs of https://codereview.chromium.org/1955993002/). Ideally, I would not call the flatc compiler once for each source file, but one time for all source files together. If I call "flatc monster_test.fbs include_test1.fbs", the compiler would figure out that that include_test1_generated.h would only need to be generated once. I haven't figured out how to get that while keeping correct outputs (in the sense of outputs = [ "$out_dir/{{source_name_part}}_generated.h" ]). I have explored these options: 1) As Balazs suggested: create a new build target that contains only the include_test*.fbs. The include_test*.fbs files would still be compiled multiple times. 2) Using inputs = []: I explored to use inputs = [] to create dependencies. I think that works for creating the dependencies but it fails to provide proper outputs = [] entries. 3) flatc would be happy to take multiple frame buffer input files in one call. I could call flatc with parameters "monster_test.fbs include_test1.fbs include_test2.fbs" and it would figure out itself that it needs to generate the code for the include_test*.fbs only once. But this is incompatible with compiled_action_foreach(). I checked whether I could use compiled_action() but then I don't know how to build the outputs as I cannot use the {{source_name_part}} variable substitution (as in "$out_dir/{{source_name_part}}_generated.h") in compiled_action(). Maybe the best alternative is to teach flatc to compile and IDL but only generate one header file, no headerfiles for included IDLs.
,
May 9 2017
,
Jun 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/387b2a94017449569f0f5b2f865d3331e3646a3a commit 387b2a94017449569f0f5b2f865d3331e3646a3a Author: pkalinnikov <pkalinnikov@chromium.org> Date: Tue Jun 06 10:47:36 2017 Update FlatBuffers to include multiple improvements. This CL updates FlatBuffers to the latest version 1.6.x. This is necessary to include the following changes to the library: - d7ac378: Offset<T>::IsNull method. - 93c0960: --keep-prefix compiler option. - c559eb4: Generate files for empty schemas. - aaf5598: Fix path comparison on Windows. - fb87c0d: Improve allocator handling (fixes test). - 22743ca: Fix --keep-prefix bugs. - 642254b: Fix one more --keep-prefix issue. - dd05f32: Add tests for nested FlatBuffers. - 01c50d5: Get rid of move semantics in VectorIterator. BUG= 611351 , 713774 Review-Url: https://codereview.chromium.org/2883063002 Cr-Commit-Position: refs/heads/master@{#477254} [modify] https://crrev.com/387b2a94017449569f0f5b2f865d3331e3646a3a/DEPS [modify] https://crrev.com/387b2a94017449569f0f5b2f865d3331e3646a3a/third_party/flatbuffers/BUILD.gn [modify] https://crrev.com/387b2a94017449569f0f5b2f865d3331e3646a3a/third_party/flatbuffers/README.chromium [modify] https://crrev.com/387b2a94017449569f0f5b2f865d3331e3646a3a/third_party/flatbuffers/flatbuffer.gni
,
Jun 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10a266981d83baf81c3eb24873c8732699ed087e commit 10a266981d83baf81c3eb24873c8732699ed087e Author: pkalinnikov <pkalinnikov@chromium.org> Date: Tue Jun 06 11:32:49 2017 Revert of Update FlatBuffers to include multiple improvements. (patchset #5 id:80001 of https://codereview.chromium.org/2883063002/ ) Reason for revert: Fails "Win" build: https://build.chromium.org/p/chromium/builders/Win/builds/55836 Original issue's description: > Update FlatBuffers to include multiple improvements. > > This CL updates FlatBuffers to the latest version 1.6.x. This is necessary to > include the following changes to the library: > - d7ac378: Offset<T>::IsNull method. > - 93c0960: --keep-prefix compiler option. > - c559eb4: Generate files for empty schemas. > - aaf5598: Fix path comparison on Windows. > - fb87c0d: Improve allocator handling (fixes test). > - 22743ca: Fix --keep-prefix bugs. > - 642254b: Fix one more --keep-prefix issue. > - dd05f32: Add tests for nested FlatBuffers. > - 01c50d5: Get rid of move semantics in VectorIterator. > > BUG= 611351 , 713774 > > Review-Url: https://codereview.chromium.org/2883063002 > Cr-Commit-Position: refs/heads/master@{#477254} > Committed: https://chromium.googlesource.com/chromium/src/+/387b2a94017449569f0f5b2f865d3331e3646a3a TBR=palmer@chromium.org,engedy@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 611351 , 713774 Review-Url: https://codereview.chromium.org/2926563002 Cr-Commit-Position: refs/heads/master@{#477258} [modify] https://crrev.com/10a266981d83baf81c3eb24873c8732699ed087e/DEPS [modify] https://crrev.com/10a266981d83baf81c3eb24873c8732699ed087e/third_party/flatbuffers/BUILD.gn [modify] https://crrev.com/10a266981d83baf81c3eb24873c8732699ed087e/third_party/flatbuffers/README.chromium [modify] https://crrev.com/10a266981d83baf81c3eb24873c8732699ed087e/third_party/flatbuffers/flatbuffer.gni
,
Jun 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/488ae5249bcea6c46af5e832f8be87dcc8df56e1 commit 488ae5249bcea6c46af5e832f8be87dcc8df56e1 Author: pkalinnikov <pkalinnikov@chromium.org> Date: Thu Jun 08 12:09:44 2017 Reland: Update FlatBuffers to include multiple improvements. This CL updates FlatBuffers to the latest version 1.6.x. This is necessary to include the following changes to the library: - d7ac378: Offset<T>::IsNull method. - 93c0960: --keep-prefix compiler option. - c559eb4: Generate files for empty schemas. - aaf5598: Fix path comparison on Windows. - fb87c0d: Improve allocator handling (fixes test). - 22743ca: Fix --keep-prefix bugs. - 642254b: Fix one more --keep-prefix issue. - dd05f32: Add tests for nested FlatBuffers. - 01c50d5: Get rid of move semantics in VectorIterator. This is a reland of https://codereview.chromium.org/2883063002/ with a compile flag added to Windows build suppressing the alignment warning. BUG= 611351 , 713774 TBR=palmer@chromium.org Review-Url: https://codereview.chromium.org/2923203002 Cr-Commit-Position: refs/heads/master@{#477947} [modify] https://crrev.com/488ae5249bcea6c46af5e832f8be87dcc8df56e1/DEPS [modify] https://crrev.com/488ae5249bcea6c46af5e832f8be87dcc8df56e1/third_party/flatbuffers/BUILD.gn [modify] https://crrev.com/488ae5249bcea6c46af5e832f8be87dcc8df56e1/third_party/flatbuffers/README.chromium [modify] https://crrev.com/488ae5249bcea6c46af5e832f8be87dcc8df56e1/third_party/flatbuffers/flatbuffer.gni
,
Jun 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/35b1f087f6c61741160807549ff9051fb4d015e0 commit 35b1f087f6c61741160807549ff9051fb4d015e0 Author: pkalinnikov <pkalinnikov@chromium.org> Date: Thu Jun 08 19:47:27 2017 Uncomment test fbs files. This CL ensures that crbug/611351 has been fixed in crrev/2883063002. BUG= 611351 Review-Url: https://codereview.chromium.org/2925803002 Cr-Commit-Position: refs/heads/master@{#478061} [modify] https://crrev.com/35b1f087f6c61741160807549ff9051fb4d015e0/third_party/flatbuffers/BUILD.gn
,
Jun 9 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by pkalinnikov@chromium.org
, May 9 2017Status: Started (was: Assigned)