Skip to content
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

Modern CMake #125

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

sodevel
Copy link

@sodevel sodevel commented Aug 25, 2022

I tried to use this library as subproject during a CMake build but this failed because the current CMake files are pre-Modern-CMake age and the library target doesn't carry the usage requirements. I wanted to fix that and ended up rewriting the main CMake files completely. I tried to stay compatible with the current variant, but i had to change some user visible aspects to follow recommended CMake practices. I also fixed some issues with the library that looked like errors to me. I did not update the CMake files of the examples and tests except two small changes to make them work with the rewritten files. I did not touch the CMake file in the hal directory, it references even more elements not present in the repository and is not necessary to build the library.

User visible changes:

  1. All cache variables use a common prefix. This reduces conflicts when used as subproject in another CMake build.
  2. TLS support can now be enabled with a cache variable.
  3. Honor BUILD_SHARED_LIBS and build only one library variant at a time. This seems to be the recommended way and simplifies the CMake files a lot.
  4. Remove homegrown cross-compiling. This doesn't work correctly, use toolchain files for cross-compiling.
  5. Add file-service to public API. It looks like this should be public, an example uses this API and won't work otherwise.
  6. Change the include path in the PkgConfig file to match the path used by the examples. The examples include the files without path, the PkgConfig file should use the same path, or the client application needs to change the paths depending if the library is used as subproject or library.

Internal changes:

  1. Use Mbedtls as subproject. It's just better to use its CMake build system than compiling its files on your own.
  2. Apply PLATFORM_IS_BIGENDIAN result. While the test was performed, the result was actually never used.
  3. Remove marking of source files as CXX for MSVC. I see no reason why this should be done, maybe ancient MSVC versions lacked some C features?
  4. Remove using of Linker def file for MSVC. The file isn't present in the repository anyway.
  5. Remove manual configuration of Resource Compiler. This should work just fine on its own in current CMake versions.
  6. Remove generation of export headers. These aren't used at all.
  7. Don't hardcode path segments for install target. Use CMake variables to specify these locations in a safe manner.
  8. Don't hardcode path segments in PkgConfig file. Use the CMake variables used by the install target to stay in sync.

I have tested these new CMake files on Linux only, they are working fine in all configurations and with or without Mbedtls. I couldn't test on other platforms. I also did not test CPack.

sodevel added 16 commits June 3, 2024 16:27
Use recommended practices and rewrite main CMake files for Modern CMake.
Remove possible outdated and unused elements. Keep the library target name
to stay compatible with existing CMake files of examples and tests.
Install public API headers using these sets. Remove now unneeded explicit
include directory definitions.
Use the variables used by the new CMake files.
Use more variables inside PkgConfig template to prevent hardcoding of
path segments.
Grant the unit tests access to private components of the library.
If TLS is enabled but no local Mbedtls copy available, use FetchContent
to download the required version from the official GitHub repository.
Use a postfix for the external project name.
This allows to easily disable the installation if used as subproject.
This moves the responsibility from the usage side to the definition side
and the corresponding code closer to the location where the current mbedtls
version is declared. In a future update only this location needs to be adapted.
Don't apply the flag in the top level file or it will also be applied to Mbed TLS.
Prevent possible name clashes when used as subproject.
This version is some patch levels newer than the one used by upstream.
This version does not require fixing its usage requirements.
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.

1 participant