-
Notifications
You must be signed in to change notification settings - Fork 222
cmake: flatcc_generate_sources + add_custom_command + crossbuild + ci #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
e75c8a4
cmake: use add_custom_command(OUTPUTS xx.h) + add_executable(.. xx.h)…
madebr e231644
cmake: install cmake targets
madebr 17b346a
cmake: use alias'ed flatcc::(cli|libflatcc|runtime)
madebr 7d836bb
cmake: generate + install flatcc-config.cmake
madebr 30bfbaf
cmake: use add_Test(NAME .. COMMAND ..) to support commands with exec…
madebr c7d5302
cmake: add custom target for parallel MSVC builds
madebr cb6563e
Enable test_generated test
madebr 6fdd281
Create cmake subfolder
madebr 519ed9a
#168: add cmake helper function to generate flatbuffer code
5f03ecd
Use flatcc_generate_source to generate headers + track dependencies
madebr 90202ba
cmake: don't use file(REAL_PATH)
madebr a40d6c3
ci: output message when test fails
madebr 199bcac
cmake: apply regex on sources to track dependencies
madebr 40174a7
cmake: install flatcc-config-version.cmake to allow version discrimin…
madebr 1f0ab8f
cgen_test: buill test code as library (to make use of cmake build sys…
madebr d2322c8
ci: add github actions script
madebr 41b1a5f
fix test_generated
madebr 38acb79
remove lib/libtest* libraries in scripts/cleanall.sh
madebr 10f6a9e
ci: add cross building github action
madebr c1f1fd9
Add .editorconfig
madebr 9808df5
Restored changed after botched force push
madebr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| root = true | ||
|
|
||
| [{*.c, *.h, *.cpp, *.fbs, *.sh, *.cmake, *.cmake.in, *.txt}] | ||
| indent_style = space | ||
| indent_size = 4 | ||
| insert_final_newline = true | ||
| trim_trailing_whitespace = true | ||
|
|
||
| [{*.yml, *.json}] | ||
| indent_style = space | ||
| indent_size = 2 | ||
| insert_final_newline = true | ||
| trim_trailing_whitespace = true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| on: | ||
| schedule: | ||
| # Run this script every saturday, so you have the whole weekend to fix problems :evil-grin: | ||
| - cron: '0 0 * * 6' | ||
| jobs: | ||
| merge_into_ci_more: | ||
| # Only enable this job on dividelabs' repo to avoid running this job on forks | ||
| if: ${{ github.repository_owner == 'dividelabs' }} | ||
| runs-on: 'ubuntu-latest' | ||
| steps: | ||
| - name: 'Checkout repo' | ||
| uses: actions/checkout@v2 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: 'Configure git' | ||
| run: | | ||
| git config --global user.name "ci-more cron job" | ||
| git config --global user.email "ci-more-cron-job@example.com" | ||
| - name: 'Checkout ci-more' | ||
| run: | | ||
| git checkout ci-more | ||
| - name: 'Backup .travis.yml and appveyor.yml' | ||
| run: | | ||
| cp .travis.yml .travis.yml.backup | ||
| cp appveyor.yml appveyor.yml.backup | ||
| - name: 'Checkout .travis.yml and appveyor.yml from master' | ||
| run: | | ||
| git checkout origin/master -- .travis.yml appveyor.yml | ||
| git add .travis.yml appveyor.yml | ||
| git commit -m "Temporarily add .travis.yml and appveyor.yml from master" | ||
| - name: 'Merge master into ci-more' | ||
| run: | | ||
| git merge origin/master -m "Merge master into ci-more" | ||
| - name: 'Restore .travis.yml and appveyor.yml' | ||
| run: | | ||
| mv .travis.yml.backup .travis.yml | ||
| mv appveyor.yml.backup appveyor.yml | ||
| git add -- .travis.yml appveyor.yml | ||
| git commit -m "Restore .travis.yml and appveyor.yml" | ||
| - name: 'Check whether the working directory is clean' | ||
| run: | | ||
| git diff --exit-code | ||
| - name: 'Check whether only .travis.yml and appveyor.yml differ with master' | ||
| run: | | ||
| if test $(git diff origin/master --name-only | wc -l) != 2; then | ||
| echo "ci-more should diverge from origin/master with exactly 2 files (.travis.yml and appveyor.yml)"; | ||
| git diff origin/master --exit-code; # origin/master | ||
| exit 1; | ||
| fi | ||
| - name: 'Push ci-more to server' | ||
| run: | | ||
| git push origin ci-more | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,183 @@ | ||
| on: | ||
| push: | ||
| branches-ignore: | ||
| - 'ci-more' | ||
| pull_request: | ||
| jobs: | ||
| test_unix: | ||
| name: 'Test Unix with CMake' | ||
| strategy: | ||
| matrix: | ||
| include: | ||
| - os: 'ubuntu-latest' | ||
| config: 'Release' | ||
| cmake_generator: 'Unix Makefiles' | ||
| shared: 'OFF' | ||
| - os: 'ubuntu-latest' | ||
| config: 'Debug' | ||
| cmake_generator: 'Ninja' | ||
| shared: 'OFF' | ||
| - os: 'ubuntu-latest' | ||
| config: 'Release' | ||
| cmake_generator: 'Unix Makefiles' | ||
| shared: 'ON' | ||
| - os: 'macos-latest' | ||
| config: 'Release' | ||
| cmake_generator: 'Unix Makefiles' | ||
| shared: 'OFF' | ||
| - os: 'macos-latest' | ||
| config: 'Debug' | ||
| cmake_generator: 'Ninja' | ||
| shared: 'OFF' | ||
| - os: 'macos-latest' | ||
| config: 'Release' | ||
| cmake_generator: 'Ninja' | ||
| shared: 'ON' | ||
| runs-on: ${{ matrix.os }} | ||
| steps: | ||
| - name: 'Checkout repo' | ||
| uses: actions/checkout@v2 | ||
| - name: 'Install meson (Ubuntu)' | ||
| if: ${{ contains(matrix.os, 'ubuntu') && (matrix.cmake_generator == 'Ninja') }} | ||
| run: | | ||
| sudo apt-get install ninja-build | ||
| - name: 'Install meson (Macos)' | ||
| if: ${{ contains(matrix.os, 'macos') && (matrix.cmake_generator == 'Ninja') }} | ||
| run: | | ||
| brew install meson | ||
| - name: 'CMake configure' | ||
| run: | | ||
| mkdir build | ||
| cd build | ||
| cmake .. -DBUILD_SHARED_LIBS=${{ matrix.shared }} -G "${{ matrix.cmake_generator }}" \ | ||
| -DCMAKE_BUILD_TYPE=${{ matrix.config }} -DFLATCC_INSTALL=ON \ | ||
| -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/prefix | ||
| - name: 'CMake build' | ||
| run: | | ||
| cmake --build build --parallel | ||
| - name: 'CTest' | ||
| run: | | ||
| cd build | ||
| ctest -V -C ${{ matrix.config }} | ||
| - name: 'Install' | ||
| run: | | ||
| cmake --build build --target install | ||
| - name: 'Test installed flatcc prefix' | ||
| run: | | ||
| mkdir user && cd user | ||
| cmake "${{ github.workspace }}/test/install_test" -DCMAKE_PREFIX_PATH="${{ github.workspace }}/prefix" | ||
| cmake --build . | ||
| ctest . | ||
| test_windows: | ||
| name: 'Test Windows with CMake' | ||
| strategy: | ||
| matrix: | ||
| config: ['Release', 'Debug'] | ||
| arch: ['Win32', 'x64'] | ||
| runs-on: 'windows-latest' | ||
| steps: | ||
| - name: 'Checkout repo' | ||
| uses: actions/checkout@v2 | ||
| - name: 'CMake configure' | ||
| run: | | ||
| mkdir build | ||
| cd build | ||
| cmake .. -A ${{ matrix.arch }} -DFLATCC_INSTALL=ON -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/prefix | ||
| - name: 'CMake build' | ||
| run: | | ||
| cmake --build build --parallel --config ${{ matrix.config }} --verbose | ||
| - name: 'CTest' | ||
| run: | | ||
| cd build | ||
| ctest -V -C ${{ matrix.config }} | ||
| - name: 'Install' | ||
| run: | | ||
| cmake --build build --target install --config ${{ matrix.config }} | ||
| - name: 'Test installed flatcc prefix' | ||
| run: | | ||
| mkdir user && cd user | ||
| cmake "${{ github.workspace }}/test/install_test" -A ${{ matrix.arch }} ` | ||
| -DCMAKE_PREFIX_PATH="${{ github.workspace }}/prefix" | ||
| cmake --build . --config ${{ matrix.config }} | ||
| ctest . -C ${{ matrix.config }} | ||
| test_mingw_on_linux: | ||
| name: 'Test cross building to Windows with CMake' | ||
| strategy: | ||
| matrix: | ||
| config: ['Release'] | ||
| arch: ['x64'] | ||
| shared_build: ['ON'] | ||
| config_build: ['Release'] | ||
| shared_host: ['ON'] | ||
| config_host: ['Release'] | ||
| runs-on: 'ubuntu-latest' | ||
| steps: | ||
| - name: 'Install mingw' | ||
| run: | | ||
| sudo apt-get install mingw-w64 | ||
| - name: 'Checkout repo' | ||
| uses: actions/checkout@v2 | ||
| - name: 'CMake configure/build/install build flatcc (native)' | ||
| run: | | ||
| mkdir build_native | ||
| cd build_native | ||
| cmake .. -DBUILD_SHARED_LIBS=${{ matrix.shared_build }} -DCMAKE_BUILD_TYPE=${{ matrix.config_build }} \ | ||
| -DFLATCC_INSTALL=ON -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/prefix_build | ||
| cmake --build . --parallel | ||
| cmake --build . --target install | ||
| - name: 'CMake configure/build/install host flatcc (cross)' | ||
| run: | | ||
| mkdir build_mingw | ||
| cd build_mingw | ||
| cmake .. -DCMAKE_TOOLCHAIN_FILE=../test/cmake/x86_64-w64-mingw32-toolchain.cmake \ | ||
| -DBUILD_SHARED_LIBS=${{ matrix.shared_host }} -DFLATCC_INSTALL=ON \ | ||
| -DCMAKE_BUILD_TYPE=${{ matrix.config_host }} \ | ||
| -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/prefix_host \ | ||
| -DFLATCC_ROOT_FOR_BUILD=${{ github.workspace }}/prefix_build | ||
| cmake --build . --parallel | ||
| cmake --build . --target install | ||
| - name: 'Build a flatbuffers project with both native and host flatcc' | ||
| run: | | ||
| mkdir project | ||
| cd project | ||
| cmake ../test/install_test -DCMAKE_TOOLCHAIN_FILE=../test/cmake/x86_64-w64-mingw32-toolchain.cmake \ | ||
| -DCMAKE_PREFIX_PATH=${{ github.workspace }}/prefix_host \ | ||
| -DFLATCC_ROOT_FOR_BUILD=${{ github.workspace }}/prefix_build | ||
| cmake --build . | ||
| test_mingw_on_windows: | ||
| name: 'Test mingw on Windows' | ||
| strategy: | ||
| matrix: | ||
| shared: ['ON'] | ||
| config: ['Release'] | ||
| runs-on: 'windows-latest' | ||
| steps: | ||
| - name: 'Checkout repo' | ||
| uses: actions/checkout@v2 | ||
| - name: 'CMake configure' | ||
| run: | | ||
| mkdir build | ||
| cd build | ||
| cmake .. -DBUILD_SHARED_LIBS=${{ matrix.shared }} -DCMAKE_BUILD_TYPE=${{ matrix.config }} ` | ||
| -DFLATCC_INSTALL=ON -DCMAKE_INSTALL_PREFIX="${{ github.workspace }}/prefix" ` | ||
| -G "MinGW Makefiles" | ||
| - name: 'CMake build' | ||
| run: | | ||
| cmake --build build --parallel | ||
| - name: 'CTest' | ||
| run: | | ||
| # FIXME: try using https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT.html on each test | ||
| $env:Path += ";${{ github.workspace }}/bin" | ||
| cd build | ||
| ctest -V -C ${{ matrix.config }} | ||
| - name: 'Install' | ||
| run: | | ||
| cmake --build build --target install | ||
| - name: 'Test installed flatcc prefix' | ||
| run: | | ||
| $env:Path += ";${{ github.workspace }}/prefix/bin" | ||
| mkdir user && cd user | ||
| cmake "${{ github.workspace }}/test/install_test" ` | ||
| -DCMAKE_PREFIX_PATH="${{ github.workspace }}/prefix" -G "MinGW Makefiles" | ||
| cmake --build . | ||
| ctest . |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| build/* | ||
| bin/* | ||
| lib/* | ||
| lib64/* | ||
| release/* | ||
| scripts/build.cfg |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you end up removing
.travis.ymlandappveyor.ymlfrom master, thenci-more.ymlcan be reduced.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard for me to tell if this is implemented correctly, so I am going (mostly) for the documentation. I really don't know how CMake operates in this context.
Note, I might deprecate --schema and rename to --binary-schema or --reflection in a future flatcc version, but that doesn't change much here.
Looking at the comments in FlatccGenerateSources.cmake:
I wrote earlier that I'm not sure that setting READER as the default argument. But it also seems that ALL, RECURSIVE is being set, which I think is more useful:
But later the following section says differently:
There is an old error message here, should be "No flatbuffer schema files provided":
I don't understand this line:
Later you append -I prefix to FLATCC_PATHS - I'm note sure what a keyword args on other CMake context.
Regarding the COMPILE_FLAGS option, this is (probably) correct, but I wonder if EXTRA_ARGS, EXTRA_FLAGS, or something else would be more meaningful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function now accepts
BINARY_SCHEMAonly.This function/file is meant to be distributed along flatcc, so it can be kept in sync.
When some options changes in the future, there are ways to ease the transition.
Is
ALLthe new agreed-upon default for cmake then?You can also choose for a non-standard option e.g.
COMMON+BUILDER+VERIFIER+READERwithoutRECURSIVE.But I think this will be harder to document, for no gain.
What this does, is iterate over all output_options (
ALL BINARY_SCHEMA COMMON COMMON_READER COMMON_BUILDER BUILDER READER) and if none of these are set, then assume the caller has passedREADER(=default output option when no output has been selected).Fixed (also removed all reference of definition file to schema file).
This variable is later used in
cmake_parse_arguments.It is a list of arguments that except multiple arguments.
SCHEMA_FILEScan be one file or more.I copied the terminology from
FindBISON. But I guessEXTRA_FLAGSis ok too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created another PR to @madebr 's branch.
Things still to do/check:
If you can postpone the merge to Tuesday or so, I can do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wdobbe
I'm in no hurry to merge.
@madebr
If the READER is default, what does the following mean?
No I am raising the discussion. I would expect at least READER + COMMON_READER as default because without a COMMON_READER the result will not be usable unless the common file is created elsewhere.
But in general ALL + RECURSIVE will build anything a user would expect when not caring too much about arguments, which is why I think that will be a good choice. But I am open to suggestions.
It looks to me as if you expand all the -I paths without the -I prefix and later try to parse it, but I am probable just not understanding the mechanism:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! And merged!
This is added, and tested.
See https://github.com/madebr/flatcc/actions/runs/432848623
I'll leave that up to you :D