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

Lua 5.2 support #4

Open
daurnimator opened this issue Jan 3, 2015 · 17 comments
Open

Lua 5.2 support #4

daurnimator opened this issue Jan 3, 2015 · 17 comments

Comments

@daurnimator
Copy link

Continuing from #3 (comment)

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 :)

@markuman
Copy link
Contributor

markuman commented Jan 4, 2015

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

@franko
Copy link
Owner

franko commented Jan 5, 2015

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.

@markuman
Copy link
Contributor

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)

@daurnimator
Copy link
Author

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) :)

@markuman
Copy link
Contributor

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?

@daurnimator
Copy link
Author

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
Copy link
Author

Any progress here?

@markuman
Copy link
Contributor

From my side, unfortunately, not.

@alerque
Copy link

alerque commented Jan 31, 2024

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?

@franko
Copy link
Owner

franko commented Jan 31, 2024

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.

@alerque
Copy link

alerque commented Jan 31, 2024

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.

@franko
Copy link
Owner

franko commented Feb 1, 2024

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

@franko
Copy link
Owner

franko commented Feb 4, 2024

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.

@alerque
Copy link

alerque commented Feb 5, 2024

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.

@franko
Copy link
Owner

franko commented Feb 5, 2024

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.

@alerque
Copy link

alerque commented Feb 7, 2024

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.

@franko
Copy link
Owner

franko commented Feb 8, 2024

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.

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

No branches or pull requests

4 participants