Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -346,5 +346,6 @@ set_target_properties(${dist_targets}

if (FLATCC_INSTALL)
install(DIRECTORY include/flatcc DESTINATION include)
install(DIRECTORY cmake DESTINATION ${CMAKE_INSTALL_PREFIX})
endif()

37 changes: 37 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ executable also handle optional json parsing or printing in less than 2 us for a
* [Online Forums](#online-forums)
* [Introduction](#introduction)
* [Project Details](#project-details)
* [Use in CMake build](#use-in-cmake-build)
* [Poll on Meson Build](#poll-on-meson-build)
* [Reporting Bugs](#reporting-bugs)
* [Status](#status)
Expand Down Expand Up @@ -186,6 +187,42 @@ their place.

**NOTE: Big-endian platforms are only supported as of release 0.4.0.**

## Use in CMake build

If your project uses the CMake build system, you can include cmake module
`cmake/FlatccGenerateSources.cmake` that provides the following cmake
function:

flatcc_generate_sources(GENERATED_SOURCE_DIRECTORY <output directory>
GENERATE_BUILDER
GENERATE_VERIFIER
EXPECTED_OUTPUT_FILES <list of expected output files>
DEFINITION_FILES <list of flatbuffer files (.fbs)>
)

`GENERATE_BUILDER` and `GENERATE_VERIFIER` are boolean options. When specified
they will instruct flatcc to generate builder / verifier source code.

Optionally you can let cmake know the directory where the flatcc executable
is located in environment variable `FLATCC_BUILD_BIN_PATH`. This is especially
usefull when cross-compiling. In that case you should provide the directory
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useful

Copy link
Copy Markdown
Author

@wdobbe wdobbe Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding minimum required version, there is reason why flatcc uses cmake_minimum_required (VERSION 2.8)

Otherwise it will not build on some older conservative platforms. If it is really necessary to use a more recent version and it only affects users own programs opting in to this feature then maybe OK, but otherwise it would interfere with portability which is important to FlatCC.

Edit: sorry was confused with PRs from Conan.
I had to update the cmake_minimum_required version from 2.8 to 3.1 in another recipe PR today because a conan-center hook rejects the PR if the cmake_minimum_required is not at least 3.1.

In this case the minimum cmake version is 3.5 because from that version cmake function cmake_parse_arguments checks for value arguments that are miss their value, so it makes the function more fool-proof.

Copy link
Copy Markdown
Author

@wdobbe wdobbe Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suggesting to add an example of using this module within the FlatCC project, but perhaps not replacing existing since it could break portability with CMake version. I'm not sure if the module only works after CMake install and the README should probably also clarify how to make it available.

Ok, I will do that.
The only thing to do is that you have to do to make the module work is add the FlatCC cmake subdir to your CMAKE_MODULE_PATH.

Edit: when we also export a flatcc cmake config file as suggested by @madebr then appending CMAKE_MODULE_PATH is no longer necessary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if it works the same for correct arguments on earlier versions then an earlier MIN version will still support the check for users with a recent CMake while not breaking the build for those who do not.

where the build arch flatcc compiler executable is located.

The flatcc_generate_sources function will create a cmake custom command to
generate the output files during just before compilation (so in the build
step, not the configure step).

Example:

include(FlatccGenerateSources)
flatcc_generate_sources(GENERATED_SOURCE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/datadef
GENERATE_BUILDER
GENERATE_VERIFIER
EXPECTED_OUTPUT_FILES datadef/seclif_protocol_reader.h
datadef/seclif_protocol_builder.h
datadef/seclif_protocol_verifier.h
DEFINITION_FILES ${CMAKE_CURRENT_SOURCE_DIR}/datadef/seclif_protocol.fbs
)

## Poll on Meson Build

Expand Down
86 changes: 86 additions & 0 deletions cmake/FlatccGenerateSources.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
cmake_minimum_required(VERSION 3.5)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would split this file in 2:

  • Export the flatcc target as flatcc::flatcc with install(TARGETS) and install(EXPORT).
  • Inside flatcc-config.cmake, do include("${CMAKE_CURRENT_LIST_DIR}/FlatccGenerateSources.cmake") which then only contains the function. Instead of the FLATCC_COMPILER, it can then use the imported flatcc::flatcc executable target.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be a better solution.


# Use the following function to generate C source files from flatbuffer definition files:
#
# flatcc_generate_sources(GENERATED_SOURCE_DIRECTORY <directory where to write source files>
# GENERATE_BUILDER
# GENERATE_VERIFIER
# EXPECTED_OUTPUT_FILES <list of files that flatcc is supposed to generate>
# DEFINITION_FILES <list of flatbuffer definition files (.fbs)>
# )
#
# GENERATE_BUILDER and GENERATE_VERIFIER are boolean options. When specified they will instruct
# flatcc to generate builder / verifier source code.
#
# With cross-compiling you should provide the directory where the flatcc compiler executable is located
# in environment variable FLATCC_BUILD_BIN_PATH. If you use Conan and add flatcc as a build requirement
# this will be done automatically.


function(flatcc_generate_sources)

# parse function arguments
set(OUTPREFIX "FLATCC") #variables created by 'cmake_parse_arguments' will be prefixed with this
set(NO_VAL_ARGS GENERATE_BUILDER GENERATE_VERIFIER)
set(SINGLE_VAL_ARGS GENERATED_SOURCE_DIRECTORY)
set(MULTI_VAL_ARGS DEFINITION_FILES EXPECTED_OUTPUT_FILES CC_OPTIONS)

cmake_parse_arguments(${OUTPREFIX}
"${NO_VAL_ARGS}"
"${SINGLE_VAL_ARGS}"
"${MULTI_VAL_ARGS}"
${ARGN}
)
if (GENERATED_SOURCE_DIRECTORY IN_LIST FLATCC_KEYWORDS_MISSING_VALUES)
message(FATAL_ERROR "No directory provided after GENERATED_SOURCE_DIRECTORY keyword")
endif()
if (NOT FLATCC_GENERATED_SOURCE_DIRECTORY)
set(FLATCC_GENERATED_SOURCE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
endif()
message(STATUS "Flatcc sources will be generated in directory ${FLATCC_GENERATED_SOURCE_DIRECTORY}")

if (FLATCC_GENERATE_BUILDER)
list(APPEND FLATCC_CC_OPTIONS --builder)
endif()
if (FLATCC_GENERATE_VERIFIER)
list(APPEND FLATCC_CC_OPTIONS --verifier)
endif()

if (FLATCC_DEFINITION_FILES)
if (NOT EXISTS ${FLATCC_GENERATED_SOURCE_DIRECTORY})
file(MAKE_DIRECTORY ${FLATCC_GENERATED_SOURCE_DIRECTORY})
endif()

message(VERBOSE "Executing command ${FLATCC_COMPILER} ${FLATCC_CC_OPTIONS} -o ${FLATCC_GENERATED_SOURCE_DIRECTORY} ${FLATCC_DEFINITION_FILES}")
add_custom_command(OUTPUT ${FLATCC_EXPECTED_OUTPUT_FILES}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs DEPENDS for dependency tracking of the input flatbuffer files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without reading carefully and from memory, just adding DEPENDS will break Ninja build because CMake doesn't handle dependecies properly, even if it works with Make. If you look at some of the examples that build with CMAKE, these are rather clumsy because of this.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But without DEPENDS, the target sources don't get regenerated when the input sources get modified?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add_custom_command documentation does mention ninja with multiple arguments, but not with DEPENDS btw.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know because I am not a CMake expert. But I do know that I have spent several days trying to make it work (or something similar). Another who believed he could fix it had to admit he couldn't.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking into it now, I'll open a pr for installing flatcc-config.cmake.

This allows your users to use flatcc by doing find_package(flatcc REQUIRED) and call flatcc using flatcc::flatcc.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like on every make, all tests are re-built.
This is because the dependency tree is not correct.
I'll look into that too.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wdobbe
You can then re-use/rebase your FlatccGenerateSources.cmake script on my pr.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, here is an old discussion about broken dependencies - I haven't reread it, but linked here for reference:

#24 (comment)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking into it now, I'll open a pr for installing flatcc-config.cmake.

This allows your users to use flatcc by doing find_package(flatcc REQUIRED) and call flatcc using flatcc::flatcc.

Yes, that is a good idea. I have plenty examples of how to generate the config files (correctly) so if you want me to do it, let me know.

COMMAND ${FLATCC_COMPILER} ${FLATCC_CC_OPTIONS} -o ${FLATCC_GENERATED_SOURCE_DIRECTORY} ${FLATCC_DEFINITION_FILES}
WORKING_DIRECTORY ${FLATCC_GENERATED_SOURCE_DIRECTORY})
else()
message(WARNING "No flatbuffer definition files provided, no sources will be generated")
endif()

endfunction()


#### Main ####

#When cross-compiling user can provide location of the flatbuffers to C compiler in build arch via
#environment variable FLATCC_BUILD_BIN_PATH
set(FLATCC_BIN_PATH "$ENV{FLATCC_BUILD_BIN_PATH}")
if (FLATCC_BIN_PATH)
#user provided location where asn1c compiler executable is installed
find_program(FLATCC_COMPILER flatcc
PATHS ${FLATCC_BIN_PATH}
NO_DEFAULT_PATH
NO_SYSTEM_ENVIRONMENT_PATH
NO_CMAKE_SYSTEM_PATH
)
else()
#Find compiler exe
find_program(FLATCC_COMPILER flatcc)
endif()
Comment on lines +69 to +81
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to do this with one find_program:

  • it's possible to override by doing -DFLATCC_COMPILER=xxx.
  • Using -DFLATCC_ROOT, you can set a prefix of flatxx
  • Using the FLATCC_ROOT environment variable, you can also set a prefix

I think the options NO_DEFAULT_PATH, NO_SYSTEM_ENVIRONMENT_PATH and NO_CMAKE_SYSTEM_PATH are not needed.

Suggested change
set(FLATCC_BIN_PATH "$ENV{FLATCC_BUILD_BIN_PATH}")
if (FLATCC_BIN_PATH)
#user provided location where asn1c compiler executable is installed
find_program(FLATCC_COMPILER flatcc
PATHS ${FLATCC_BIN_PATH}
NO_DEFAULT_PATH
NO_SYSTEM_ENVIRONMENT_PATH
NO_CMAKE_SYSTEM_PATH
)
else()
#Find compiler exe
find_program(FLATCC_COMPILER flatcc)
endif()
#user provided location where asn1c compiler executable is installed
find_program(FLATCC_COMPILER flatcc
PATHS ${FLATCC_ROOT} ENV FLATCC_ROOT
PATH_SUFFIXES bin
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#user provided location where asn1c compiler executable is installed

asn1c?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, copy/paste error from a similar script for the ASN1C compiler.
I'll correct that.



if (NOT FLATCC_COMPILER)
message(FATAL_ERROR "Could not locate the flatcc compiler executable")
endif()