search

godotengine / godot-mono-builds

Mono build scripts for Godot
MIT License
56 stars 35 forks source link

Issues with enabling BTLS on Windows #66

Open akien-mga opened 2 years ago

akien-mga commented 2 years ago

OS/device including version: Windows, cross-compiling from Fedora with MinGW

Issue description: PR #49 introduced build issues that affect both macOS ARM64 builds and the Windows 64-bit builds (at least cross-compiled from Fedora with MinGW).

See https://github.com/godotengine/build-containers/issues/95 for the macOS ARM64 build.

For several months I had this PR reverted locally in build-containers to solve it (https://github.com/godotengine/build-containers/commit/d8d17c5cae2a1bb3d0d58d262749e15b396d5cd7), and then in #64 and #65 we decided to improve it to make it Windows-only to prevent the macOS ARM64 issue.

And I've used that in 3.5 RC 1 builds based on https://github.com/godotengine/godot-mono-builds/commit/c865201ebaa29135e6e72787d46ba464f39e2a15, but it seems that it's still broken on Windows 64-bit now: https://github.com/godotengine/godot/issues/61176

So for now I've reverted #65 and #49 with https://github.com/godotengine/godot-mono-builds/commit/20368d72891fe815cb25c2d4cb996fe7614a4fdb and https://github.com/godotengine/godot-mono-builds/commit/aaee521d08fcb2849aa16760a4acfa69ac631f54.

This can be looked into further to figure out how to actually make BTLS support work on Windows with MinGW, and possibly finally fix https://github.com/godotengine/godot/issues/54758 (ref #47 too for context). Until we're sure that it's working, keeping these WIP patches seem to do more harm than good.

akien-mga commented 2 years ago

I'll have to update the above, it seems like I misunderstood things and I did unnecessarily reverts this morning, this doesn't seem to be the cause of the issue I'm debugging at https://github.com/godotengine/godot/issues/61176.

neikeq commented 2 years ago

If this causes too much trouble, we may as well forget about supporting this in Godot 3.x. It's too much trouble. In 4.0 we don't need to worry about this at all, it will just work.

akien-mga commented 2 years ago

So after more testing I found that https://github.com/godotengine/godot/issues/61176 was indeed not related to BTLS at all, but it's a MinGW + GCC + binutils regressions which present in a specific combination of versions of these components, as found in Fedora 35. Using Fedora 34 (like we used to) or Fedora 36 as a baseline works fine.

So we could revert my revert commits and re-add btls-shared on Windows builds. I thought that we didn't have it in my previous Fedora 34 based container due to https://github.com/godotengine/build-containers/commit/d8d17c5cae2a1bb3d0d58d262749e15b396d5cd7, but actually I built the Windows container one day before that commit - probably I made that commit to fix the macOS build after the Windows build had succeeded, so they used a slightly different version of godot-mono-builds (which is cloned in the host by the build script).

But indeed, since we don't intend to do more work to actually make this useful, it's maybe fine as is. My (limited) understanding is that even though we managed to build btls-shared, this didn't solve the issues that users are having in https://github.com/godotengine/godot/issues/54758.

akien-mga commented 2 years ago

So according to https://github.com/godotengine/godot/issues/61364 the BTLS lib on Windows was actually working/useful (CC @zaksnet). So I'll revert my revert so we build it again on Windows.

Edit: Done: 2fa04b9dc73a9e84037ef165c88f0cddfaa8705b