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.

Sign up for GitHub

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

Jump to bottom

Pass MBEDTLS_CONFIG_FILE defines through cmake #6914

Merged
merged 6 commits into from
Jul 7, 2023
Merged

Pass MBEDTLS_CONFIG_FILE defines through cmake #6914

merged 6 commits into from
Jul 7, 2023

Conversation

davidhorstmann-arm
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm commented Jan 12, 2023 •
edited
Loading

When -DMBEDTLS_CONFIG_FILE or -DMBEDTLS_USER_CONFIG_FILE are passed to cmake, pass them through as compile definitions. This allows different mbedtls configs to be passed at configure time without modifying any cmake files.

The following will build with a different config in my_config.h:

cd mbedtls
cmake -DMBEDTLS_CONFIG_FILE=my_config.h .
make

Or to add extra options from a config file extra_config.h:

cd mbedtls
cmake -DMBEDTLS_USER_CONFIG_FILE=extra_config.h .
make

Fixes #4805
Fixes #4496

Gatekeeper checklist

@gilles-peskine-arm gilles-peskine-arm added needs-work component-platform Portability layer and build scripts priority-medium Medium priority - this can be reviewed as time permits labels Jan 12, 2023
@davidhorstmann-arm davidhorstmann-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 and removed needs-work labels Jan 23, 2023
@paul-elliott-arm paul-elliott-arm self-requested a review January 24, 2023 10:09
@davidhorstmann-arm davidhorstmann-arm added the needs-backports Backports are missing or are pending review and approval. label Jan 27, 2023
@davidhorstmann-arm davidhorstmann-arm mentioned this pull request Jan 27, 2023
Merged
3 tasks
@davidhorstmann-arm davidhorstmann-arm added needs-ci Needs to pass CI tests and removed needs-backports Backports are missing or are pending review and approval. labels Jan 27, 2023
lpy4105
lpy4105 suggested changes Feb 15, 2023
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
library/CMakeLists.txt Outdated Show resolved Hide resolved
tests/scripts/all.sh Show resolved Hide resolved
lpy4105
lpy4105 previously approved these changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@lpy4105 lpy4105 left a comment

Choose a reason for hiding this comment

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

LGTM

@davidhorstmann-arm davidhorstmann-arm added the historical-reviewing Currently reviewing (for legacy PR/issues) label Jun 9, 2023
@tom-cosgrove-arm tom-cosgrove-arm self-requested a review June 9, 2023 13:54
This was referenced Jun 27, 2023
Closed
Closed
@davidhorstmann-arm
Pass MBEDTLS_CONFIG_FILE defines through cmake
82f11ff 
When -DMBEDTLS_CONFIG_FILE or -DMBEDTLS_USER_CONFIG_FILE are passed to
cmake, pass them through as compile definitions. This allows different
mbedtls configs to be passed at configure time without modifying any
cmake files.

Signed-off-by: David Horstmann <david.horstmann@arm.com>
@davidhorstmann-arm
Pass config file options to mbedtls_test(_helpers)
ae33ab8 
Signed-off-by: David Horstmann <david.horstmann@arm.com>
@davidhorstmann-arm
all.sh component to test cmake custom config file
20550e3 
Signed-off-by: David Horstmann <david.horstmann@arm.com>
@davidhorstmann-arm
Use CONFIG_H variable rather than config file name
969c145 
Signed-off-by: David Horstmann <david.horstmann@arm.com>
@davidhorstmann-arm
Use emptiness-checks rather than DEFINED checks
0f1dd57 
For the MBEDTLS_CONFIG_FILE and MBEDTLS_USER_CONFIG_FILE variables,
check that they are non-empty and defined. This means they can be
unconditionally created in the cache, simplifying the CMakeLists.txt

Signed-off-by: David Horstmann <david.horstmann@arm.com>
@davidhorstmann-arm davidhorstmann-arm force-pushed the cmake-pass-through-config-defines branch from 08cd059 to 0f1dd57 Compare July 5, 2023 13:21
@davidhorstmann-arm
Copy link
Contributor Author

Rebased on development. Only conflict: the mbedtls_test_helpers target was added, so this needed an extra target_compile_definitions().

@davidhorstmann-arm davidhorstmann-arm linked an issue Jul 5, 2023 that may be closed by this pull request
Alternative configuration files in CMake #4496
Closed
@davidhorstmann-arm davidhorstmann-arm removed the request for review from aditya-deshpande-arm July 5, 2023 13:44
paul-elliott-arm
paul-elliott-arm previously approved these changes Jul 5, 2023
View reviewed changes
Copy link
Member

@paul-elliott-arm paul-elliott-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

@davidhorstmann-arm davidhorstmann-arm removed the needs-ci Needs to pass CI tests label Jul 5, 2023
lpy4105
lpy4105 previously approved these changes Jul 6, 2023
View reviewed changes
Copy link
Contributor

@lpy4105 lpy4105 left a comment

Choose a reason for hiding this comment

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

LGTM

@lpy4105 lpy4105 added approved Design and code approved - may be waiting for CI or backports and removed 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 Jul 6, 2023
@davidhorstmann-arm
Add ChangeLog entry for CMake config defines
2d3ba07 
Signed-off-by: David Horstmann <david.horstmann@arm.com>
@davidhorstmann-arm davidhorstmann-arm force-pushed the cmake-pass-through-config-defines branch from cd06913 to 2d3ba07 Compare July 7, 2023 10:26
@davidhorstmann-arm
Copy link
Contributor Author

Changes:

  • Add ChangeLog entry
  • [Force-push] Remove accidental tab from ChangeLog entry

@davidhorstmann-arm davidhorstmann-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 needs-ci Needs to pass CI tests and removed approved Design and code approved - may be waiting for CI or backports needs-reviewer This PR needs someone to pick it up for review labels Jul 7, 2023
tom-cosgrove-arm
tom-cosgrove-arm approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-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

paul-elliott-arm
paul-elliott-arm approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@paul-elliott-arm paul-elliott-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

@paul-elliott-arm paul-elliott-arm added the approved Design and code approved - may be waiting for CI or backports label Jul 7, 2023
@tom-cosgrove-arm tom-cosgrove-arm removed the needs-review Every commit must be reviewed by at least two team members, label Jul 7, 2023
@paul-elliott-arm paul-elliott-arm merged commit 2dfe799 into Mbed-TLS:development Jul 7, 2023
@paul-elliott-arm paul-elliott-arm removed the needs-ci Needs to pass CI tests label Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gilles-peskine-arm gilles-peskine-arm gilles-peskine-arm left review comments

@tom-cosgrove-arm tom-cosgrove-arm tom-cosgrove-arm approved these changes

@paul-elliott-arm paul-elliott-arm paul-elliott-arm approved these changes

@lpy4105 lpy4105 Awaiting requested review from lpy4105

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports component-platform Portability layer and build scripts enhancement historical-reviewing Currently reviewing (for legacy PR/issues) priority-medium Medium priority - this can be reviewed as time permits
Projects
Non-roadmap pull requests
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support to CMake scripts to set user-defined config.h file Alternative configuration files in CMake
5 participants
@davidhorstmann-arm @lpy4105 @gilles-peskine-arm @paul-elliott-arm @tom-cosgrove-arm