Skip to content

Fix missing linkage of mbed-mbedtls-cryptocell310 to mbed-mbedtls #14435

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

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Mar 16, 2021

Summary of changes

The CMake target mbed-mbedtls-cryptocell310 was not linked by default, resulting in a compilation error on targets that use
Mbed TLS:

fatal error: mbedtls_device.h: No such file or directory

Other Mbed TLS drivers simply add themselve to mbed-mbedtls, so this commits does the alignment to fix the build error.

Note: the driver and the core Mbed TLS have mutual dependency, so they need to be one CMake target.

Impact of changes

Migration actions required

Documentation

None.


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@0xc0170 @hugueskamba @rajkan01


0xc0170
0xc0170 previously approved these changes Mar 16, 2021
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

feature object libraries branch has this fixed for all drivers almost.

@mergify mergify bot added the needs: CI label Mar 16, 2021
@0xc0170 0xc0170 added the release-type: patch Indentifies a PR as containing just a patch label Mar 17, 2021
@mergify mergify bot added needs: work and removed needs: CI labels Mar 18, 2021
@mbed-ci
Copy link

mbed-ci commented Mar 18, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_cmake-example-ARM
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 18, 2021

The failure is related.

to my understanding: mbedtls is not enabled in CMake, build fails. This will work for examples that link to mbedtls, but not for apps that do not need mbedtls. We looked at this briefly but haven't found a solution. It's known issue. A target can add additional libraries, in this case tls as the driver requires it, but should it be linked always? An app can choose what to link to, but what about a target.
Any suggestions?

@LDong-Arm
Copy link
Contributor Author

@0xc0170 In other Mbed TLS drivers we simply add the driver code as part of mbed-mbedtls. I can do that.

@LDong-Arm LDong-Arm force-pushed the CRYPTOCELL310_linking branch from 2321e8e to 29474cf Compare March 18, 2021 12:00
@mergify mergify bot dismissed 0xc0170’s stale review March 18, 2021 12:00

Pull request has been modified.

@LDong-Arm LDong-Arm force-pushed the CRYPTOCELL310_linking branch from 29474cf to 2228aa7 Compare March 18, 2021 12:05
@LDong-Arm
Copy link
Contributor Author

@0xc0170 Done

The CMake target mbed-mbedtls-cryptocell310 was not linked by
default, resulting in a compilation error on targets that use
Mbed TLS:

    fatal error: mbedtls_device.h: No such file or directory

Other Mbed TLS drivers simply add themselve to mbed-mbedtls, so this
commits does the alignment to fix the build error.

Note: the driver and the core Mbed TLS have mutual dependency, so
they need to be one CMake target.
@LDong-Arm LDong-Arm force-pushed the CRYPTOCELL310_linking branch from 2228aa7 to 4c6f886 Compare March 18, 2021 12:10
@mergify mergify bot added needs: CI and removed needs: work labels Apr 7, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 7, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 7, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 13, 2021

CI restarted

@mbed-ci
Copy link

mbed-ci commented Apr 13, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit b1e26c6 into ARMmbed:master Apr 13, 2021
@mergify mergify bot removed the ready for merge label Apr 13, 2021
@mbedmain mbedmain removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants