openfl / lime

A foundational Haxe framework for cross-platform development
https://lime.openfl.org/
MIT License
753 stars 365 forks source link

Update curl and mbedtls submodules #1682

Closed joshtynjala closed 1 year ago

joshtynjala commented 1 year ago

Updated to match 8.2.0-Dev branch

Fixes "SSL connect error" on macOS native targets that was introduced in Lime 8.0.1. Most likely from the CI server switching from building on macOS 10.15 to building on macOS 11.

joshtynjala commented 1 year ago

@player-03 Since you did the work on submodules in 8.2.0-Dev, I was hoping that you'd give it a once over before I merge this into develop.

player-03 commented 1 year ago

So the main problem I see is that people won't have the correct remote installed. This came up on Discord a couple months ago, and I said I wanted to code a workaround, but I never got around to it.

joshtynjala commented 1 year ago

Your instructions for updating the submodules seem to work for me. I ran the following commands to check out my fork and then switch between develop and the branch with the new remotes:

git clone git@github.com:joshtynjala/lime.git
cd lime
git submodule init
git submodule sync
git submodule update
git checkout update-curl-and-mbedtls
git submodule sync
git submodule update

I'm not sure that there's a way around requiring this additional step. Using the Git repo instead of a Haxelib release is always going to have some additional overhead, whether it's occasional unstable code or some extra repo maintenance.

To get this critical bug fix released soon, I'm willing to live with a little extra work to support folks on Discord/forums.

player-03 commented 1 year ago

With a slightly different url, it works for me. Maybe it isn't so bad.

@tobil4sk Does this work for you?

git clone https://github.com/joshtynjala/lime.git
cd lime
git submodule init
git submodule sync
git submodule update
git checkout update-curl-and-mbedtls
git submodule sync
git submodule update
tobil4sk commented 1 year ago

Does this work for you?

Yes, running these commands to create a fresh copy of the lime repo works without any issues.

In my main checked out copy I ended up adding both remotes to each of the submodules, since it became too tedious to switch between them every time.

With a slightly different url, it works for me. Maybe it isn't so bad.

I guess the issue comes when the remote branches have diverged so much that git has issues switching between their main branches.

player-03 commented 1 year ago

The remotes aren't diverging, though. To diverge they'd have to share at least one past commit. These are two entirely separate codebases with no shared history.

And that should only be a problem if you try to merge. I think your error had to do with Git not even finding the remote, which is why it worked after git remote add.

joshtynjala commented 1 year ago

CI seems to be getting stuck building mbedtls for the Android NDLL:

Error: Error while running command arm-linux-androideabi-g++ -Ilib/mbedtls/include/ -Ilib/zlib/ --sysroot=/Users/runner/hostedtoolcache/ndk/r15c/x64/platforms/android-21/arch-arm -I/Users/runner/hostedtoolcache/ndk/r15c/x64/sources/cxx-stl/gnu-libstdc++/4.9/include -I/Users/runner/hostedtoolcache/ndk/r15c/x64/sources/cxx-stl/gnu-libstdc++/4.9/libs/armeabi/include -DHXCPP_VISIT_ALLOCS -DHXCPP_API_LEVEL=0 -I/Users/runner/hostedtoolcache/haxe/4.2.5/x64/lib/hxcpp/4,2,1/include -Iinclude -fpic -fvisibility=hidden -ffunction-sections -funwind-tables -fstack-protector -fno-short-enums -Wno-overflow "-D_LINUX_STDDEF_H " -Wno-psabi -DHXCPP_CPP11 -DHXCPP_ARMV5 -DARM_ARCH_5 -DARM_ARCH_5T -DARM_ARCH_5E -DARM_ARCH_5TE -march=armv5te -mtune=xscale -msoft-float -fomit-frame-pointer -fexceptions -fno-strict-aliasing -finline-limit=10000 -DANDROID=ANDROID -DHX_ANDROID -DHXCPP_ANDROID_PLATFORM=21 -Wa,--noexecstack -O2 -DNDEBUG -c -x c ./lib/mbedtls/library/bignum.c -o/Users/runner/work/lime/lime/project/obj/android/062ae6ec_bignum.obj

./lib/mbedtls/library/bignum.c: In function 'mpi_select': ./lib/mbedtls/library/bignum.c:1994:5: error: 'for' loop initial declarations are only allowed in C99 or C11 mode for( size_t i = 0; i < T_size; i++ ) ^ ./lib/mbedtls/library/bignum.c:1994:5: note: use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code

