Update hashlink.

[Apprentice-Alchemist]

Update hashlink to latest version (1.12), and add all the hdll's required for the haxe std.
Also change the names of exported functions in lime.hdll, in the interest of making future HL/C support easier.
Closes #1516.

Tested on Linux and Windows.

[player-03]

Also change the names of exported functions in lime.hdll, in the interest of making future HL/C support easier.

I'm unsure how this makes things easier. Are apps going to have to load from both lime.hdll and lime.ndll, causing name conflicts? Are you saying it's clearer to have unique names?

---

The fact that this is all one commit makes it hard to follow. If it was me, I'd break it up into three commits:

1. Fix # 1516. (That's just line 3950 of ExternalInterface.cpp plus line 24 of KeyEvent.h, right?)
2. Update HashLink. (That's just hl.h, as far as I can tell.)
3. Rename the functions. (Everything else.)

Even with simple commit messages, this will make it much easier to see why each change was made. (Though for the third commit I'd recommend a sentence or two of explanation.)

[Apprentice-Alchemist]

Fix # 1516. (That's just line 3950 of ExternalInterface.cpp plus line 24 of KeyEvent.h, right?)

Correct

Update HashLink. (That's just hl.h, as far as I can tell.)

hl.h + the binaries

I'm unsure how this makes things easier. Are apps going to have to load from both lime.hdll and lime.ndll, causing name conflicts? Are you saying it's clearer to have unique names?

It makes it easier by doing what the haxe compiler expects us to do.
When building to HL/C, apps would have to be linked with lime.hdll.
At the moment the functions exported in lime.hdll (those marked with HL_PRIM) are named hl_lime_foo, whereas the C code generated by the haxe compiler would expect functions with named lime_lime_foo.

Now the functions are named lime_hl_foo, and the haxe code has been changed accordingly.

[Apprentice-Alchemist]

I've split the commit up into three, as you suggested.

[player-03]

I've split the commit up into three, as you suggested.

Perfect!

hl.h + the binaries

Ah, of course.

At the moment the functions exported in lime.hdll (those marked with HL_PRIM) are named hl_lime_foo, whereas the C code generated by the haxe compiler would expect functions with named lime_lime_foo.

So you're saying HashLink/C was broken, and this was never detected because we'd only tested HashLink bytecode. (It wouldn't be the first time something like that happened...)

Just to make sure I can understand, wouldn't it have required fewer changes to switch to lime_lime_foo? You would still have needed to update all the .cpp files, but it wouldn't have required so many changes on the Haxe side of things. If you went to the extra effort, you must prefer lime_hl_foo. Is this just for clarity?

[Apprentice-Alchemist]

So you're saying HashLink/C was broken, and this was never detected because we'd only tested HashLink bytecode.

Yes.

Just to make sure I can understand, wouldn't it have required fewer changes to switch to lime_lime_foo? You would still have needed to update all the .cpp files, but it wouldn't have required so many changes on the Haxe side of things. If you went to the extra effort, you must prefer lime_hl_foo. Is this just for clarity?

Personally I think lime_hl_foo is clearer than lime_lime_foo.
And the only changes on the Haxe side are to the @:hlNative metadata's.
The function names on the haxe side are still exactly the same as before.

[Apprentice-Alchemist]

I cannot reproduce the Windows build failure on my machine, so I don't know what's wrong.

As for the Android failure, Lime seems to be using API level 16, whereas uchar.h was added in API level 21.
(API level 16 is Android 4.1, which was released in 2012)
(API level 21 is Android 5.0, which was released in 2014)
According to Android Studio, 98% of Android devices run API level 21 or higher, so I think it would be okay to use API level 21.

Alternatively I can try patching Hashlink, but I would really like to avoid that.

[player-03]

As for the Android failure, Lime seems to be using API level 16, whereas uchar.h was added in API level 21.

Huh, well that led me down a rabbit hole.

Lime targets API level 28 but loads header files from the android-16 folder. According to 6dce7ad, the folder's number has to match the minimum SDK version, which is why that's also 16.

Lime does already use android-21 for 64-bit builds, because previous ones didn't support it. Everything else uses android-16 to maximize backwards compatibility. Interestingly, the NDK we're using goes as high as android-26, but since Lime is all about backwards compatibility, that isn't likely to happen.

If you want to update the minimum SDK version - and I agree you have a good case for it - please submit it as a separate pull request, so we can discuss it separately. (You can use 6dce7ad as a template - search for the strings "android-14" and "android-16".)

[inovento]

We can repro the build error on Windows namely the halt at JPEG.cpp. How can we help to track the root cause down?

[player-03]

Does it still fail if you combine this with #1519?

[inovento]

That’s the only way we tried and it fails. Haven’t tried without that commit.

[player-03]

I'm surprised it's the same error. I don't know what's up with that, but I'll take a closer look later.

[Apprentice-Alchemist]

I can also reproduce the Windows error now, I think I forgot to checkout my branch when I tested it last time.

The issue is the same as here: native-toolkit/libjpeg#2.

[player-03]

Interesting. So the solution is to comment #include <Windows.h>? (Presumably both times?)

[Apprentice-Alchemist]

Or to merge the pull request I linked (or even to move to libjpeg-turbo).

Personally I would prefer to keep our bundled hashlink headers as close as possible to upstream.
The less patches, the easier to update.

(Ideally we could even add upstream hashlink as a git submodule and make lime rebuild hl rebuild that too, making it a lot easier to update hashlink, it would just be a question of doing git -C path/to/hl pull origin master.)

[player-03]

I'd love to see HL added as a submodule, but I'm pretty sure Lime makes several other modifications. All of them will have to be undone before we can pull it directly.

[Apprentice-Alchemist]

The only modification lime makes at the moment is renaming the DEFINE_PRIM macro to DEFINE_HL_PRIM (or something like that), and that can be fixed with a bit of #undef magic.

[Apprentice-Alchemist]

So, hashlink is now a submodule.
In theory CI should pass now (at least for Linux and macOS).
Windows requires native-toolkit/libjpeg#2.

[Apprentice-Alchemist]

And of course I forget to install the bloody packages.
And macOS is being difficult, as usual.
Oh well, I'll deal with that tommorow.

[Apprentice-Alchemist]

I think the iOS failure is because char16_t and char32_t are defined in C++ (but not C, at least on iOS).
hl.h tries to define those types, since it's included in a C++ files, we get an error.

[Apprentice-Alchemist]

I've updated the PR to use the latest version of hashlink (1.12, which is now released).

[ShaharMS]

Is there any reason why this isn't merged?

[player-03]

It seem to work on my machine. At least, I can rebuild lime.hdll and test DisplayingABitmap.

Looking at BuildHashlink.xml (and LinuxPlatform), it looks like lime.hdll doesn't incorporate any of the other libraries (Cairo, SDL, etc.). And that would explain why you compile lime.ndll at the same time: Hashlink apps require both. Is that right?

Yet somehow lime.hdll is slightly larger than lime.ndll... Maybe that's just because Hashlink includes its own copies of several of the same libraries (inconveniently spread across two folders).

[Apprentice-Alchemist]

BuildHashlink.xml builds hl(.exe), libhl.so/dylib/exe, ssl.hdll, fmt.hdll, mysql.hdll, ui.hdll and uv.hdll (these hdlls are needed for functionality of the Haxe std, eg sys.ssl depends on ssl.hdll).
On Windows these indeed use hashlink's copies of the libraries, on Linux/macOS they use system packages.

lime.hdll is still built by Build.xml and includes all of lime's dependencies (SDL, Harfbuzz, etc).

Hashlink apps do not depend on lime.ndll, only on lime.hdll (and the hashlink runtime of course)

(inconveniently spread across two folders).

/include contains the copies of the libraries, /libs contains the wrapper code.

[player-03]

Looking back through open issues, I came across #1524, which still needs resolution. Is fmt.hdll one of the binaries you added?

[Apprentice-Alchemist]

Yes, all the binaries needed for things in the Haxe standard library are added.
(See previous message.)

[player-03]

Oh hey, you did mention it. Whoops.

[ShaharMS]

lets gooo this got merged!

[joshtynjala]

If anyone has issues rebuilding after this pull request was merged, don't forget to update submodules!

I got the following error on macOS:

In file included from ./src/ui/TextEvent.cpp:1:
include/system/CFFI.h:6:10: fatal error: 'hl.h' file not found
#include <hl.h>
^~~~~~
1 error generated.

But it was an easy fix:

git submodule update --recursive

[joshtynjala]

Getting the following error when I try lime test hl on macOS:

dyld[29362]: Library not loaded: @executable_path/libhl.dylib
Referenced from: /Users/joshtynjala/Development/feathersui/feathersui-openfl/samples/components-explorer/bin/hl/bin/Components.app/Contents/MacOS/Components
Reason: tried: '/Users/joshtynjala/Development/feathersui/feathersui-openfl/samples/components-explorer/bin/hl/bin/Components.app/Contents/MacOS/libhl.dylib' (no such file), '/usr/local/lib/libhl.dylib' (no such file), '/usr/lib/libhl.dylib' (no such file)

It appears to be looking for libhl.dylib in the wrong location. When I "Show Package Contents" in the .app file, it is actually located here:

/Users/joshtynjala/Development/feathersui/feathersui-openfl/samples/components-explorer/bin/hl/bin/Components.app/libhl.dylib

[Apprentice-Alchemist]

Hmm... I'll have to look again at were the tooling copies things.
I'd have expected it to copy the libhl library to the same place were it puts the executable (although that's not the right place for libraries iirc).

[player-03]

If anyone has issues rebuilding after this pull request was merged, don't forget to update submodules!

That's going to come up a bunch in the near future.

tried: '/Users/joshtynjala/Development/feathersui/feathersui-openfl/samples/components-explorer/bin/hl/bin/Components.app/Contents/MacOS/libhl.dylib' (no such file)

A search for "Contents/MacOS" turns up MacPlatform.hx and a bunch of SDL files, meaning the problem lies in MacPlatform. Some code that's using executableDirectory should instead use applicationDirectory, or vice versa. Maybe this line is copying the files into the app root when it should be copying them into executableDirectory?

[joshtynjala]

I'd have expected it to copy the libhl library to the same place were it puts the executable (although that's not the right place for libraries iirc).

Just in case it helps, I should point out that I saw that most of the .hdll files are in the root of the .app directory with the libhl.dylib too. lime.hdll in in Contents/MacOS, though.

[player-03]

Found the line responsible for libhl.dylib in particular.

System.copyFile(Path.combine(hlPath, "libhl.dylib"), Path.combine(applicationDirectory, "libhl.dylib"));

And here's what that line was replacing.

-// System.copyFile(targetDirectory + "/obj/ApplicationMain" + (project.debug ? "-Debug" : "") + ".hl",
-// 	Path.combine(executableDirectory, project.app.file + ".hl"));
-System.recursiveCopyTemplate(project.templatePaths, "bin/hl/mac", executableDirectory);
-// let's not keep around hxcpp's hash files
-for (file in System.readDirectory(applicationDirectory))
-{
-	if (Path.extension(file) == "hash")
-	{
-		System.deleteFile(file);
-	}
-}
-System.copyFile(targetDirectory + "/obj/ApplicationMain.hl", Path.combine(executableDirectory, "hlboot.dat"));
-System.renameFile(Path.combine(executableDirectory, "hl"), executablePath);
+HashlinkHelper.copyHashlink(project, targetDirectory, applicationDirectory, executablePath);

The old code mostly referenced executableDirectory with one instance of applicationDirectory, but now the new code only has access to applicationDirectory. That'd explain why it puts it in the wrong place. And I think the instance of applicationDirectory was a mistake. If it was supposed to clean up after recursiveCopyTemplate(), it should have used executableDirectory too.

[player-03]

@joshtynjala Since I'm not set up for Mac testing, can you be the one to change line 209 and push the commit? I'm sure it's the answer, I just want to do things properly.

[joshtynjala]

Changing from applicationDirectory to executableDirectory on Line 209 seems to be the correct solution there. The .dylib and .hdll files are now placed in Contents/MacOS, and lime test hl is working for me.