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

[GeanyLua] For version 5.4.3? #1133

Open
jradxl opened this issue Nov 10, 2021 · 32 comments · May be fixed by #1233 or #1238
Open

[GeanyLua] For version 5.4.3? #1133

jradxl opened this issue Nov 10, 2021 · 32 comments · May be fixed by #1233 or #1238

Comments

@jradxl
Copy link

jradxl commented Nov 10, 2021

Any change this plugin will be upgraded to Lua 5.4.3 any time soon?

@elextr
Copy link
Member

elextr commented Nov 11, 2021

Since Geanylua is orphaned (ie has no maintainer) I suspect it will be a case of someone needing to do it.

@xiota
Copy link
Contributor

xiota commented Mar 2, 2023

LuaJIT appears to be a drop-in replacement for Lua 5.1 that would require only build script or other minor changes. Based on Git activity, LuaJIT is actively maintained. Perhaps GeanyLua could switch to LuaJIT (issue #1228)?

@elextr
Copy link
Member

elextr commented Mar 2, 2023

As I noted above, the plugin is orphaned, so somebody needs to take it over and then make the change.

@xiota
Copy link
Contributor

xiota commented Mar 2, 2023

I am willing to look into what would be necessary to switch to LuaJIT, but if there would be opposition to such a change, it wouldn't be worth the effort.

In principle, switching to LuaJIT should be easier than switching to Lua 5.4+ because LuaJIT is intended to be a drop-in replacement for Lua 5.1. LuaJIT appears to be actively maintained, while Lua 5.1 has been deprecated for nearly two decades. At the moment, it's working, but there's a possibility that it will eventually be dropped by some distros or stop compiling because of some future gcc, glibc, or other update.

@elextr
Copy link
Member

elextr commented Mar 3, 2023

Well, there is no plugin maintainer to object ;-)

The simplest solution would be just to make it a new plugin ("Geanyluajit" maybe) until its stable and supports the platforms the existing one does. That also means a Geanylua is still available while the distros get their heads around the new dependency.
Then Geanylua can be simply deleted when it breaks.

I can't figure out if the bundles include Geanylua and if a bundled lua dll is included for Windows (@eht16) or OSX (@techee)?

@techee
Copy link
Member

techee commented Mar 3, 2023

It's not included for macOS - I don't remember why, if it was just that I didn't try or know how to use the plugin, or if there was some actual problem. Right now the macOS binary uses the "hardened runtime" where JIT is disabled but one can easily add a certain flag and enable it so it shouldn't be a problem.

@techee
Copy link
Member

techee commented Mar 3, 2023

Lua 5.4 is not a drop-in replacement for Lua 5.1, so it would require major rewrites.

Just out of curiosity, what are the changes that make it hard to migrate to the new version?

@xiota
Copy link
Contributor

xiota commented Mar 3, 2023

Just out of curiosity, what are the changes that make it hard to migrate to the new version?

I'm not familiar with Lua, but took a look a while ago. I saw accumulated API-breaking changes. Compatibility functions/macros did exist between adjacent releases, but those that were of interest had been deprecated and removed by 5.4. Both 5.1 and 5.4 APIs would have to be learned well enough to almost be able to rewrite the plugin from scratch. Further, a 5.5 release, with more API-breaking changes, could be just a couple years away. (5.4 was released about 2.5 years ago. Average time between 5.x releases is about 4.5 years. Longest is 6 years.)

From LuaJIT Installation:

LuaJIT is API-compatible with Lua 5.1. If you've already embedded Lua into your application, you probably don't need to do anything to switch to LuaJIT, except link with a different library...

So it seems there's potential, with minimal effort, to switch to a maintained library with reduced concern for future API-breaking changes.

LuaJIT is supported on Windows and macOS. Packages are available for Debian and Fedora.

@xiota
Copy link
Contributor

xiota commented Mar 3, 2023

