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

[mbedtls] update to 3.1.0 #20860

Closed
wants to merge 16 commits into from
Closed

Conversation

JonLiu1993
Copy link
Member

@JonLiu1993 JonLiu1993 commented Oct 20, 2021

Fix #25703

Update mbedtls to the latest version 3.1.0 to support find_package

Feature pthreads tested successfully in the following triplet:

  • x86-windows
  • x64-windows
  • x64-windows-static

@JonLiu1993 JonLiu1993 added info:internal This PR or Issue was filed by the vcpkg team. category:port-update The issue is with a library, which is requesting update new revision labels Oct 20, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout a79d4a8ec7ae4464eb22f6af23e277882a67595d -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/m-/mbedtls.json b/versions/m-/mbedtls.json
index 9232e59..fca7356 100644
--- a/versions/m-/mbedtls.json
+++ b/versions/m-/mbedtls.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "0579930c1af250dd8b65891b533397cf5ca0c067",
+      "git-tree": "1253d35afb1cda738ecfaefd4d36dc3e692c7041",
       "version": "3.0.0",
       "port-version": 0
     },

@JonLiu1993
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JonLiu1993
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JonLiu1993 JonLiu1993 changed the title [mbedtls] update to 3.0.0 [mbedtls] update to 3.1.0 Jan 6, 2022
@dg0yt
Copy link
Contributor

dg0yt commented Jan 18, 2022

What is the state of this PR?

Is it feasible at all to update port mbedtls from 2 to 3? AFAIU there are API changes, and so it depends on downstream ports to adapt to these changes before it can be upgraded in vcpkg.
This problem is mitigated by hiding incompatible combinations from CI by features, e.g. SSL alternatives.
In the meantime, it might be better to do a (security!) update within the version 2 family.

Port mebdtls also has issues with mingw, and it relies on invalid CMake Find module support from port pthreads.

@JonLiu1993
Copy link
Member Author

What is the state of this PR?

Is it feasible at all to update port mbedtls from 2 to 3? AFAIU there are API changes, and so it depends on downstream ports to adapt to these changes before it can be upgraded in vcpkg. This problem is mitigated by hiding incompatible combinations from CI by features, e.g. SSL alternatives. In the meantime, it might be better to do a (security!) update within the version 2 family.

Port mebdtls also has issues with mingw, and it relies on invalid CMake Find module support from port pthreads.

Thanks for your suggestion, this pr is successfully tested locally but there are still some problems on CI, I will re-fix it when I have time

@dg0yt
Copy link
Contributor

dg0yt commented Jan 18, 2022

this pr is successfully tested locally but there are still some problems on CI

IMO the CI results confirm that the mbedtls API changes are not compatible with downstream ports.
I.e. mbedtls may require changes to downstream ports which are "feature development", not qualifying for patching in vcpkg.

@JonLiu1993
Copy link
Member Author

this pr is successfully tested locally but there are still some problems on CI

IMO the CI results confirm that the mbedtls API changes are not compatible with downstream ports. I.e. mbedtls may require changes to downstream ports which are "feature development", not qualifying for patching in vcpkg.

I'll go check my pr again to see if my changes introduced these issues

@JonLiu1993
Copy link
Member Author

Close this pr temporarily, I will reopen when I have time to fix it

@JonLiu1993 JonLiu1993 closed this Jan 25, 2022
@ekilmer
Copy link
Contributor

ekilmer commented Feb 1, 2022

@JonLiu1993 The build failure of mbedtls on Linux can be fixed by this patch (tested locally)

diff --git a/ports/mbedtls/portfile.cmake b/ports/mbedtls/portfile.cmake
index 5c1c84f41..9e8a16c9a 100644
--- a/ports/mbedtls/portfile.cmake
+++ b/ports/mbedtls/portfile.cmake
@@ -18,6 +18,8 @@ vcpkg_check_features(
         pthreads ENABLE_PTHREAD
 )
 
+vcpkg_find_acquire_program(PYTHON3)
+
 vcpkg_cmake_configure(
     SOURCE_PATH "${SOURCE_PATH}"
     OPTIONS
@@ -29,7 +31,7 @@ vcpkg_cmake_configure(
 
 vcpkg_cmake_install()
 
-vcpkg_cmake_config_fixup(CONFIG_PATH "CMake")
+vcpkg_cmake_config_fixup(CONFIG_PATH "cmake")
 
 vcpkg_copy_pdbs()

But as @dg0yt mentioned, there are multiple ports in vcpkg that either need to be updated or require non-trivial changes in order to support mbedtls v3.

Is there some vcpkg policy regarding how to update packages that aren't compatible with new major versions of their dependencies other than bugging the maintainers to fix it?

The above question is also related to the process of adding new ports that may rely on mbedtls v3+.

@JonLiu1993
Copy link
Member Author

@ekilmer ,Thanks for your investigation and help. At present, when we encounter this kind of problem, we need to fix the error that other ports cannot be built due to the update of this port version together.

@BullyWiiPlaza
Copy link
Contributor

It would be nice to be able to use mbedtls with CMake since as of right now only v3 supplies a CMake configuration file. Any workarounds for that or will the v3 migration happen any time soon?

@JonLiu1993 JonLiu1993 deleted the dev/Jon/mbedtls branch August 28, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mbedtls] update to 3.2.1
6 participants