-
-
Notifications
You must be signed in to change notification settings - Fork 383
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
Link against hxcpp's mbedtls in static builds #1818
Conversation
This avoids lime's mbedtls overwriting hxcpp's, which has caused issues in the past.
Hxcpp 4.3.0 has an mbedtls_config.h file in ${HXCPP}/src/hx/libs/ssl, which we have to ensure is included, but older versions do not. To support both 4.3.0 and older versions, we can add an empty mbedtls_config.h and add the include path at the end. This way it will only be used if it does not exist in the previous include paths.
Hxcpp's mbedtls has MBEDTLS_NET_C disabled, which meant that older versions of curl which use this feature cannot be linked against it. This version of curl no longer requires this feature, which avoids the issue.
The targets do not create a file with the target name, so they should be marked as .PHONY We are not using implicit rules either, so we can disable them by making .SUFFIXES empty Group targets together Also remove LIB_BASE variable, it has been unused since: f7ab6ab
This requires explicitly running the 'haxe' target, as the 'default' target does not output a file. It was added in hxcpp 3.2: HaxeFoundation/hxcpp@3ff9733
It is no longer used, and was only there to provide compatibility with hxcpp versions older than 3.2
The latest hxcpp makes it easier to link against the internal mbedtls, however we still need backwards compatibility. HaxeFoundation/hxcpp#1133
13903b7
to
c9b8a10
Compare
I've made the required changes so it should be linking properly now on ios. It seems like android static builds are not possible at the moment, but if they are done using the static cpp template then it should include mbedtls correctly. |
With hxcpp 4.3.2 (using mbedtls 2.9.0) and running import lime.app.Application;
import haxe.Http;
class Main extends Application {
public function new () {
super ();
var connection:Http = new Http("https://google.com");
connection.request();
}
} So the ios crash would probably still occur with hxcpp 4.3.2 unless this patch is applied. |
This avoids issues with lime's mbedtls taking priority over hxcpp's, which has happened in the past and has broken haxe standard library calls. See #1767 and HaxeFoundation/hxcpp#1126.
The static lime library no longer includes mbedtls definitions. Mbedtls is now added at the end when everything is linked together. This means that if lime was linked against an incompatible version of mbedtls to what the current hxcpp version has, there will be a linker error instead of both versions being merged together silently and resulting in a buggy build. We are compiling hxcpp_ssl, which includes hxcpp's SSL.cpp but this is a small inconvenience and I think most linkers should remove the unneeded definitions. If hxcpp's xml files are reorganised, we can avoid this with future hxcpp versions (see HaxeFoundation/hxcpp#1133).
MBEDTLS_NET_C
is currently undefined by the mbedtls config for hxcpp (see HaxeFoundation/hxcpp#1128). This causes this linker errors with curl, but we can avoid it by updating to curl 7.87.0.Further work seems to still be required to get this working on iOS and android, but right now I'm not too familiar with how the builds work there.