I just tried rebuilding GeanyLua with LuaJIT. It's fairly simple:

  • Edit geanylua/*.c to use luaL_Reg instead of luaL_reg:
    • The real name of the struct is luaL_Reg. luaL_reg is defined for compatibility.
    • LuaJIT does not include the define for luaL_reg.
  • Update build/geanylua.m4 to:
    • Refer to luajit instead of lua5.1
    • Use versions between 2.0 and 3.0 instead of 5.1 and 5.2.

Would a single PR be acceptable, or would separate PRs be preferred?

@techee
Copy link
Member

techee commented Mar 3, 2023

I'm just wondering - what problem does the migration to luajit try to solve? As far as I can say, Lua 5.1 is available as a package and offered by distributions. I've just checked luajit's open bugs and one of them is this one

LuaJIT/LuaJIT#559

where it doesn't support pointer authentication which is used on ARM-based Apple computers:

https://support.apple.com/guide/security/operating-system-integrity-sec8b776536b/web

If there is some compelling reason to move to luajit, it might be worth it and sacrifice the macOS version of the plugin (which doesn't exist yet anyway) but I haven't seen such a reason mentioned above.

@elextr
Copy link
Member

elextr commented Mar 3, 2023

geany/geany-osx#41 (comment) seems to suggest that the plugin will never work on Windows or Wayland either.

If "somebody" contributed it I bet it could just hook into the Geany keyevents and use GDK platform independently like Geany does.

@xiota
Copy link
Contributor

xiota commented Mar 3, 2023

There's no "problem" to "solve", per se.

  • Changing luaL_reg to luaL_Reg would make for one less thing to fix when/if switching to a different Lua release, whether 5.4+ or LuaJIT.
  • LuaJIT performs better, even with JIT turned off. Probably doesn't matter much for Geany.
  • Switching to something else improves the chances of GeanyLua becoming maintained. Those with Lua 5.1 expertise have likely moved on to later 5.x releases or LuaJIT. Those without existing expertise are more likely to be willing to learn 5.4+ or LuaJIT.
  • Since LuaJIT is a drop-in replacement for Lua 5.1, switching back and forth wouldn't be difficult. The build/*.m4 script might even be able to detect and support both.
  • LuaJIT supports macOS, so it might be possible to make the currently non-existent (Intel) macOS version of the plugin exist. To workaround the ARM64e issue, Lua 5.1 could be used, if it works. If not, it's a wash and there is no "sacrifice" either way. Since LuaJIT is maintained, there's a reasonable chance the issue will be fixed sometime around when mainline Linux fully supports Apple M# processors.
  • LuaJIT has some interesting features of its own, but that could break Lua 5.1 compatibility.
  • Lua 5.1 is at risk of being dropped someday. Maybe not for another two decades, but the number of packages that use it is dwindling. In the distro I'm currently using, there are seven packages that depend on Lua 5.1, after excluding Lua 5.1 and modules. I recall that there were more in the past. At least one appears to have dropped Lua support entirely, rather than migrate. LuaJIT is almost as popular as Lua 5.4. There are nearly three dozen packages using LuaJIT, including gegl, mpv, obs, and vlc. There are over three dozen packages using Lua 5.4, while Lua 5.2 and 5.3 are unused.

@elextr
Copy link
Member

elextr commented Mar 4, 2023

IIUC the reason things like gegl moved to luajit was for speed, its a pixel manipulation library after all, they really don't care about whatever the new features of Lua 5.4 are since they just use Lua as an extension language, not an application language. And now there is an ecosystem of extensions in Lua 5.1 I doubt they want to upgrade to Lua 5.4 until Luajit supports it and then only if it is strictly Lua 5.1 backward compatible so existing Lua extensions work.

I would have thought Geanylua is in the same category, performance that minimises UI delays is important if Lua code runs between keystrokes, writing whole applications in Geanylua is not.

Given that Geanylua only appears to work on Linux X11 it is likely to become unusable from that before Lua 5.1 is removed, so I'm not sure if @xiota wants to invest the effort in porting to Luajit or Lua 5.4 before that is fixed.

Also @kugel- does Peasy support luajit?

@xiota
Copy link
Contributor

xiota commented Mar 4, 2023

Given that Geanylua only appears to work on Linux X11 it is likely to become unusable from that before Lua 5.1

Wayland is still feature incomplete. The number of distributions supporting X11 hasn't dwindled to a mere handful yet.

Are there any other X11 issues besides the key event hooks?

I'm not sure if xiota wants to invest the effort in porting to Luajit or Lua 5.4 before that is fixed.

I'm already using GeanyLua with LuaJIT. "Porting" was mostly changing the build script. My existing (simple) Lua scripts are working as expected with no change.

I keep revisiting GeanyLua because it's useful, but dislike using/working on it because it's on a deprecated for nearly two decades branch of Lua. LuaJIT seems to be the most expedient path.

@elextr
Copy link
Member

elextr commented Mar 4, 2023

Wayland is still feature incomplete.

Thats intended, Wayland was invented to drop old historic parts of X11, but maybe it dropped too much at the start.

The number of distributions supporting X11 hasn't dwindled to a mere handful yet.

No distro is going to "not support X11" any time soon, thats what XWayland is for. Where the problem comes is that if the Geany packaged by the distro defaults to running on Wayland then X11 events won't be received, so that Geanylua code won't work, or even worse will attempt to interpret a Wayland event as an X11 event.

And Wayland by default is increasing, Ubuntu 22.04 is Wayland by default IIUC, Fedora has been for a while, Debian Gnome since Debian 10 IIUC.

Are there any other X11 issues besides the key event hooks?

AFACGSCT no (As Far As Crappy Github Search Can Tell ;-)

LuaJIT seems to be the most expedient path.

If its only a build change then basically I agree. But the X11 problem still remains, I will open a separate issue for that.

@hyperair how do we make sure packagers are aware that the dependency of a plugin changed?

@techee
Copy link
Member

techee commented Mar 4, 2023

geany/geany-osx#41 (comment) seems to suggest that the plugin will never work on Windows or Wayland either.

It's not that bad. First, there's a native code for Windows for this already in the plugin. Second, the whole code I think just serves for exposing some key event grabbing to lua scripts which I think isn't so essential - I think users won't make some advanced GUI stuff with the plugin so if this functionality isn't available on macOS or Wayland, nothing too bad will happen. One can just ifdef-out the code starting from here

#ifndef G_OS_WIN32

and ending here

plus the line here

{"keygrab", glspi_keygrab},

on these platforms and the whole plugin compiles (I succeeded to compile it on macOS, I just did run into some autoconf-fighting issues so didn't get it running yesterday, will try when I have more time).

Since LuaJIT is maintained, there's a reasonable chance the issue will be fixed sometime around when mainline Linux fully supports Apple M# processors.

We are not talking about linux here but native macOS build which would break because of the pointer authentication issue. IMO much better thing would be to keep using lua and rather try to port the plugin to the latest lua version. I don't think performance is an issue here as I would suspect users use this plugin for some simple extensions rather than CPU-intensive tasks.

@elextr
Copy link
Member

elextr commented Mar 4, 2023

First, there's a native code for Windows for this already in the plugin.

Yeah, I belatedly noticed, see edit on #1230 (comment)

As noted in that issue, I think it can be implemented by connecting to proper GTK key-press and key-release signals and be portable.

As for the luajit vs Lua 5.4.3, apart from unknown work to port it, is 5.4.3 backward compatible at the Lua level, or will scripts break?

@xiota
Copy link
Contributor

xiota commented Mar 4, 2023

As for the luajit vs Lua 5.4.3, apart from unknown work to port it, is 5.4.3 backward compatible at the Lua level, or will scripts break?

The Lua manuals list "incompatibilities". 5.2, 5.3, 5.4. I don't know how significant they are for GeanyLua, but I'd expect simple scripts would probably keep working, while more complicated ones are more likely to run into problems.

@kugel-
Copy link
Member

kugel- commented Mar 4, 2023

Also @kugel- does Peasy support luajit?

Generally yes, I have found that distributions generally do not include lua support in libpeas packages. That is one reason that I want to build a geany-flatpak so that I can include a lua-enabled libpeas.

@xiota
Copy link
Contributor

xiota commented Mar 4, 2023

Since LuaJIT is maintained, there's a reasonable chance the issue will be fixed sometime around when mainline Linux fully supports Apple M# processors.

We are not talking about linux here but native macOS build which would break because of the pointer authentication issue.

My understanding is that it's an ARM64e processor feature/issue. When Linux gets to the point that M# processors are fully supported, there will be more developers who would be interested in looking at the issue.

IMO much better thing would be to keep using lua and rather try to port the plugin to the latest lua version. I don't think performance is an issue here as I would suspect users use this plugin for some simple extensions rather than CPU-intensive tasks.

LuaJIT can be supported with a build script change. The script can fall back on plain Lua 5.1 when LuaJIT is unavailable. #1233 works with both LuaJIT and Lua 5.1 on Linux. Packagers should have to change only the depends. Testing/adjustments for Windows and Mac would be appreciated.

@techee
Copy link
Member

techee commented Mar 4, 2023

My understanding is that it's an ARM64e processor feature/issue.

Yes, but I have a macbook with this processor already and I provide macOS binaries of Geany for it here https://download.geany.org/geany-1.38_osx_arm64.dmg which, if geanylua is added (I've just managed to get it running), will probably crash when the plugin is enabled (have to try what happens exactly).

LuaJIT can be supported with a build script change. The script can fall back on plain Lua 5.1 when LuaJIT is unavailable. #1233 works with both LuaJIT and Lua 5.1 on Linux. Packagers should have to change only the depends. Testing/adjustments for Windows and Mac would be appreciated.

But again, aren't we just complicating our lives with this unnecessarily? Why having two different dependencies doing basically the same when the current Lua 5.1 works fine?

@xiota
Copy link
Contributor

xiota commented Mar 4, 2023

But again, aren't we just complicating our lives with this unnecessarily? Why having two different dependencies doing basically the same when the current Lua 5.1 works fine?

  • There are people who do not use or work on GeanyLua (eg, fix bugs) because it is using an obsolete branch of Lua.
  • Supporting multiple packages can expose bugs. For instance, #1233 fixes a bug where --with-lua-pkg= was previously completely ignored.
  • LuaJIT has better performance, which elextr has indicated would be beneficial for Geany. (Note: I know at least one GeanyLua user [who isn't me] is using lower-end hardware and has had performance issues with Geany.)
  • kugel- has indicated that the process of dropping support for Lua 5.1 may have already begun: "distributions generally do not include lua support in libpeas packages".
  • Switching this way is less work overall in the long run because I have already found the alternative and done the work to support it. If I hadn't, someone else would eventually have to do it. Making the change now allows package maintainers to switch at their leisure, rather than everyone be forced by external forces to change all at once.

I have a macbook with this processor already and I provide macOS binaries of Geany... if geanylua is added (I've just managed to get it running), will probably crash when the plugin is enabled (have to try what happens exactly).

If GeanyLua crashes on Mac with Lua 5.1, there is no change in plugin availability whether LuaJIT is used or not. If GeanyLua works on Mac with Lua 5.1, but not LuaJIT, it can be built with Lua 5.1 either by autodetection or setting the --with-lua-pkg flag (which currently isn't working, but is fixed by #1233).

Does that ARM64e bug only affect JIT? JIT can be turned off in LuaJIT. (I believe that is the default without some changes to turn JIT on.)

@elextr
Copy link
Member

elextr commented Mar 5, 2023

Why having two different dependencies doing basically the same when the current Lua 5.1 works fine?

Its not two dependencies, its alternate dependencies.

PR #1233 is a build system change only, #1231 is 8 occurrences of a name updated to use the real name (even on Lua5.1) and no longer use the compat name that is Lua5.1 only.

Given the size of the change required, to me there seems to be no reason not to offer a choice of either, a distro/bundle can decide which it wants to offer its users, known but slower and unsupported, or faster and supported (or in the case of Macos if its lua5.1 only due to security issues, thats fine).

The Lua manuals list "incompatibilities". 5.2, 5.3, 5.4. I don't know how significant they are for GeanyLua, but I'd expect simple scripts would probably keep working, while more complicated ones are more likely to run into problems.

Yeah, I didn't understand the importance (or not) of those so I hoped a Geanylua user could comment.

@xiota
Copy link
Contributor

xiota commented Mar 5, 2023

Yeah, I didn't understand the importance (or not) of those so I hoped a Geanylua user could comment.

I don't really know. But the changes in number handling seem significant and potentially able to cause problems in unexpected places. For instance, number handling changes, along with changes in the Geany API, are related to why geany.activate wasn't working.

@hyperair
Copy link
Member

hyperair commented Mar 5, 2023

@elextr

@hyperair how do we make sure packagers are aware that the dependency of a plugin changed?

Just change the dependency of the plugin in build/geanylua.m4, and packagers should figure out that something changed when the next version fails at the configure step with the old build-dependencies I think? Maybe also drop a note in NEWS as well so if packagers go looking they'll find it.

@techee
Copy link
Member

techee commented Mar 5, 2023

@elextr @xiota I think we maybe didn't understand each other - I have no real problem with using LuaJIT, I just think it's more beneficial to migrate to a new Lua version than being stuck at 5.1 - IMO it's just a workaround and not a real fix.

I just created this PR migrating GeanyLua to Lua 5.3 (as mentioned in the PR, I don't have Lua 5.4 in my distro so I didn't try that one): #1235. The changes were mostly cosmetic, nothing big popped up.

@xiota
Copy link
Contributor

xiota commented Mar 5, 2023

@techee I tried migrating to 5.4 shortly before this issue was opened. I got it to compile, but segfaults on load. Maybe gave up too quickly. I did not know about LuaJIT at the time. Stepwise migration would have been easier, but the distro I was using at the time made that difficult.

Problems with 5.3 and 5.4:

  • 5.3 is yet another deprecated branch.
  • 5.3 is not very popular, by package count.
  • 5.4 is not in your distro. Would you be looking at supporting building with both 5.3 and 5.4, along with language changes causing scripts to become incompatible depending on which distro has which version of Lua available?
  • Potential incompatibility with existing scripts (especially number handling).
  • GeanyLua will be unable to use LuaJIT in the future. Will be stuck with Lua and it's incompatible releases.

Benefits of LuaJIT over 5.4 are:

  • Better performance.
  • Continued compatibility with existing Lua scripts.
  • FFI to call C functions from Lua. (But scripts using it will not be compatible with Lua 5.1.)
  • Almost as popular as 5.4, by package count.
  • Can fallback on Lua 5.1 if needed. Can change course in the future for 5.4/5.5 if desired.

Problems with LuaJIT

  • Potential issues with ARM64e Macs. But can disable JIT or fallback on Lua 5.1.
  • Edge case incompatibilities with Lua 5.1. (From forum posts, looks like mainly recursion issues.)

@techee
Copy link
Member

techee commented Mar 5, 2023

I just tried and on my other linux VM geanylua with the PR runs with Lua 5.4 without any modifications needed.

I'm not really convinced by the performance argument - I just don't see how anyone would notice. The whole Geany except Scintilla lexers and ctags parsers could be written in a scripting language and I don't think anyone would notice any performance difference.

(And, and this is probably just a personal preference, by adding JIT you make from a nice little interpreter a beast which introduces its own set of problems like CPU/OS/architecture dependence. I'm not sure how LuaJIT's JIT works but since most of the scripts will be short-running, it could make the execution of the scripts slower if it performs JIT compilation every time and doesn't store the precompiled binary somewhere. IMO, the only language where JIT makes sense is javascript in web browsers, but anywhere else where you need the performance, it's best to go with the native code.)

Regarding compatibility, yes, that could be an argument. But I suspect users of this plugin will use it for smaller utilities related to their workflow and they won't use any advanced features from the language where they might see differences.

Anyway, my personal preference would be using pure Lua but I'm fine if others have a different opinion and the LuaJIT patch gets merged.

@xiota
Copy link
Contributor

xiota commented Mar 6, 2023

I just tried and on my other linux VM geanylua with the PR runs with Lua 5.4 without any modifications needed.

I saw that note. (I'll try your branch in a little while.) The problem is the Lua language changes between versions. And there may be subtle API differences that work fine on the C side, but result in different Lua execution results. So if different platforms are using different versions of Lua, say 5.3 and 5.4, the scripts won't necessarily be compatible. Also, existing Lua 5.1 scripts may break.

I'm not really convinced by the performance argument - I just don't see how anyone would notice. The whole Geany except Scintilla lexers and ctags parsers could be written in a scripting language and I don't think anyone would notice any performance difference.

Whether someone notices depends on how performant their hardware is. I've seen Geany touted as being suitable for use on low-end hardware. At least a few such users would notice.

... by adding JIT you make from a nice little interpreter a beast which introduces its own set of problems like CPU/OS/architecture dependence. I'm not sure how LuaJIT's JIT works but since most of the scripts will be short-running, it could make the execution of the scripts slower if it performs JIT compilation every time and doesn't store the precompiled binary somewhere.

LuaJIT can run without JIT (as I understand, this is the default). According to benchmarks others have run, the LuaJIT interpreter, without JIT, runs faster than the Lua interpreter.

I suspect users of this plugin will use it for smaller utilities related to their workflow and they won't use any advanced features from the language where they might see differences.

Based on code snippets posted online, changes in Lua behavior can be surprising. Even if something breaking is unlikely, it would be highly disruptive to the workflows of users who do experience problems. If not using advanced features, why switch to Lua 5.4 rather than add LuaJIT build support?

Anyway, my personal preference would be using pure Lua but I'm fine if others have a different opinion and the LuaJIT patch gets merged.

Adding LuaJIT build support now doesn't in and of itself block work toward supporting another Lua release in the future.

@elextr
Copy link
Member

elextr commented Mar 6, 2023

@techee

But I suspect users of this plugin will use it for smaller utilities related to their workflow and they won't use any advanced features from the language where they might see differences.

(@elextr chuckles to self) Of course that argument could be run backwards, if scripts won't use the newer features there is no Lua reason to upgrade 😁

Your comments on possible performance issues are valid, compiling first every time a script is run may be slower than interpreting for short scripts, but IIUC there is a compile cache so a function only gets compiled once. What the effect is can only be determined by benchmarking lots of Geanylua scripts, but we don't have those, and no Geanylua users have commented on this issue's issues.

Maybe there are no intensive users, only occasional users like @xiota so nobody will care about such details. For example it seems (#1236) that grabkey() has not been working for an unknown period of time, but nobody has complained? Dunno.

@xiota

Adding LuaJIT build support now doesn't in and of itself block work toward supporting another Lua release in the future.

True, and #1235 suggests the changes are simple enough that both 5.1 and 5.4 C APIs could be supported within the #ifdef pain threshold. As a wise man once said, if offered a questionable choice, take all three (at least until a winner is clear).

@xiota
Copy link
Contributor

xiota commented Mar 6, 2023

... performance issues...

I will ask someone who is having some performance issues while using some GeanyLua scripts whether he's willing to try LuaJIT to see if there's any noticeable difference. But I think the problem is that the computer is just slow.

True, and #1235 suggests the changes are simple enough that both 5.1 and 5.4 C APIs could be supported within the #ifdef pain threshold. As a wise man once said, if offered a questionable choice, take all three (at least until a winner is clear).

I'd request that adding support for 5.4, be deferred until late in 1.39 development or even a future release. This would make backporting bug fixes prior to the transition easier (eg, if a 1.38.1 bugfix branch were created). For instance, fixes for geany.activate (#1234) and geany.keygrab (#1236) can be applied to 1.38 with unmodified git-generated patches. #1235 itself would require more work to backport and would complicate backporting any subsequently written bugfix.
Nevermind. Figured it out.

(Is there a git command to extract only commits/patches that apply to one plugin/folder?)

git format-patch 1.38.0 -o patches -- build/geanylua.m4 geanylua/*.{c,h,am}

Then exclude 5e13096 and 01b19d2.

@techee
Copy link
Member

techee commented Mar 6, 2023

True, and #1235 suggests the changes are simple enough that both 5.1 and 5.4 C APIs could be supported within the #ifdef pain threshold. As a wise man once said, if offered a questionable choice, take all three (at least until a winner is clear).

Hmm, IMO we should support the smallest set of Lua versions, otherwise it might get hard to maintain the plugin. My preference regarding what to support would be (in this order):

  1. Lua 5.1, or Lua 5.2, or Lua 5.3, or Lua 5.4
  2. Lua 5.1 + LuaJIT
  3. Some wild combination of the above

@xiota xiota linked a pull request Mar 6, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants