graph-toolkit icon indicating copy to clipboard operation
graph-toolkit copied to clipboard

Lua 5.2 support

Open daurnimator opened this issue 10 years ago • 17 comments

Continuing from https://github.com/franko/graph-toolkit/issues/3#issuecomment-68569447

the current version of the library won't work in Lua > 5.1; for one, it uses C API functions that were removed in 5.2 (e.g. lua_getfenv/lua_setfenv).

Your uses should be simple enough to replace with lua_getuservalue.

FWIW, I write my C libraries in 5.2 style, and get them to work in 5.1+luajit with compat-5.2, it works out quite well :)

daurnimator avatar Jan 03 '15 20:01 daurnimator

skip lua 5.2 support and make it work with upcoming lua 5.3

markuman avatar Jan 04 '15 16:01 markuman

I would like to have a volunteer to port the library to Lua 5.2 and the coming Lua 5.3.

I'm personally not strongly motivated since I have the feeling that the changes in Lua 5.2 force me to adapt a lot of working code for virtually no gain. In addition I have other projects in this moment and is difficult for me to spare some more times for this task.

On the other side I guess that the support for Lua 5.2 is useful because some people are asking for it :-) and I would like to be neutral and provides all the versions so that the module is usable for a wider audience of Lua users. That's why a volunteer would be the idea solution for me. May be I'm going to ask in the Lua mailing list.

franko avatar Jan 05 '15 21:01 franko

So I've started in my fork in branch lua5.3 https://github.com/markuman/graph-toolkit/commits/lua5.3

The last thing what's missing is to port luaL_register, where I actually don't know how to made it correct.

-    luaL_register (L, NULL, plot_metatable);
+    luaL_setfuncs (L, plot_metatable, NULL);

And it's not flexible yet (static lua 5.3 only)

markuman avatar Oct 18 '15 18:10 markuman

The last thing what's missing is to port luaL_register, where I actually don't know how to made it correct.

  • luaL_register (L, NULL, plot_metatable);
  • luaL_setfuncs (L, plot_metatable, NULL);

You should pass 0 to luaL_setfuncs, not NULL. Otherwise that is equivalent (as long as 2nd arg to luaL_register was NULL) :)

daurnimator avatar Oct 18 '15 18:10 daurnimator

This ends up in an invalid conversion

lua-draw.cpp:332:40: error: invalid conversion from 'const luaL_Reg*' to 'int' [-fpermissive]
     luaL_setfuncs (L, 0, draw_functions);

Because it's void luaL_setfuncs (lua_State *L, const luaL_Reg *l, int nup); I came up to switch the inputs to luaL_setfuncs (L, draw_functions, 0); It's in lua-draw.cpp

So if this is right, only this part is left which I can't handle https://github.com/markuman/graph-toolkit/blob/lua5.3/src/lua-graph.cpp#L58 How to handle the string "graphcore" in luaL_setfuncs?

markuman avatar Oct 18 '15 18:10 markuman

How to handle the string "graphcore" in luaL_setfuncs?

Remove it (use NULL instead) => libraries should not set globals anyway.

(the 2nd arg is what global variable to make the library available under)

daurnimator avatar Oct 18 '15 19:10 daurnimator

Any progress here?

daurnimator avatar Dec 22 '16 11:12 daurnimator

From my side, unfortunately, not.

markuman avatar Dec 22 '16 13:12 markuman

Ping:

  • @markuman Do you have your progress on this posted anywhere? The link from your GH fork seems to be dead.
  • @franko Are you around to move this forward if somebody were to get a working PR together?

alerque avatar Jan 31 '24 14:01 alerque

This is an old thread but of course a Lua upgrade would be a good thing. However I am wondering if we should upgrade directly to Lua 5.4. I would like to have some informed advices about the goto version we should adopt with some practical arguments.

Otherwise I may just go for Lua 5.4 for the upgrade.

franko avatar Jan 31 '24 18:01 franko

Of course 5.4 is the only logical target at this point ... while retaining LuaJIT support of course. Once you do that you have all the ones in between anyway. Mention was already made of compat-5-3 which makes it pretty easy to make something buildable against the whole range of available Lua engines.

alerque avatar Jan 31 '24 18:01 alerque

Good, thank you for the feedback, I will try to port to Lua 5.4 while maintaining compatibility with LuaJIT.

franko avatar Feb 01 '24 08:02 franko

Hi there,

I added support for Lua 5.4 while maintaining the compatibility. Please test the branch:

https://github.com/franko/graph-toolkit/tree/lua-5.4-compat

To compile use the command: "meson setup -Dlua=lua5.4 build". The possible options are lua5.1, lua5.4 and luajit.

I removed the makefile based build system to use Meson, much better now. I have also updated the C++ code to avoid warning on deprecated stuff. The library libagg is now a subproject so it will be automagically built, user just need to install freetype2 and lua dev packages.

My tests on Linux are ok for the moment but I tested only with Lua 5.4.

franko avatar Feb 04 '24 23:02 franko

Thanks for looking into this. I'm sure we'll get some millage out of it somehow.

I understand Meson is a good build system and probably beats custom Makefiles for a lot of scenarios, but I think you might have overlooked one major factor here. One of the main ways of using Lua library projects as dependencies is via through LuaRocks, and out of the box as far as I know LuaRocks does not mave a Meson backend. It does make a make backend which is what your own rockspec is configured to use. The rockspec is now broken/non-functional. I believe it would be adapted using the custom commands backend or adding a meson backend module yourself. That will work with an additional downside for downstream use: you've now introduced a new build time dependency that isn't resolvable with the Lua ecosystem package manager and hence made things more complicated for downstream projects.

alerque avatar Feb 05 '24 10:02 alerque

If it is required I can restore the Makefile, I didn't know it was a requirement for the luarock but it can be done.

franko avatar Feb 05 '24 13:02 franko

I don't know about required, but unless you want to write a LuaRocks backend for meson (e.g. here is a backend for cargo/mlua) then I think adapting the changes to a Makefile based build again so that the rockspec works is going to be the best way forward for this to be usable in the Lua ecosystem. I would keep the Meson support since you already wrote it and it might be useful to some projects, but being usable via LuaRocks is far and away the most useful distribution channel for Lua stuff.

alerque avatar Feb 07 '24 07:02 alerque

I brought back the Makefile based build system simplified and revised. Finally the makefile is pretty fine I think and it should work as before.

Now it would be nice if someone can test the building and the rockspec itself.

I am also going to merge this branch into the master. It would be nice if someone can post in the Lua mailing list about this update of the project. They may test and give us some useful feedbacks.

franko avatar Feb 08 '24 09:02 franko