Flatbuffers: Is the build_flatbuffers cmake function expected to work?

[dpacbach]

Hi,

I recently decided to integrate Flatbuffers into my project. I built the latest version of Flatbuffers using CMake, then installed it. I then integrated it into my project (which using CMake itself) by calling find_package and pointing it to the location of the FlatbuffersConfig.cmake file that was installed. All of that worked fine.

Then the question arose: in my own project's CMake files, how do I run the Flatbuffers compiler to compile *.fbs files into generated files? I noticed that there is a file called CMake/BuildFlatBuffers.cmake that contains a function called build_flatbuffers that appears to be for this purpose. My questions concerns this function/file:

1. Has this function/file recently been tested? I found what appears to be a bug in it, suggesting that people are not using it or testing it. I was only able to get it working by applying a patch to it correcting the flatc target name from flatc to flatbuffers::flatc.
2. Is there any documentation that covers this specific question?

More generally, the documentation that I found for Flatbuffers only seems to cover building Flatbuffers itself (using CMake or other methods), but does not cover integrating it into a parent project (say, one that also uses CMake)... is there any that I'm missing?

Thanks
David

[aardappel]

Not sure, it may well be that this is broken, and it certainly isn't well documented.

It is definitely intended to be possible to run flatc directly from CMake, and the main CMake file does this thru compile_flatbuffers_schema_to_cpp_opt declared in that same file.

The function build_flatbuffersin the file you reference creates build rules as opposed to just invoke flatc for a single file. I am not sure who uses it: https://github.com/google/flatbuffers/commits/master/CMake/BuildFlatBuffers.cmake Maybe @proppy @senior7515 @kmaragon know :)

[senior7515]

@dpacbach - It is pretty easy to write a cmake function for this. This is what we use for our project specific hooks:

https://github.com/smfrpc/smf/blob/master/cmake/smfc_generator.cmake#L58-L69

[dpacbach]

Thanks, I will import a code snippet such as the one in your example which should work fine for me.

That said, might be nice for new users to get the build_flatbuffers code fixed and put an example of using it somewhere in the docs (and also for the make install step to install it, which it currently does not do).

CapnProto on the other hand provides such a function that is included when installing the package and it works out of the box:

https://github.com/capnproto/capnproto/blob/master/c%2B%2B/cmake/CapnProtoMacros.cmake

I think my particular question has been answered, but I'll leave it up to the maintainers whether or not they want to leave this issue open.

[senior7515]

On Thu, Oct 3, 2019 at 12:24 PM David P. Sicilia [e-mail]
wrote:

Thanks, I will import a code snippet such as the one in your example which
should work fine for me.

That said, might be nice for new users to get the build_flatbuffers code
fixed and put an example of using it somewhere in the docs (and also for
the make install step to install it, which it currently does not do).

CapnProto on the other hand provides such a function that is included when
installing the package and it works out of the box:

https://github.com/capnproto/capnproto/blob/master/c%2B%2B/cmake/CapnProtoMacros.cmake

I think my particular question has been answered, but I'll leave it up to
the maintainers whether or not they want to leave this issue open.

You could also grab that example and submit a quick fix, for future users
as well as rename the issue. Seems like a good an easy fix.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/google/flatbuffers/issues/5548?email_source=notifications&email_token=AABKYZV65KDCBGXGFYQQEBTQMZBFXA5CNFSM4I45EUDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAJJJ3I#issuecomment-538088685,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABKYZUSH3RD63QUWOWTZUTQMZBFXANCNFSM4I45EUDA
.

[dpacbach]

Actually on looking at the code in build_flatbuffers again, it looks like it's actually OK. The only change I had to make to get it to work was to change the flatc target name to flatbuffers::flatc, which I thought was a bug.

However, it turns out that the target name depends on whether one imports the installed version of the package (e.g. via FlatbuffersConfig.cmake) in which case it will be flatbuffers::flatc or if one adds Flatbuffers as a subproject (e.g. git submodule) in a parent project and uses it via CMake's add_subdirectory, in which case the target name will simply be flatc.

Apart from that, the function actually works (sorry for the noise). I have submitted a PR that adds a note about this step to the docs.

[dpacbach]

