Mingw cross compiler support.

[zombieCraig]

Added host detection in SConstruct and set the flags for mingw++ compiling for linux and osx for compiling windows.

[beniwtv]

Hey I've been meaning for some time to implement this myself, so thanks for working on this!

A couple of comments:

• I would leave the platform= argument as it works currently, as that is used this way in other Godot-related projects (and Godot itself), so it would still be the same for us and not break and expectations / current scripts people may have
• Instead, I suggest specifying the target platform as a target_platform= argument, if it is not given assume the target is the same as the current platform

:)

[BastiaanOlij]

@beniwtv I don't think @zombieCraig is changing that behavior, whats new here is that you can perform the windows platform build on a non windows platform and therefor require to find out the host platform.

Godot I believe does the same for its cross compiling. I've never experimented with it myself

I need to take a closer look but if my understanding is correct I'm happy to merge this in. My only preliminary remark is whether it makes sense to check the host platform so early as its now also checked when you're compiling a linux build on linux, not that it would do any harm.

[zombieCraig]

Oh yes, Sorry I didn't respond earlier. I saw the message then got distracted and forgot to reply. The behavior should be the exact same. I just add an extra check for mingw and I did it the same way godot core does it. That's actually the reason there is some extra code. It was just to make it work in almost the exact same way with the same variable names. I'm not very familiar with SCons so let me know if I should redo this.

On Thu, Jul 23, 2020 at 9:45 PM Bastiaan Olij [email protected] wrote:

@beniwtv https://github.com/beniwtv I don't think @zombieCraig https://github.com/zombieCraig is changing that behavior, whats new here is that you can perform the windows platform build on a non windows platform and therefor require to find out the host platform.

Godot I believe does the same for its cross compiling. I've never experimented with it myself

I need to take a closer look but if my understanding is correct I'm happy to merge this in. My only preliminary remark is whether it makes sense to check the host platform so early as its now also checked when you're compiling a linux build on linux, not that it would do any harm.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GodotVR/godot_openvr/pull/99#issuecomment-663344442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAELD5PRWRB2TQM5R4252CDR5EGYDANCNFSM4OOKP7XA .

[lyuma]

I'd like to put in a vote for this too!

I actually independently made an almost identical change, available here: https://github.com/lyuma/godot_openvr/commit/a71fde0c18c9af02e56f61f9d5fbda6343d878ad Only main difference is I moved host_platform detection into the windows case.

I think it's a good approach. Tested my variant on Linux (both native linux and mingw64) and on windows (msvc 2017)

[beniwtv]

Ah yes I must have mistaken that variable, sorry, I got mixed up and thought platform was being overwritten :) Looks good to me then.

[beniwtv]

Hi managed to test this successfully with two small issues:

1. In line 63 and 66 scons was complaining about mixed tabs and spaces for indentation
2. The resulting file generated libgodot_openvr.so as name, even though according to file it is a proper Windows dll file.

:)

[zombieCraig]

Oh no! :facepalm: I can't tell you how many times python has bitten me for things like that. My IDE is VI, and I only usually code python patch others code. Needless to say, whitespace programming languages and myself do not get along. I'll fix that and resubmit.

[lyuma]

Note: If you build with MinGW (gcc or llvm variants), you will run into this bug. https://github.com/ValveSoftware/openvr/issues/103

I'm going to see if the proposed fix works, running this script on openvr.h to generate a compatible version: https://gist.github.com/tunabrain/1fc7a4964914d61b5ae751d0c84f2382

I'll let you know if I am able to make headway on this crash, but basically OpenVR is designed using a C++ ABI and it's a bit scary to link code made with two different compilers

Just a heads-up so you don't spend lots of time debugging crashes.

Also, to workaround the .so problem, add this into the if statement:

    env['target_name'] += ".dll"

I have a complete diff if you want it, which includes llvm-mingw support, PDB generation using / -gcodeview in CCFLAGS and "-Wl,-pdb=" in LINKFLAGS, which I needed to figure out the above crash :-p

[beniwtv]

Thanks for the heads up on the crash!

We did link LLVM with GCC compiled things when GCC was causing a crash before, and there weren't any issues, but I guess you never know!

[lyuma]

@beniwtv Got it working!!

I have pushed my version of wrap_mingw.py here: https://github.com/lyuma/godot_openvr/blob/mingw/wrap_openvr.py

You can see my commits here: https://github.com/lyuma/godot_openvr/tree/mingw It includes llvm-mingw support and I get nice .pdb files.

(Note, requires a patch to replace env = DefaultEnvironment with env = Environment(ENV=os.environ) in SConstruct, but that's a different request for a different day.)

Here are the commands I am running from my CI system:

python wrap_openvr.py
rm -f demo/addons/godot-openvr/bin/win64/libgodot_openvr.dll
PATH=/opt/llvm-mingw/bin:$PATH scons werror=no platform=windows target=release -j`nproc` use_lto=no deprecated=no use_mingw=yes use_llvm=yes use_lld=yes use_thinlto=yes LINKFLAGS=-Wl,-pdb= CCFLAGS='-g -gcodeview' debug_symbols=no

[lyuma]

Good news, Benedikt has updated wrap_openvr.py to have a MIT license: https://gist.github.com/tunabrain/1fc7a4964914d61b5ae751d0c84f2382

would a PR for this be helpful? I'd be curious if anyone else has run into the issues I had when using a HMD with mingw-compiled libgodot_openvr.dll (the crash only happens when actually running in VR)

[beniwtv]

We'd need to run this file either manually or via scons during (before) cross-compile right? I have compiled the .dll, but as I have no Windows I am unable to test if it crashes without this wrapper, but I guess if it does for your build mine would be the same.

The Python script creates a wrapper, that uses OpenVR's C code instead of the C++ one, so we are using the C ABI which is more compatible, as I understand it.

@BastiaanOlij Do you think this would be an acceptable approach for us?

[BastiaanOlij]

If we can script it so it only does that when the mingw build is run, I don't have anything against that solution. It's no different than the extra step done on MacOS

[BastiaanOlij]

This needs a rebase

[beniwtv]

Since we now have automated builds for every platform, is this still something we want to pursue?

[zombieCraig]

I haven't tried the new system but if I can build on Linux then we don't need this.

On Mon, Nov 16, 2020 at 9:46 AM Benedikt [email protected] wrote:

Since we now have automated builds for every platform, is this still something we want to pursue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GodotVR/godot_openvr/pull/99#issuecomment-728220254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAELD5MFIEWFXO7Y3LGARYDSQFQQ3ANCNFSM4OOKP7XA .

[lyuma]

@beniwtv Apologies for being a bit out-of-the-loop on this, but what exactly is the new automated builds approach?

If this is a reference to GitHub Actions, unfortunately that is not an acceptable approach for us. GitHub Actions is proprietary (even if free-as-in-beer), and requirements for my project are that we be able to use our own hardware to compile releases... Running Visual Studio emulated with Wine might be possible, but it sounds like a maintenance nightmare.

@zombieCraig mentions Linux. If there is a fix for Linux based (non-MS VIsual Studio) compilers such as mingw GCC, llvm-mingw, would it be possible to point me in that direction?

[beniwtv]

@lyuma Yes, the automatic builds won't then fulfill those requirements then -> so we still need this. Feel free to rebase & resend :)