lime
lime copied to clipboard
openfl
Restructure and update submodules
I'm not a fan of how Lime's submodules have been structured up until now. They were created by hand-copying files, and so in order to update them, you have to hand-copy all the new files. However, almost all of the source projects are public Git repos. This pull request takes advantage of that, pointing directly to these repos. Now, updating is simply a matter of checking out a more recent commit, and perhaps updating files.xml.
- I added a readme file to introduce people to the project folder, Lime's C++ code, and the submodules.
- I switched to libjpeg-turbo, since it has a public Git repo, and is also faster.
- We can (and must) now rebuild for Android using more recent NDKs. (Tested with NDK 21 so far.)
- liblzma is untouched, as it doesn't have a public Git repo.
Because it's so much easier to update submodules, I went ahead and updated almost all of them to the latest version. (Excluding OpenAL until hxcpp receives an update, and excluding LZMA since there's no official public Git repo.)
Of particular note: updating zlib fixes a significant security issue (CVE-2018-25032).
Because some changes had to be reverted in order to pull this off, I've compiled a list of differences. Hopefully, we can find some other way to resolve these issues, now that directly editing the source files is off the table.
Two of the differences are warnings we chose to suppress. They're now back, but is that really a big deal?
- [ ] https://github.com/openfl/libpng/commit/601ea3ce9b4af315bed7f371348305b28b3a231c
- [ ] https://github.com/openfl/libopenal/commit/e5b5a559075eff71b899094c19d887aec96e1ff3
Then there are some more specific changes:
- [ ] Removing a block of code related to font scaling.
- [x] Declaring a variable as
assigninstead ofretain. - [ ] Including libkern/OSAtomic.h.
None of these are explained in the commit history. CI builds seem to work fine without them, but I notice that the latter two are related to the HAVE_GCC_ATOMICS macro. Maybe the CI build defines it, hiding an error. Or maybe it'll work fine now that we updated SDL.
I tried updating to NDK r23, but it turns out both hxcpp and Pixman are incompatible with it. Until they're updated, we're stuck with NDK r21e. (But hey, it's better than being stuck with r15c.)
Almost forgot to mention: since this pull request switches to libjpeg-turbo, it resolves #1530.
Gave this PR a quick try today, and it seems to be working well on macOS. I didn't do any extensive testing, but clicking around the Feathers UI components-explorer didn't show any obvious issues.
It actually seems to fix an issue I've had with HTTP requests lately. When I rebuild lime.ndll on my local machine, they don't work on neko/cpp. However, I don't have the same issue when using lime.ndll from Github Actions. With your branch, HTTP requests work properly when I rebuild lime.ndll on my local machine.
I'm excited to see this merged.
Regarding this difference, I don't think anything needs to be done.
This is the single non-whitespace change, as you pointed out:
+ #if OS_OBJECT_USE_OBJC
@property (nonatomic, retain) dispatch_queue_t bleSerialQueue;
+ #else
+ @property (nonatomic, assign) dispatch_queue_t bleSerialQueue;
+ #endif
What the OS_OBJECT_USE_OBJC flag means, as well as when it's set, can be seen from its definition here. The change here only has an effect when OS_OBJECT_USE_OBJC is false, but for our purposes, it should always be true. In the hxcpp toolchain files, the current default iOS deployment target is 9, and the current default macOS deployment target is 10.9. This wasn't the case back when the change was made.
Of course, I can't say anything about why the change was made in the first place, but it seems inapplicable now.
I've tested this targeting iOS/macOS/Android/Hashlink so far and it all seems to work.
Is there anything specific you'd like help with to push this forward?
The main question is whether we want this to be part of the big 8.0.0 release. With a change this big, there's always the chance it'll break people's builds. I'd hate to release a year's worth of bug fixes along with a new bug that prevents them from updating. OTOH, there's the argument that this is what major version releases are for.
Besides that, it'd be nice to get a response about what the other unexplained changes were for, but at this point I'm not holding my breath.
Oh, and there's the matter of OpenAL's generated files taking up ~4MB of space. Would it be better to have lime rebuild run cmake to generate the files? That would require cmake to be installed...
Regarding the OpenAL issue, maybe requiring cmake isn't too terrible. After all, this is for people who are building lime from source. They already have git and a native toolchain anyway, things that aren't necessarily required when using lime otherwise.
Alternatively, they could just be part of the git repository. Although I don't prefer such an approach myself, there are lots of other things in the lime git repo that I normally wouldn't include in a source code repository either (like all the minified js in the dependencies folder and all the executables in the templates folders).
We're using the version of Clang that comes bundled with the Android NDK. I don't think any other version would work.
CI builds seem to work fine without them, but I notice that the latter two are related to the HAVE_GCC_ATOMICS macro. Maybe the CI build defines it, hiding an error. Or maybe it'll work fine now that we updated SDL.
it seems that CI builds work fine, but the app in armv7 is dieing with sdl error: cannot locate symbol "__atomic_fetch_sub_4" referenced by "(path)....../lib/arm/liblime.so"
what changes can cause this?
i can't say why, but recently, this started to happen in this lime version when installing submodules:
error: Server does not allow request for unadvertised object 2681e426ddaebc8e2764a7823b4b9d69564d1684
fatal: Fetched in submodule path 'project/lib/tinyfiledialogs', but it did not contain 2681e426ddaebc8e2764a7823b4b9d69564d1684. Direct fetching of that commit failed.
That's annoying. I guess the repo's settings mean we can only fetch the latest commit? Two days ago they made a new commit, which is probably what broke it.
I believe my change will fix it for you, but actions/checkout isn't so forgiving, hence the errors. Edit: I'm not going to bother trying to make the SourceForge server cooperate. We're doing a GitHub mirror.
Good news: the latest NDK is working! Bad news: only if you use my HXCPP pull request.
What is the current status of the pull request ? Using the latest submodules in official release looks great.
Glad to hear it! Can always use more eyes on a large change like this.
This won't be released until Lime 8.1.0 or 8.2.0. I don't think any of the TODOs would block it, but it would certainly be nice to check them off.
@flashultra reported the following error with libpng:
From https://github.com/freedesktop/pixman * branch 244383bf9f3493c014985de46876e40fd5db43f3 -> FETCH_HEAD Submodule path 'project/lib/pixman': checked out '244383bf9f3493c014985de46876e40fd5db43f3' error: Server does not allow request for unadvertised object a40189cf881e9f0db80511c382292a5604c3c3d1 Fetched in submodule path 'project/lib/png', but it did not contain a40189cf881e9f0db80511c382292a5604c3c3d1. Direct fetching of that commit failed.
This is similar to the error we saw with tinyfiledialogs, and I was never able to find a direct solution. Instead I switched to a GitHub mirror (ae25706), which is probably my plan here too. The main difference is that there's an official or semi-official mirror we can use.
The last change works for me . libpng in github looks official for me. My understanding about the error is that: As github modules contain SHA1 information for a specific commit some servers refuse to directly serve that SHA1. You can get only branches and tags. Maybe one possible solution will be to add branch, not to get the HEAD At example you need to specify the branch ( not tested) :
[submodule "project/lib/tinyfiledialogs"]
path = project/lib/tinyfiledialogs
url = https://git.code.sf.net/p/tinyfiledialogs/code
branch = master
Some other suggestions:
git submodule sync --recursive
git submodule update
and here is also small discussion : https://buildroot.uclibc.narkive.com/32IZXomj/v3-01-13-core-pkg-download-change-all-helpers-to-use-common-options#post15
Maybe one possible solution will be to add branch, not to get the HEAD
According to a highly-upvoted StackOverflow answer, even if you specify a branch, you still have to point at a specific commit. And even if that wasn't the case, we would still want control over which version we're pointing at. Specifying a tag would be ideal but doesn't seem possible.
git submodule sync --recursive git submodule update
There are certainly ways to make it work from the command line, but it also needs to work in CI. Unfortunately, actions/checkout offers only two options: submodules: true and submodules: false. Not a lot of room for configuration.
I'm trying to use TextField , but i've got the error TextField.hx:1753: characters 17-33 : lime.ui.Window has no field setTextInputRect
Maybe it's need to be merged with the latest master branch ?
~~Merging wouldn't help. Judging from TextField.hx, the problem is that Haxelib isn't reporting the correct version of Lime. For whatever reason, Haxelib judges library version based off the folder name, not haxelib.json.~~
~~I'd say the easiest workaround is to rename the folder:~~
~~> cd [haxelib folder]/lime~~
~~> mv git 8,1,0~~
~~> haxelib set lime 8.1.0~~
I'm usng lime from submodules branch and there is no method setTextInputRect It exist here : https://github.com/openfl/lime/blob/develop/src/lime/ui/Window.hx#L547 but not in submodule branch: https://github.com/player-03/lime/blob/submodules/src/lime/ui/Window.hx#L542 Renaming my submodules folder to 8,1,0 did not help.
I must have misread the code. I thought the first #end said #else, meaning it would only run setTextInputRect() if Lime was older than 8.0.0.
That also means Haxelib might have fixed version number detection. Neat!