After some more thought, I just realized that the docs that I added (see my PR) have one shortcoming (which also applies to the build_flatbuffers function AFAICS): that is, the custom build rules do not know how to detect dependencies among schema files, e.g. when one schema file includes another. If A.fbs includes B.fbs then the correct rebuilding behavior is to recompile both files when B.fbs changes. How do users of flatbuffers typically handle this? If you like I can add mention of this to my doc additions in the PR.

[aardappel]

We have a -M option that can generate dependencies for make, which has been used in the past to automatically compile dependants, I think. But beyond that flatc has no automatic way to compile dependants.. people would typically do this by hand. Worth mentioning, yes.

[keef-cognitiv]

Sorry I hadn't been checking my forums email folder. I'm admittedly a few versions back but still actively use it. It generally works out of the box for my use case. But perhaps I'm not using some arguments that you are.

find_package(FlatBuffers REQUIRED)
...
build_flatbuffers("${FLATBUFFER_SOURCES}" "" "name_of_target")

Generally works for me. As long as I add the CMAKE_CURRENT_BINARY_PATH in the include paths.

I may have missed it but what version of CMake is this?

I should probably update my Conan package to a newer version anyway. So it'll be good to know a solution to the issue that I might be able to apply in case I face it as well.

I don't see any changes in the flatbuffers repo that would cause you to need something like this but it's conceivable that there might be some new CMake things perhaps.

[ghorbanzade]

Hi Folks. I ran into a problem today that maybe related to this issue:

When using Conan, the packaged CMake modules are missing BuildFlatBuffers. And that is a bit unfortunate because it makes it impossible to use the FindFlatBuffers CMake module since the latter includes the former (see here).

I could be wrong but I thing the point of including the CMake module is to allow users to leverage the function flatbuffers_generate_c_headers which is not possible due to the missing BuildFlatBuffers. I like to suggest that we include this file in the conan package (one liner change in this file. Is that reasonable? Otherwise, please advise on the right way to generate source in CMake when using Conan.

Obviously this only an inconvenience since users always have the option to call flatc in their own CMake functions.

[ipburbank]

the custom build rules do not know how to detect dependencies among schema files, e.g. when one schema file includes another.

Would providing --gen-all solve this issue?

--gen-all Generate not just code for the current schema files, but for all files it includes as well.

The documentation could suggest:

SET(FLATBUFFERS_FLATC_SCHEMA_EXTRA_ARGS "--gen-all")

[dpacbach]

--gen-all Generate not just code for the current schema files, but for all files it includes as well.

I believe that would be the opposite of what we want; if A.fbs includes B.fbs then a change to A.fbs should _not_ cause B.fbs to be regenerated. But a change to B.fbs _should_ cause A.fbs to be regenerated.

Now, if you wanted to add that capability behind a flag to flatbuffers then it would require running flatbuffers command with the complete set of flatbuffers files in the project specified on the command line. This is so that it could have a global view and then do some analysis/scanning to find all the fbs files that directly or indirectly include B.fbs. Running flatbuffers on a per-file basis would not allow this.

However, if flatbuffers were run with multiple files in this situation, it would need to have yet more logic so as to _avoid_ regenerating the fbs files that don't need to be. This is to avoid needlessly updating them and triggering more (e.g. C++) compilation in the code that uses the generated output.

If flatbuffers could do the above, then that would be great with me, and I could just pass it in all of my flatbuffers files and it would just work, and this problem would go away. But, since it doesn't at the moment, I was forced to hack together my own solution consisting of running a script at the start of each incremental build that manually scans all the fbs files and judges, based on their time stamps and dependency structure, which ones need to be updated.

[stefansjs]

Forgive the stupid question, but I seem to be having a problem with this function that I can't get better debugging information on.

It looks like the documentation in the PR is not about build_flatbuffers. So the PR is not about documenting/fixing build_flatbuffers?

I can't seem to get build_flatbuffers to work for my project. If I do

add_subdirectory(../../third_party/flatbuffers ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY} EXCLUDE_FROM_ALL)
build_flatbuffers("${flatbuffers_schemas}" # flatbuffers_schemas
                  ${flatbuffers_resources_dir} # schema_include_dir
                  schema # custom_target_name
                  )

I get an error with the build_flatbuffers function itself

CMake Error at schema/CMakeLists.txt:23 (build_flatbuffers):
  build_flatbuffers Function invoked with incorrect arguments for function
  named: build_flatbuffers

