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

Add crypto only CMake build system #9445

Merged

Conversation

ronald-cron-arm
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm commented Aug 2, 2024

Description

Fix #9283

PR checklist

  • changelog not required because: no changes for the user point of view
  • development PR provided: this one
  • framework PR not required
  • 3.6 PR not required because: 4.0 only changes
  • 2.28 PR not required because: 4.0 only changes
  • tests provided: new all.sh component testing the crypto only build

@ronald-cron-arm ronald-cron-arm added enhancement component-crypto Crypto primitives and low-level interfaces priority-high High priority - will be reviewed soon labels Aug 2, 2024
@davidhorstmann-arm davidhorstmann-arm self-requested a review August 2, 2024 14:10
@davidhorstmann-arm
Copy link
Contributor

Since this touches all.sh it will be held up by #8226.

@davidhorstmann-arm davidhorstmann-arm added the needs-preceding-pr Requires another PR to be merged first label Aug 2, 2024
@ronald-cron-arm
Copy link
Contributor Author

yes indeed

@minosgalanakis minosgalanakis self-assigned this Aug 2, 2024
@ronald-cron-arm ronald-cron-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Aug 2, 2024
@minosgalanakis minosgalanakis force-pushed the tf-psa-crypto-cmake-build branch from d85da81 to 8de9f9b Compare August 7, 2024 14:04
@minosgalanakis minosgalanakis added needs-ci Needs to pass CI tests and removed needs-preceding-pr Requires another PR to be merged first labels Aug 7, 2024
Remove dependency on mbedtls_test_helpers
to build the crypto test suites.
mbedtls_test_helpers is TLS specific.

Signed-off-by: Ronald Cron <[email protected]>
Move library options to the top CMakeLists.txt.
That way:
- we will be able to set the TF-PSA-Crypto
library options according to the Mbed TLS ones.
- we can define the crypto library target names
in the top CMakeLists.txt and not in the library
one that is dedicated to the TLS and x509
libraries now.

Signed-off-by: Ronald Cron <[email protected]>
They were mistakenly moved to the
core directory from the library
directory.

Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
ronald-cron-arm and others added 9 commits October 1, 2024 15:32
Copy of mbedtls top CMakeLists.txt file.
The TF-PSA-Crypto top CMakeList.txt file
will be derived from that file to outline
what is common and what is different
between the two.

Signed-off-by: Ronald Cron <[email protected]>
Do not support package config, install, apidoc
and lcov for the time being.

Signed-off-by: Ronald Cron <[email protected]>
Remove framework and pkgconfig for the time
being.

Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Minos Galanakis <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Minos Galanakis <[email protected]>
@ronald-cron-arm ronald-cron-arm force-pushed the tf-psa-crypto-cmake-build branch from 8de9f9b to 393f9a1 Compare October 1, 2024 13:34
@gabor-mezei-arm gabor-mezei-arm self-requested a review October 2, 2024 09:28
@ronald-cron-arm ronald-cron-arm removed the request for review from davidhorstmann-arm October 2, 2024 09:36
@ronald-cron-arm ronald-cron-arm removed the needs-ci Needs to pass CI tests label Oct 2, 2024
@minosgalanakis minosgalanakis self-requested a review October 2, 2024 11:01
@ronald-cron-arm ronald-cron-arm removed the needs-reviewer This PR needs someone to pick it up for review label Oct 2, 2024
Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

Looks good overall.

I have tested the out of source builds as well as the USE_SHARED_MBEDTLS_LIBRARY combinations.

It also adds the capability to append custom configurations options to crypto_config file.

@@ -95,6 +96,19 @@ else()
option(ENABLE_TESTING "Build Mbed TLS tests." ON)
endif()

option(USE_STATIC_MBEDTLS_LIBRARY "Build Mbed TLS static library." ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note. Those parameter iterations were tested and confirmed locally on my machine.

Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@ronald-cron-arm ronald-cron-arm added this pull request to the merge queue Oct 3, 2024
Merged via the queue into Mbed-TLS:development with commit 467edcd Oct 4, 2024
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-crypto Crypto primitives and low-level interfaces enhancement needs-review Every commit must be reviewed by at least two team members, priority-high High priority - will be reviewed soon
Development

Successfully merging this pull request may close these issues.

Add CMake build system in tf-psa-crypto directory
4 participants