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

Fix Mbed TLS build for Android #15680

Merged
merged 23 commits into from
Jan 25, 2021
Merged

Fix Mbed TLS build for Android #15680

merged 23 commits into from
Jan 25, 2021

Conversation

ahmedyarub
Copy link
Contributor

@ahmedyarub ahmedyarub commented Jan 15, 2021 •
edited by JackBoosY
Loading

Describe the pull request
Allows using VCPKG toolchain + Android toolchain. Otherwise, we would not be able to use Android toolchain together with VCPKG building Mbed TLS. After this, SDK building should work on VCPKG. It also fixes the check for the environment (Windows) to make it compatible with cross-compilation.
This change can be easily extended to enable building with VCPKG + iOS/Wasm or any other cross-platform build.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@JackBoosY JackBoosY self-assigned this Jan 18, 2021
@JackBoosY JackBoosY added category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. category:port-bug The issue is with a library, which is something the port should already support labels Jan 18, 2021
@JackBoosY
Copy link
Contributor

Please run command vcpkg x-add-version --all then commit the changes.

Thanks.

@ahmedyarub
Copy link
Contributor Author

Please run command vcpkg x-add-version --all then commit the changes.

Thanks.

Done

@JackBoosY
Copy link
Contributor

@ahmedyarub I found that some of the items in your comment have not yet been implemented. Is this PR ready?

@ahmedyarub
Copy link
Contributor Author

@ahmedyarub I found that some of the items in your comment have not yet been implemented. Is this PR ready?

I`m sorry which comments exactly? If you are talking about Wasm and iOS support then that would be done in a separate PR during the next week.

@JackBoosY
Copy link
Contributor

Test all features successfully on x86-windows and x64-windows-static.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jan 18, 2021
@ahmedyarub
Copy link
Contributor Author

Fixed according to https://vcpkg.readthedocs.io/en/latest/users/android/

@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Jan 20, 2021
@JackBoosY
Copy link
Contributor

@ahmedyarub Ready for review now?

@ahmedyarub
Copy link
Contributor Author

@ahmedyarub Ready for review now?

Yes it is

ras0219
ras0219 suggested changes Jan 21, 2021
View reviewed changes
Copy link
Contributor

@ras0219 ras0219 left a comment

Choose a reason for hiding this comment

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

LGTM once my two comments are addressed!

ports/mbedtls/portfile.cmake Outdated Show resolved Hide resolved
ports/mbedtls/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jan 21, 2021
@JackBoosY
Copy link
Contributor

Already merged from master.

@ahmedyarub
Copy link
Contributor Author

This should be ready for last review and merge.

@dan-shaw dan-shaw merged commit be2092a into microsoft:master Jan 25, 2021
@ahmedyarub ahmedyarub deleted the mbedtls_android branch January 29, 2021 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ras0219 ras0219 ras0219 requested changes

@JackBoosY JackBoosY JackBoosY approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ahmedyarub @JackBoosY @ras0219 @dan-shaw @ahmedalnuaimi @strega-nil