Skip to content

zephyr: CMake portability fixes #6611

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
wants to merge 1 commit into from
Closed

zephyr: CMake portability fixes #6611

wants to merge 1 commit into from

Conversation

rleigh-lumiradx
Copy link

@dpgeorge This PR is based upon the comments I made in #6542 (comment)

It's not directly mergeable, since there were differences between the two branches I've not yet fully reconciled. It looks like some changes were merged which obsoletes some of what I was doing. However, I hope some of the general ideas here are useful:

  • The main part is the last bit which adds quote and dequote scripts to replace the use of sed.
  • shell commands like echo and sed are replaced with CMake-native replacements. This provides for portability beyond UNIX e.g. to Windows
  • some configuration currently done at build-time can be done at configure-time, e.g. version header generation. There's no real need for this to be a build rule. This means it won't regenerate the header upon every invocation of the build. I added a bit of logic to make it automatically regenerate whenever the git HEAD changes, which means it will still happen, but less frequently. That could be extended to also include other sources of version information, e.g. documentation.
  • looking more closely, I do wonder if all of the qstr processing couldn't be done at configure time and regenerated on demand if needed

@rleigh-lumiradx
Copy link
Author

One additional comment. Some variables used by mkrules.cmake are defined in ports/esp32/CMakeLists.txt and are not usable by other ports, e.g. zephyr. I wonder if some of these couldn't be factored out into a more general location.

One thought I had: Why is the CMake support port-specific? Why not have a generic CMake build which can be used for any platform, and then as the basis for customisation in individual ports. mkrules.cmake gets us part of the way there, but it could be a bit more general than this.

@dpgeorge
Copy link
Member

@rleigh-lumiradx now that cmake support is in master, can you please rebase this on master and then I can do a review?

@rleigh-lumiradx
Copy link
Author

I'll have a look, and see if this is now necessary--it may already have been addressed by the CMake support branch.

@dpgeorge dpgeorge closed this May 18, 2021
@dpgeorge dpgeorge deleted the branch micropython:esp32-idf41-cmake May 18, 2021 01:41
tannewt added a commit to tannewt/circuitpython that referenced this pull request Aug 2, 2022
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.

2 participants