But I can't even get the build_flatbuffers target if I do find_package instead, which might be because FlatbuffersConfig.cmake doesn't include BuildFlatBuffers.cmake, whereas FindFlatbuffers.cmake does 😕

find_package(Flatbuffers REQUIRED
             PATHS ../../third_party/flatbuffers/CMake
             )
build_flatbuffers("${flatbuffers_schemas}" # flatbuffers_schemas
                  ${flatbuffers_resources_dir} # schema_include_dir
                  schema # custom_target_name
                  )

CMake Error at schema/CMakeLists.txt:23 (build_flatbuffers):
  Unknown CMake command "build_flatbuffers".

My current workaround involves a lot of code, and requires detecting the host architecture to build flatc, so it would be really convenient for me to replace all of this brittle code with one function that works correctly 🥰

if(CMAKE_CROSSCOMPILING)
    # And here's one problematic detail. We cross-compile for phone targets (on e.g. ARM architectures), but we want to
    # run flatc on the machine performing the compile. We set the CMAKE_SYSTEM_NAME property on the flatc target so that
    # it compiles for the host system instead of the target system

    # By invoking cmake externally, we forget all the flags, settings, toolchain, etc.
    add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY}/flatc
                       COMMAND ${CMAKE_COMMAND} ${flatbuffers_dir}
                       COMMAND make flatc
                       DEPENDS ${flatbuffers_dir}
                       WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY}/
                       COMMENT "Building flatc")
    # custom command allows us to reference it with the same name as when added with add_subdirectory
    add_custom_target(flatc DEPENDS ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY}/flatc)

    # make sure the directory exists and gets cleaned up
    file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY})
    set_directory_properties(PROPERTIES ADDITIONAL_MAKE_CLEAN_FILES ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY})
else()
    add_subdirectory(${flatbuffers_dir} ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY} EXCLUDE_FROM_ALL)
    add_executable(flatc ALIAS flatc)
endif()

# Create a command for building the schemas (and a sentinel file)
set(sentinel ${CMAKE_BINARY_DIR}/${FLATBUFFERS_BUILD_DIRECTORY}/.schema_done)

[dpacbach]

@stefansjs

The quick answer to your CMake error is that you are missing a few parameters in the build_flatbuffers function. If you look inside BuildFlatBuffers.cmake it has the following function signature:

function(build_flatbuffers flatbuffers_schemas
                           schema_include_dirs
                           custom_target_name
                           additional_dependencies
                           generated_includes_dir
                           binary_schemas_dir
                           copy_text_schemas_dir)

and so it looks like you are missing four parameters, which you might be able to set to empty strings if you don't need them. I'd recommend looking at the function's implementation to understand how they are used and to determine if you need them. Also, you are correct that the function is not included in the "installed" flatbuffers package (at least that was the case at the time of writing).

Regarding this Github issue/PR in general: As I documented earlier in this thread, shortly after raising this issue I did some further testing with the build_flatbuffers function and determined that it basically works as-is. All that I needed to do in my case was to edit one or two lines of the function implementation to add a namespace to the flatc target name (to become flatbuffers::flatc) because in my local setup it was using an alias (which will not always be the case for everyone, see my previous comments in this thread).

I also later documented in this thread an observation that the build_flatbuffers has a shortcoming: although it works for simple projects, it does not setup a build that can do proper/optimal incremental building in response to changed *.fbs files that are depended on (included) by other *.fbs files, in the way that the C++ or Java build system would e.g. rebuild A if A includes B and B is updated.

So therefore, the PR, in its current state, seeks to do a few things:

1. Document the existence of the build_flatbuffers function so that people are aware of it.
2. Suggest some alternatives to build_flatbuffers if someone wants to create their own version.
3. Document the shortcoming of the above methods with respect to incremental building.

The PR has been sitting there a while, I have been meaning to get back to it. In my own project I ended up not using build_flatbuffers; I just create custom CMake build commands (add_custom_command) to invoke the flatc compiler and I have my own pre-build script that runs before each incremental build to invalidate (touch the timestamp on) any flatbuffers files that need to be rebuilt due to having included other out-of-date flatbuffers files, which otherwise would be challenging to do with bare invocations of the flatc compiler and/or CMake only. Hope that helps.

[github-actions[bot]]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.