Skip to content

Add component CMakeLists.txt file for use with CMake-based build system #1508

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

Merged
merged 1 commit into from
Jun 24, 2018

Conversation

projectgus
Copy link
Contributor

@projectgus projectgus commented Jun 18, 2018

ESP-IDF has a new CMake-based build system currently in preview release.

More details here

This PR adds a CMakeLists.txt file so arduino-esp32 component can be used with CMake-based IDF projects.

The one unusual restriction here is that the directory containing the component must be named arduino-esp32. Will fix this restriction before the CMake branch is made the default branch in ESP-IDF.

@PerMalmberg
Copy link

@projectgus Using file-globbing is not recommended as per the CMake documentation.

https://cmake.org/cmake/help/latest/command/file.html

Note We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate. The CONFIGURE_DEPENDS flag may not work reliably on all generators, or if a new generator is added in the future that cannot support it, projects using it will be stuck. Even if CONFIGURE_DEPENDS works reliably, there is still a cost to perform the check on every rebuild.

@projectgus
Copy link
Contributor Author

@PerMalmberg That's right. However, within the IDF build system we do some tricks to make globbing more viable:
https://docs.espressif.com/projects/esp-idf/en/feature-cmake/api-guides/build-system.html#file-globbing-incremental-builds

That said, I have forgotten to use any of those tricks here - thanks for the reminder. Tomorrow I'll amend this PR to either use git_describe to track the arduino-esp32 git revision, or name all the source files individually.

@PerMalmberg
Copy link

@projectgus Ah, yes that does make a difference. 👍

@me-no-dev
Copy link
Member

@projectgus ping me please when you are ready :)

@projectgus projectgus force-pushed the feature/cmakelists_component_file branch from 3600ab5 to c11d36b Compare June 19, 2018 00:09
@projectgus
Copy link
Contributor Author

projectgus commented Jun 19, 2018

@PerMalmberg Redone with individual file names, as I realised there's no guarantee that arduino-esp32 will be a git repo in all cases. Thanks again for pointing that out.

@me-no-dev PTAL. Will send a follow-up PR once we have support in IDF for overriding component names, but this should work for now.

@projectgus projectgus force-pushed the feature/cmakelists_component_file branch 2 times, most recently from 8ccd6ce to dbd707a Compare June 21, 2018 04:10
@projectgus
Copy link
Contributor Author

@me-no-dev Now with Travis checks to make sure the source file list is up to date.

@me-no-dev
Copy link
Member

@projectgus please remove the script calling line from travis.yml because it will cause merge conflicts with the update branch that I am working on. We have moved scripting out of yml and I can add the line manually to the appropriate script :) thanks!

Includes verification script (for Travis) that CMakeLists.txt contents match
repo & submodule source files
@projectgus projectgus force-pushed the feature/cmakelists_component_file branch from dbd707a to 0b0a756 Compare June 22, 2018 07:45
@me-no-dev me-no-dev merged commit 7abd586 into master Jun 24, 2018
@me-no-dev me-no-dev deleted the feature/cmakelists_component_file branch June 24, 2018 12:59
Curclamas pushed a commit to Curclamas/arduino-esp32 that referenced this pull request Aug 21, 2018
…em (espressif#1508)

Includes verification script (for Travis) that CMakeLists.txt contents match
repo & submodule source files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants