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

For cross-compiling to Windows/macOS/Linux, respect -hl flag. #1610

Open
wants to merge 2 commits into
base: 8.2.0-Dev
Choose a base branch
from

Conversation

Apprentice-Alchemist
Copy link
Contributor

@Apprentice-Alchemist Apprentice-Alchemist commented Dec 13, 2022

This changes the behaviour of lime build <windows|mac|linux> by respecting the users preference for HL or Neko (current behavior is to force Neko).

Also changes the default target for cross-compilation to HL.

@Apprentice-Alchemist Apprentice-Alchemist changed the title For cross-compiling to Windows/macOS/Linux, default to HL unless user… For cross-compiling to Windows/macOS/Linux, respect -hl flag. Dec 13, 2022
@player-03
Copy link
Contributor

This feels to me like a new feature rather than a bug fix. Could you target 8.1.0-Dev instead? (Probably requires cherry-picking and a force-push.)

@Apprentice-Alchemist Apprentice-Alchemist changed the base branch from develop to 8.1.0-Dev April 16, 2023 18:00
@tobil4sk
Copy link
Member

This will cause issues with #1661. I think it might be good if the code for deciding whether to use hl or neko (or cpp) is moved here, where it sort of already is (though it later gets ignored):

else if (platformType == DESKTOP && target != cast System.hostPlatform)
{
defines.set("native", "1");
if (target == Platform.WINDOWS)
{
defines.set("targetType", "cpp");
defines.set("cpp", "1");
defines.set("mingw", "1");
}
else
{
defines.set("targetType", "neko");
defines.set("neko", "1");
}
}

@player-03
Copy link
Contributor

As an HXP project user, I wouldn't want to put this in ProjectXMLParser.

It would be nice to clean up some of this duplicate code, but I'm not sure there's a good place to put it. Also, it isn't an exact duplicate, which I'm sure is why it was split up into all the "Platform" classes in the first place.

@tobil4sk
Copy link
Member

As an HXP project user, I wouldn't want to put this in ProjectXMLParser.

Ah, right I completely missed that... yes then it probably should all be cleaned up and moved into the platform files. I'm worried that some of these defines might be missing in hxp projects then?

@player-03
Copy link
Contributor

As for which one should be the default, it's hard to say. If I type lime test windows on Windows, I'm expecting a C++ build, not HL. On the other hand, MinGW is one of a few similar options, and not one I'm frequently exposed to as a Haxe dev, so I wouldn't be expecting that either.

Honestly, I think if there's no obvious default, maybe we shouldn't have one. Maybe the fall-through behavior should be an error message along the lines of "Please specify -hl or -mingw to cross-compile to Windows."

I'm worried that some of these defines might be missing in hxp projects then?

Can't rule it out. The HXP format doesn't get as much attention as I think it deserves.

@player-03
Copy link
Contributor

Actually, hang on.

current behavior is to force Neko

Are you saying that if I typed lime build windows on Mac or Linux, it would compile an ordinary .n file, just as if I'd typed lime build neko?

If so, is there a difference between lime build windows -hl and lime build hl? (I'm certainly more willing to believe this in the case of HL, but I want confirmation.)

@Apprentice-Alchemist
Copy link
Contributor Author

Actually, hang on.

current behavior is to force Neko

Are you saying that if I typed lime build windows on Mac or Linux, it would compile an ordinary .n file, just as if I'd typed lime build neko?

... I've just checked and current behavior is actually quite messed up.
lime build windows on Linux will give you a Game.exe that's actually a Linux binary.
Lime just invokes nekotools boot which on Linux creates a Linux binary.
(nekotools boot just takes the neko executable and embeds a .n file)

If so, is there a difference between lime build windows -hl and lime build hl? (I'm certainly more willing to believe this in the case of HL, but I want confirmation.)

On Windows there is no difference, on Mac/Linux lime build windows -hl should result in (real) Windows binaries, whereas lime build hl would result in a mac or linux build.
(Note that this only works with the haxelib releases, a development build on Linux won't have HL Windows binaries)

@player-03
Copy link
Contributor

Lime just invokes nekotools boot which on Linux creates a Linux binary.

Whoops! Yeah, we gotta change that.

on Mac/Linux lime build windows -hl should result in (real) Windows binaries, whereas lime build hl would result in a mac or linux build.

Good. Then we definitely want to support build windows -hl.

As for build windows alone, how do we pick between HL and MinGW? Both should be capable of making a distribution-ready binary, but I'd assume HL compiles faster while MinGW has better runtime performance. Maybe MinGW is better since you want the final product to perform well? Or maybe HL is better because that's what we'll use for Mac-Linux cross-compilation, and we should default to the most consistent option?

@tobil4sk
Copy link
Member

(Note that this only works with the haxelib releases, a development build on Linux won't have HL Windows binaries)

Similar situation with MinGW currently. The haxelib release provides a Windows build of lime.ndll, but using a git version of lime you'd have to compile the lime.ndll yourself. Currently it doesn't compile with MinGW, though this might be solvable with some more work in #1660.

On the other hand, if a haxelib release of lime is installed, it sounds like the user should already have everything for a hashlink cross-build? MinGW requires additional packages installed on the system. So from that perspective, it might be better to have a default that doesn't depend on external packages? At least, for regular users who install Lime from haxelib.

From my perspective, I'm not too fussed about which is the default, adding -mingw is not too big an inconvenience. I think cross compiling is specific enough a use case that it would be expected for a user to read up on the documentation to figure out exactly what the cross-compilation builds are doing.

@player-03 player-03 changed the base branch from 8.1.0-Dev to 8.2.0-Dev August 18, 2023 01:11
@DigiEggz
Copy link

Lime just invokes nekotools boot which on Linux creates a Linux binary.

Whoops! Yeah, we gotta change that.

on Mac/Linux lime build windows -hl should result in (real) Windows binaries, whereas lime build hl would result in a mac or linux build.

Good. Then we definitely want to support build windows -hl.

As for build windows alone, how do we pick between HL and MinGW? Both should be capable of making a distribution-ready binary, but I'd assume HL compiles faster while MinGW has better runtime performance. Maybe MinGW is better since you want the final product to perform well? Or maybe HL is better because that's what we'll use for Mac-Linux cross-compilation, and we should default to the most consistent option?

I think choosing the best option for the end user's experience is most important. I'd vote for natively using minGW instead of HashLink. The dev can always change target with -hl if ever needed, but I think the default should be pointed toward runtime performance.

@player-03
Copy link
Contributor

I'm thinking HXProject should make the final decision of what platform to compile to, so that it can add all the correct flags. (Specifically, we'd move all the project.target != System.hostPlatform checks into there.) This way, if the target changes to HL or MinGW or something, it'll happen early enough for the user to check for it in project.xml.

The CommandLineTools.hx change is still good, since that happens even earlier on.

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.

4 participants