Could this be because develop is still using NDK r15c and 8.2.0-Dev is using r21e?

player-03 commented 1 year ago

Maybe. NDK 21+ uses Clang rather than GCC (with a whole other toolchain to go with it), so there's potentially a lot that's different.

For the minimum possible change, try <cflag value="-std=c11" /> in mbedtls-files.xml.

joshtynjala commented 1 year ago

<cflag value="-std=c11" /> made no difference, but I tried <compilerflag value="-std=c11" />, and it gave me a different failure. A few of these "PIC register clobbered" errors on different lines:

Error: In file included from ./lib/mbedtls/library/bignum.c:41:0: ./lib/mbedtls/library/bignum.c: In function 'mpi_mul_hlp': ./lib/mbedtls/library/bn_mul.h:86:13: error: PIC register clobbered by 'ebx' in 'asm'

define asm __asm

         ^

./lib/mbedtls/library/bn_mul.h:102:5: note: in expansion of macro 'asm' asm( \ ^ ./lib/mbedtls/library/bignum.c:1414:9: note: in expansion of macro 'MULADDC_INIT' MULADDC_INIT ^

player-03 commented 1 year ago

Maybe it needs NDK 21 and/or Clang. I do recall something being incompatible with the old version.

Before debugging this any further, can you check 8.0.0? If 8.0.0 works and 8.0.1 fails, there might be an easier way to solve this.

joshtynjala commented 1 year ago

Yes, Lime 8.0.0 works correctly. Assuming that the issue is switching the CI from macOS 10.15 to 11, we could potentially set up a VM or computer running 10.15 to build the ndll for macOS.

joshtynjala commented 1 year ago

If needed, I have an old MacBook running 10.13 that can't upgrade any further.

player-03 commented 1 year ago

But does 8.0.0 work on Mac 11?

joshtynjala commented 1 year ago

Lime 8.0.0 built with mac11 has the same SSL connect error.

player-03 commented 1 year ago

Then I guess the only viable path is forwards.

From a quick search, the compiler flag -fno-pic should get around this. No idea if it'll cause other problems, but from how people talked about it, it's at least sometimes a good choice.

joshtynjala commented 1 year ago

Adding <compilerflag value="-fno-pic"/> didn't seem to have any effect. I get the same "clobbered" errors.

I found this PR for mbedtls that seems relevant. Mbed-TLS/mbedtls#1986

It appears to have been merged into mbedtls 3.3.0. It looks like we're at 3.2.0 in 8.2.0-Dev, and this branch.

player-03 commented 1 year ago

So that means NDK 15 shipped with GCC < 5? Seems plausible.

In that case, there's no urgent need to update 8.2.0-Dev, but we might as well try it here.

DigiEggz commented 1 year ago

Can confirm that this pull fixes the SSL error. It also fixes another SSL error I was having with HashLink.

joshtynjala commented 1 year ago

CI passes after updating to mbedtls v3.3.0, fixing the issue with Android. I also had to add the <compilerflag value="-std=c11" /> line, mentioned earlier in this thread.

I can no longer reproduce the "SSL connect error" message with Lime either built locally or built on CI.

I confirm that HTTPS URLs are loading correctly on macOS and Windows (C++, Neko, and HL), Android (C++), and iOS (C++).

Galactic00 commented 3 months ago

So what's the easiest way to fix this? i had the same error: C:/Users/**/Desktop/** (Android)/android/hxcpp/git/project/thirdparty/mbedtls-2.28.2/library/bignum.c:1990:5: error: 'for' loop initial declarations are only allowed in C99 or C11 mode for( size_t i = 0; i < T_size; i++ ) ^

player-03 commented 3 months ago

Did you try the Git version of hxcpp?

Galactic00 commented 3 months ago

Yes, I used git, I'm also using the most recent versions of lime, flixel, open fl and all the others needed. I can try to re-install the git again maybe.

Galactic00 commented 3 months ago

Did about as much as I expected.

When it prompted with This version of hxcpp appears to be a source/developement version. Before this can be used, you need to:

  1. Rebuild the main command-line tool, this can be done with: cd tools/hxcpp haxe compile.hxml
  2. FOR HXCPP API < 330: Build the binaries appropriate to your system(s), this can be done with: cd project neko build.n

Would you like to do this now [y/n]

i said yes as I assumed I was meant to. Not really sure on what else this could be though I downloaded the NDK r15c separately and im using the most recent of the SDK's the 35.0.0=rc4 and the Android 14.0 ("UpsideDownCakePrivacySandbox") SDK Platform

Not sure what else I could provide sorry.