Skip to content

Enable use of Mbed TLS as a CMake subproject #2706

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
Merged
Patater merged 5 commits into from
Jul 10, 2019
Merged

Enable use of Mbed TLS as a CMake subproject #2706

Patater merged 5 commits into from
Jul 10, 2019

Conversation

@Patater
Copy link
Contributor

@Patater Patater commented Jun 18, 2019

Remove use of CMAKE_SOURCE_DIR in case mbedtls is built from within
another CMake project. Define MBEDTLS_DIR to ${CMAKE_CURRENT_SOURCE_DIR}
in the main CMakeLists.txt file and refer to that when defining target
include paths to enable mbedtls to be built as a sub project.

Fixes #2609

Signed-off-by: Ashley Duncan ashes.man@gmail.com
Signed-off-by: Jaeden Amero jaeden.amero@arm.com

Description

A few sentences describing the overall goals of the pull request's commits.

Status

IN DEVELOPMENT

Requires Backporting

NO - but may be useful in a 2.18.1

Migrations

NO

Additional comments

Any additional information that could be of interest

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

@Patater Patater added enhancement needs-work needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests labels Jun 18, 2019
@Patater Patater changed the title Cmake subproject fix Enable use of Mbed TLS as a CMake subproject Jun 18, 2019
@Patater Patater force-pushed the cmake-subproject-fix branch from 3d865b1 to adc02bc Compare June 20, 2019 16:48
@Patater
Copy link
Contributor Author

Patater commented Jun 20, 2019

Rebased to add information about this newly supported feature to the README and to add a regression test.

@Patater Patater force-pushed the cmake-subproject-fix branch from adc02bc to 52692c7 Compare June 20, 2019 17:26
@Patater Patater requested review from andresag01 and dgreen-arm June 20, 2019 17:28
@Patater
Copy link
Contributor Author

Patater commented Jun 20, 2019

Force pushed to update crypto submodule

@Patater Patater force-pushed the cmake-subproject-fix branch from 52692c7 to 996411b Compare June 21, 2019 09:14
@Patater
Copy link
Contributor Author

Patater commented Jun 21, 2019

Rebased to update crypto submodule

@Patater Patater force-pushed the cmake-subproject-fix branch from 996411b to e71f8d0 Compare June 21, 2019 14:39
@Patater
Copy link
Contributor Author

Patater commented Jun 21, 2019

Rebased to not require too new of a CMake version.

andresag01
andresag01 previously approved these changes Jun 24, 2019
View reviewed changes
Copy link
Contributor

@andresag01 andresag01 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 to me. Just some minor comments, but no blockers.

@Patater Patater force-pushed the cmake-subproject-fix branch from e71f8d0 to 39f9d6e Compare June 25, 2019 12:29
@Patater Patater mentioned this pull request Jun 25, 2019
Merged
@Patater
Copy link
Contributor Author

Patater commented Jun 25, 2019

Rebased to change minimum CMake version to 2.6, to match the version required by the top level project CMakeLists.txt. Also updated the crypto submodule to the latest version of ARMmbed/mbed-crypto#152

ashesman and others added 3 commits June 25, 2019 13:33
@ashesman @Patater
Remove use of CMAKE_SOURCE_DIR
3278081 
Remove use of CMAKE_SOURCE_DIR in case mbedtls is built from within
another CMake project. Define MBEDTLS_DIR to ${CMAKE_CURRENT_SOURCE_DIR}
in the main CMakeLists.txt file and refer to that when defining target
include paths to enable mbedtls to be built as a sub project.

Fixes Mbed-TLS#2609

Signed-off-by: Ashley Duncan <ashes.man@gmail.com>
Signed-off-by: Jaeden Amero <jaeden.amero@arm.com>
@Patater
ChangeLog: Enable builds as a CMake subproject
33b6a99
@Patater
README: Enable builds as a CMake subproject
41421c4 
Update the README with information on a newly supported feature: the
ability to build Mbed TLS as a subproject of another CMake project.
@Patater Patater force-pushed the cmake-subproject-fix branch from 39f9d6e to fdeefaa Compare June 25, 2019 12:34
@Patater
Copy link
Contributor Author

Patater commented Jun 25, 2019 •
edited
Loading

Rebased to rebase atop latest development and resolve conflicts in ChangeLog and crypto submodule update.

@Patater
CMake: Add a subdirectory build regression test
ab83fdf 
If we have a regression with the "build Mbed TLS as a subdirectory with
CMake" feature and fail to build, fail the test.
@Patater Patater force-pushed the cmake-subproject-fix branch 2 times, most recently from 13b76fd to b498569 Compare June 25, 2019 14:22
@Patater
Copy link
Contributor Author

Patater commented Jun 25, 2019

Rebased to move cleanup of the CMake subproject test to cleanup(), and to update to the latest ARMmbed/mbed-crypto#152

@Patater Patater force-pushed the cmake-subproject-fix branch from b498569 to c15c3f6 Compare June 26, 2019 11:49
@Patater
Copy link
Contributor Author

Patater commented Jun 26, 2019

Rebased to update to the latest ARMmbed/mbed-crypto#152

dgreen-arm
dgreen-arm previously approved these changes Jun 28, 2019
View reviewed changes
andresag01
andresag01 previously approved these changes Jul 1, 2019
View reviewed changes
@Patater
Update Mbed Crypto to contain mbed-crypto#152
a4b94c4 
Update Mbed Crypto to a commit on its development branch that contains
the merged [mbed-crypto#152
PR](ARMmbed/mbed-crypto#152).
@Patater Patater dismissed stale reviews from andresag01 and dgreen-arm via a4b94c4 July 2, 2019 07:45
@Patater Patater force-pushed the cmake-subproject-fix branch from c15c3f6 to a4b94c4 Compare July 2, 2019 07:45
@Patater
Copy link
Contributor Author

Patater commented Jul 2, 2019

Rebased to update Mbed Crypto to a commit on its development branch containing the merged ARMmbed/mbed-crypto#152 PR.

@Patater Patater removed the needs-work label Jul 2, 2019
@Patater Patater removed the needs-ci Needs to pass CI tests label Jul 4, 2019
@Patater Patater 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, labels Jul 5, 2019
@jenia81
Copy link

jenia81 commented Jul 8, 2019

@Patater when is this PR expected to be merged?
We need the PSA multiple key usage feature - https://github.com/ARMmbed/mbedtls-psa/pull/266 that is part of Mbed TLS 2.18. And currently Mbed TLS 2.18 isn't compiled with PDMC on Linux, so this PR is needed

@Patater Patater merged commit a4b94c4 into Mbed-TLS:development Jul 10, 2019
@Patater Patater mentioned this pull request Jul 12, 2019
Merged
2 tasks
@soarqin soarqin mentioned this pull request Feb 12, 2020
Closed
@Patater Patater mentioned this pull request Mar 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@andresag01 andresag01 andresag01 approved these changes

@dgreen-arm dgreen-arm dgreen-arm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Design and code approved - may be waiting for CI or backports enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cant build mbedtls as cmake subdirectory

5 participants

@Patater @jenia81 @andresag01 @dgreen-arm @ashesman