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

Link against hxcpp's mbedtls in static builds #1818

Merged
merged 11 commits into from
Feb 10, 2025

Conversation

tobil4sk
Copy link
Member

@tobil4sk tobil4sk commented Jul 12, 2024

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.

/usr/bin/ld: /.../lime/ndll/Linux64/liblime.a(05cc3e30_mbedtls.o): in function `mbed_connect_common':
mbedtls.c:(.text+0xce1): undefined reference to `mbedtls_net_recv'
/usr/bin/ld: mbedtls.c:(.text+0xcf0): undefined reference to `mbedtls_net_send'

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.

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
@tobil4sk
Copy link
Member Author

Further work seems to still be required to get this working on iOS and android

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.

@tobil4sk
Copy link
Member Author

tobil4sk commented Feb 10, 2025

With hxcpp 4.3.2 (using mbedtls 2.9.0) and running lime test windows -static, this app crashes with current develop:

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.

@joshtynjala joshtynjala merged commit 99ca58d into openfl:develop Feb 10, 2025
30 checks passed
@tobil4sk tobil4sk deleted the fix/static-link branch February 10, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants