lgi-devs / lgi

Dynamic Lua binding to GObject libraries using GObject-Introspection
MIT License
428 stars 69 forks source link

Makefile doesn't work on systems with multiple lua versions? #314

Closed ghost closed 1 year ago

ghost commented 1 year ago

On my system, sudo env PREFIX=/usr LUA_VERSION=5.1 make -e install leads to a v5.1 module that fails on require with the C-side loader looking for missing symbols like lua_getiuservalue (a v5.4-only function). This is because the preprocessor is fetching the header for whatever is the default lua version installed, which does not necessarily match the one chosen for building LGI.

Should i be setting some "lua header version" environment variable for the makefile?

it works if i add a line on lgi/Makefile with:

CFLAGS += $(shell $(PKG_CONFIG) --cflags lua$(LUA_VERSION))

I haven't tried it yet, but changing the PKGS line to PKGS = $(GINAME) gmodule-2.0 libffi lua$(LUA_VERSION) should work, too.

psychon commented 1 year ago

lua$(LUA_VERSION))

This doesn't work everywhere either.

The fundamental problem is that Lua does not come with pkg-config files. These are added downstream by Linux distros instead. And AFAIR some of them just use lua and others use e.g. lua5.2.

Locally, I have the following laying around to make sure the correct lua version is used (notice that I also have to "mess with" LUA_LIB):

diff --git a/lgi/Makefile b/lgi/Makefile
index b7bce01..d31b00e 100644
--- a/lgi/Makefile
+++ b/lgi/Makefile
@@ -12,11 +12,11 @@ LUA_LIBDIR = $(PREFIX)/lib/lua/$(LUA_VERSION)
 LUA_SHAREDIR = $(PREFIX)/share/lua/$(LUA_VERSION)

 PKG_CONFIG = pkg-config
-GINAME = gobject-introspection-1.0
+GINAME = gobject-introspection-1.0 lua5.3
 PKGS = $(GINAME) gmodule-2.0 libffi
 VERSION_FILE = version.lua

-LUA_LIB = -llua
+LUA_LIB = -llua5.3

 ifneq ($(filter cygwin% msys% mingw%, $(HOST_OS)),)
 CORE = corelgilua51.dll
diff --git a/tests/Makefile b/tests/Makefile
index de74c8f..fa0703e 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -25,9 +25,9 @@ CCSHARED = -fPIC
 endif
 endif

-PKGS = gio-2.0 cairo cairo-gobject gobject-introspection-1.0 gmodule-2.0 libffi
-LUA = lua
-LUA_LIB = -llua
+PKGS = gio-2.0 cairo cairo-gobject gobject-introspection-1.0 gmodule-2.0 libffi lua5.3
+LUA = lua5.3
+LUA_LIB = -llua5.3
 PKG_CONFIG = pkg-config

 ifndef CFLAGS

So... dunno what to do about this.

The current default (without special variables) is to produce something that works with lua, which in your case seems to be Lua 5.4 (Debian, I guess).

ghost commented 1 year ago

It's not that big of a deal, anyways. After carefully reading the makefile and where exactly things were failing, i came up with a wrapper that fits my system. I'll keep it around on a local folder along with a 'uninstall' script and other repetitive things. I just think a little heads up will avoid trouble for newcomers.

psychon commented 1 year ago

Mostly fine with me.

The lua headers define LUA_MAJOR_VERSION and LUA_MINOR_VERSION. If possible, parse these values on the makefile and compare with the provided LUA_VERSION build variable for a sanity check.

Uhm... no, they do not?

greping for VERSION, I only find LUA_VERSION and LUA_VERSION_NUM. There is LUA_VERSION_MAJOR and LUA_VERSION_MINOR, but lua5.1 and luajit do not define it.

So... uhm... Urgh. That sounds complicated...

My first idea only works properly with Lua 5.1 and luajit:

$ for folder in /usr/include/lua* ; do echo $folder ; (echo '#include <lua.h>' ; echo 'LUA_VERSION') | gcc -I $folder -E - | tail -n 1 ; echo ; done
/usr/include/lua5.1
"Lua 5.1"

/usr/include/lua5.2
"Lua " "5" "." "2"

/usr/include/lua5.3
"Lua " "5" "." "3"

/usr/include/lua5.4
"Lua " "5" "." "4"

/usr/include/luajit-2.1
"Lua 5.1"

So... remove all spaces and quotes?

$ for folder in /usr/include/lua* ; do echo $folder ; (echo '#include <lua.h>' ; echo 'LUA_VERSION') | gcc -I $folder -E - | tail -n 1 | sed -e 's/"//g;s/ //g' ; echo ; done                                                                                                                                                                               
/usr/include/lua5.1
Lua5.1

/usr/include/lua5.2
Lua5.2

/usr/include/lua5.3
Lua5.3

/usr/include/lua5.4
Lua5.4

/usr/include/luajit-2.1
Lua5.1

Dunno what to do with luajit here. It could be detected by checking e.g. whether LUAJIT_VERSION is defined...

Make a table with lua path conventions followed by each system and choose by uname

I don't think uname is the right option here and there will always be more systems that we do not know about / do not handle correctly. Looking at the Makefile again, it already seems to be detecting Windows and Mac OS by uname.

ghost commented 1 year ago

We can just warn on the readme that the makefile defaults to building with the system's default version of lua (and that LUA_VERSION must me manually set if their version is ~= "5.1"). If no one else chimes in till next week i'll close this issue. On another related topic, it might be helpful in the future to keep a "known makefile overrides" table (the spreadsheet kind, not the code kind) that demonstrates some tweaks and variable overrides that users with exotic configurations have done to succesfully build.

ghost commented 1 year ago

Conclusion: makefiles won't be changed for now. Contributors with issues building for odd multi-version setups should modify or wrap around the generic makefile to suit their environment... or use luarocks, or build with meson.

note to readers: If you are interested in making the makefile reliably cover a wider set of configurations (including multi-version setups accross all of Linux,BSD,Windows,Mac,RTOS variants), do submit your changes. We are open to a better solution, but just haven't figured out what it is yet.