Page MenuHomePhabricator

fix compilation with Lua 5.2+
Closed, ResolvedPublic

Description

When building against current lua:

lib/evas/filters/evas_filter_parser.c: In function '_lua_backtrace':
lib/evas/filters/evas_filter_parser.c:2434:20: error: 'LUA_GLOBALSINDEX' undeclared (first use in this function)
    lua_getfield(L, LUA_GLOBALSINDEX, "debug");

That happens because from lua > 5.1, the variable LUA_GLOBALSINDEX doesn't exist anymore.

The fix is straightforward, replace the lua_getfield call with:

lua_getglobal(L, "debug");

If you want to conditionalize it on the version of Lua, you can, but Fedora doesn't have old lua anymore, so I didn't do that in my patch.

spotrh created this task.Sep 14 2015, 11:33 AM
spotrh updated the task description. (Show Details)
spotrh raised the priority of this task from to Incoming Queue.
spotrh added a project: efl.
spotrh added a subscriber: spotrh.
q66 claimed this task.Sep 17 2015, 3:07 AM

there is no need to conditionalize, lua_getglobal is a standard call. i'll fix it

q66 added a comment.Sep 17 2015, 3:10 AM

hm, actually seems like this is already fixed. 6124d0733657e425001ce51f526aea3bb8dc54e7

stefan_schmidt closed this task as Resolved.Oct 12 2015, 2:33 AM

Closing as this should be fixed. If you still see this problem with latest efl please re-open.

kubu reopened this task as Open.Jan 23 2016, 2:34 PM
kubu added a subscriber: kubu.
kubu added a comment.Jan 23 2016, 3:01 PM

Hello,

I'm investigating a build issue with efl 1.15.3 found by Buildroot autobuilders [1] and I found this patch
fixing the build issue with Lua 5.2+.

[1} http://autobuild.buildroot.net/results/e178371c2c3bf42d59c6fc26409e098081239ccb/

So, I bumped efl to 1.16.1 and compiled it with Lua 5.3 (I also tried with 5.2) but I have the
following link issue when --enable-lua-old is used:

CCLD bin/ecore_evas/ecore_evas_convert
host-efl-1.16.1/src/lib/evas/.libs/libevas.so: undefined reference to `luaL_openlib'
collect2: error: ld returned 1 exit status
Makefile:19021: recipe for target 'bin/ecore_evas/ecore_evas_convert' failed

I reopened this bug since I think it's related to the fixes applied.
As far I can see, it's caused by compatibility defines in eva_filter_parser.c:

#define LUA_COMPAT_ALL
// For 5.3 --> 5.1 compatibility
#define LUA_COMPAT_5_1

Theses define enable some compatibility code in Lua witch define luaL_register() :
From Lua 5.3.2 source: src/lauxlib.h

/* compatibility with old module system */
#if defined(LUA_COMPAT_MODULE)

LUALIB_API void (luaL_pushmodule) (lua_State *L, const char *modname, int sizehint);
LUALIB_API void (luaL_openlib) (lua_State *L, const char *libname, const luaL_Reg *l, int nup);

#define luaL_register(L,n,l) (luaL_openlib(L,(n),(l),0))

#endif

Note: The fixes break the build with Lua 5.1.

For now we don't have LuaJIT support enabled for efl package in Buildroot,
is it "recommended" to use the --enable-lua-old option ? ;-)

Help is welcome.

Best regards,
Romain

kubu changed the visibility from "All Users" to "Public (No Login Required)".Feb 12 2016, 12:37 PM
raster added a subscriber: raster.Jul 13 2016, 9:39 PM

if luajit is available for your platform its pretty much required to use it. we rely on luajit's ffi for lua bindings for example.

q66 - what should be do? just drop old lua and do luajit or nothing?

q66 added a comment.Jul 19 2016, 2:27 AM

We basically have two options. Either maintain support with all Lua versions - which is theoretically possible but then we can't guarantee that all scripts will keep working on all EFL installations (because of subtle language/library differences) and we'll have to drop it later anyway, or just drop everything but luajit... the choice is yours

i would lean to only supporting luajit. its required for ffi and bindings anyway.

How is the status for luajit for other systems like Windows or macOS? Are we happy with the number of architectures luajit does support?

I remember that I had to use lua-old for some build setups when we introduced the luajit based one. http://luajit.org/luajit.html seems to list all things we really care about. Just checking here before we remove the old lua support and get into trouble here.

q66 added a comment.Jul 19 2016, 7:02 AM

windows and mac os support is fine. I tested it myself since I had luajit integrated in my game engine and didn't experience any issues.

As far as architectures go, these days it should be at least x86, x86_64, arm, arm64, ppc and mips, with ppc64 and mips64 support underway. I think that should be enough for us. It doesn't support x32 (AFAIK), which AFAIK we had build slaves for, but I'm not sure if we need that at all, since almost nobody uses it anyway.

From my side that looks like it covers all we care about right now. We dropped the x32 support on Jenkins at some point as nobody really picked it up.

q66 added a comment.Jul 19 2016, 7:38 AM

I think we're good in that case. Do we want to drop it for 1.18 or leave it for next release?

I#m a bit worried what problems will show up if we rip it out now. If there is nothing urgent I would say it should wait for 1.19. We can do it early on after the merge window opens.

q66 triaged this task as TODO priority.Jul 19 2016, 7:54 AM

Sounds good, let's do it when merge window opens.

kubu added a comment.Jul 19 2016, 11:28 AM

Hi all,

Thanks for your comments!
Since my last post, I updated the efl package in Buildroot to use only Luajit [1] and nobody complained about this change.
It's ok for me to drop lua-old for the next release.

Best regards,
Romain

[1] https://git.busybox.net/buildroot/commit/?id=92f7591eca0d2b4ff827ed90629be94292c8b102

aaah cool. because the advice for you would be to go back to 5.1 for now. :) but ultimately luajit is where we'll move to and drop normal lua support as it makes life easier for us AND ensures lua bindings will work.

aerodynamik raised the priority of this task from TODO to Incoming Queue.Aug 2 2016, 3:15 PM

Hi,

Last time I checked, roughly 1 month ago, the current code compiled fine on ppc64 with Lua 5.2. It also seems that (beside myself) there are some interest in the debian community to support ppc64, ppc64le and x32; adding a strong dependency on Luajit will make those effort harder to come to fruition. Also looking at the future, Lua.org implementation, beeing plain ansi-c, should always be easier to get running on a new target.

With those consideration in mind, would you not think that it might be desirable to keep the compatiblity with Lua ?
Maybe keeping the compatibility with only one Lua version would be enough to reduce the maintenance burden while keeping the portability as it is now ?
Last question, is the FFI extension already in use by some E project or is there any plan on doing so ?

Thanks!

q66 added a comment.Aug 2 2016, 3:23 PM

elua (which will become increasingly more used, once the efl lua bindings are fully functional) doesn't work without luajit at all (even now), bob (when it's written) will not work without luajit, etc.

there isn't any point in extending support for normal lua since it will get removed sooner or later anyway

ppc64 support for luajit already exists and will most likely get mainlined at some point, x32 is pretty useless

i'm kind of doubtful about interest from debian since they still appear to be stuck at efl 1.8 and have always been slow with any update

Thanks for your answers. Indeed I forgot that I had to disable Elua on ppc64. Do you think that using https://github.com/facebook/luaffifb would be enough to let Elua run without Luajit ? If not, what would still be missing ? If so, would you be wiling to accept a contribution as such ?

aerodynamik added a comment.EditedAug 2 2016, 4:45 PM

Well that might be a dumb proposition since the supported platform list of luaffifb is much shorter than the one of luajit (not saying it is also based on LJ's dynAsm so that's hardly going to improve) ... :-/

raster added a comment.Aug 2 2016, 5:08 PM

hahahaha. yeah. @q66 is right. we'll rely on ffi far far more. our lua bindings already are "ffi or go away" as frankly its drastically faster for a binding to do it that way.

i think the best thing to do would be to make a pure C bytecode interpreter for luajit with an interface to libffi. this may not be quite as fast as the luajit hand-done assembly bytecode engine per arch, but it'd be portable and be a "drop in replacement" until such a time as a faster mainline hand written one exists. that is luajit's weakness. the lack of portability even if speed suffers, but these days it supports all the MAJOR architectures either with a full tracing jit, or at least a bytecode interpreter.

really the only issue i see MAY be x32, but frankly it just hasn't caught on and if anything interest is declining. a few years back there was much talk of it, but it's basically faded and not because everyone is on board now. it's a whole new arch to support in addition to x86-64 and i386 and distributions and developers are pretty loath to add yet another arch to builds, repositories, development support etc. - yes i know it has gains on memory usage and speed on 64bit capable x86. but it just doesn't look "real" given the fading interest.

or more specifically - do we make major decisions based on speculation of a possible change in interest in an architecture in the future? if it truly does become interested in, then someone will do an x32 port of luajit. :)

this is one of those things. sacrifices have to be made sometimes. :(

q66 added a comment.Aug 2 2016, 5:28 PM

@aerodynamik I looked at the lua ffi a while ago, it wasn't really very usable and it had some semantics differences compared to luajit ffi

@raster i don't think luajit's arch support is a problem any more, since it supports pretty much everything already... I guess writing a portable bytecode interpreter wouldn't be impossible though...

raster added a comment.Aug 2 2016, 7:55 PM

well it doesn't support x32... someone will pipe up and say it doesnt support 68k, sh4, parisc, risc-v... my point is that a simple not-so-fast yet portable c bytecode interpreter + libffi to do the ffi calls should solve the "ok now works on everything" even if its not the best performance. i'm kind of baffled why the guy didn't do this to begin with... :)

raster added a comment.Aug 7 2016, 7:08 PM

so... what shall we do here... ? drop anything non-luajit after 1.18 is out?

kubu added a comment.Aug 9 2016, 4:38 AM

Hi,

I don't know much about libffi/luajit-ffi stuff but if you fix and keep lua-old support even with only the latest Lua 5.3, then efl could be build by Buildroot autobuilders on m68k, sh4, microblaze, nios2, sparc, sparc64, xtensa, aarch64 on musl, uClibc-ng and glibc based system.

I don't think efl can be really used by all of them, at least I tested efl 1.15 on ARM with uClibc-ng system but without luajit.
Someone reported to use efl 1.15 on ARM with musl system.

I have no strong opinion about non-luajit support... I'm fine if you remove it after 1.18.

Best regards,
Romain

q66 added a comment.Aug 9 2016, 4:45 AM

That's nice to know, but it's sort of useless anyway if we can't properly test on those targets, as with something the size of EFL things are bound to be broken in various places on untested setups...

with luajit it should still build on all architectures that actually matter, and uclibc-ng/musl should still not be a problem (i don't see any problem with building luajit on those, it should be simple)

And then there is still the problem with scripts - as I said, subtle language differences are bound to break scripts on other versions than the script was written for, and that's very bad; I believe supporting one version is the reasonable course of action.

kubu added a comment.Aug 9 2016, 6:33 AM

I guess, Buildroot users which use those uncommon architectures should be aware of that they can found some issues on untested setups. But the Buildroot policies is to follow upstream decisions even if it restrict to a limited number of architectures.

For efl with uClibc-ng, I upstreamed a patch for adding mkostemps() function but that all [1] [2]. No other build issue related to musl/uClibc-ng toolchains has been reported by autobuilders. We are currently using efl 1.17.2 with Luajit only.

The problem about Lua version and Lua scripts is a know problem, that is why there is still the support for Lua 5.1, 5.2 and 5.3 in Buildroot. If the Lua scripts used by efl stack are know to work only with a specific Lua version, then we can add a dependency on this version.

Best regards,
Romain

[1] http://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/commit/?id=6cf35f84045f38f067365623886fecff16ca92f9
[2] https://git.busybox.net/buildroot/commit/?id=5b47168f16b4e9d9a05a866bcb1b448d9cc382c3

q66 added a comment.Aug 9 2016, 6:47 AM

Well, my point was more about external scripts (say, ones part of potential third party Enlightenment themes). If EFL supports all four versions (luajit, 5.1, 5.2, 5.3) and a script that is part of an Enlightenment theme gets written against 5.2 or for example using a LuaJIT language extension, it will not work with EFL builds built against a different version of Lua (therefore preventing usage of that theme in that build of Enlightenment).

There is no real way to solve this as far as I'm aware. But if we get rid of other versions than LuaJIT, all scripts will be written against LuaJIT (even third party ones) and things will be guaranteed to work.

raster added a comment.Aug 9 2016, 6:21 PM

just saying...

m68k, sh4, microblaze, nios2, sparc, sparc64, xtensa

no developers who work on e or efl have that hardware. we can't test or verify etc. ... and much of it legacy and shrinking more and more down to irrelevance. i get it. i used to be an m68k guy. i went 6502->68k->i586->x86064 and these days i'm very partial to arm. i get your point "you could support all these niche architectures that 3 people now use!" :) (ok a bit of tongue in cheek :)).

we have to sacrifice our codebase, complexity, and continual work keeping up with lua, having to deal with luajit AND lua "normal" where "normal" lua REFUSE to maintain abi compatibility between versions". that is their policy. the policy is "you should be shipping a COPY of lua WITH your project and not using external libs". that is lua upstream policy for development and why they break abi.

so should we accept a never-ending stream of pain here... or should we just say "stuff it" to these architectures and align with luajit now that it covers all the major things we need (it also does aarch64 too - i am not sure musl vs glibc makes a difference as the abi calling conventions are the same - EFL), and cut the pain and ongoing maintenance?

if someone REALLY cares they could write a C "generic" luajit engine with ffi and thus work everywhere. if someone does care enough, they'll do it.

and either way our whole plans for our lua bindings are luajit or nothing. normal lua is not viable (as you've found out). we can't normalize such bindings if we have to maybe work with luajit and then also traditional lua etc. they may sometimes work or not based on configure options.

and perhaps you don't know our plans for "codename bob".. which is meant to replace edje AND elementary in the end (or almost all of it - elm's basic widget infra, window abstractions, dnd/cnp etc. will likely stay). Bob's whole plan is to rely on eolian + eo bindings and thus luajit and ffi. this CAN'T be optional then. it's not possible. we can't build our entire future UI layer on something that may not be there and thus not function.

as your point about the edje lua scripts - they should stick to 5.1 features/syntax plus anything luajit may add. this is why locking down to luajit only is a good plan. you can't escape this specific featureset then. :)

Just my two cents, isn't it possible for lualian to generate C bindings -- at compilte time thus -- in favor to generate ffi stubs which are interpreted/jitted at run-time ? Might be the obvious solution or am I yet missing something ?

q66 added a comment.Aug 11 2016, 6:06 AM

it is, but LuaJIT can actually JIT FFI calls and as a result generate the same code as a function pointer call would in C, while C calls are interpreted/slow (both because of the call itself, and argument/result marshalling)

that still doesn't fix the issue where lua 5.2, .53 and so on change the script language so that means a single script may not work across multiple efl installs because the lua version used may be different.

so regardless, our "definition" of lua is luajit's definition which is an extended lua 5.1. not 5.2 or 5.3. technically building with a lua other than luajit is actually wrong as it how has a lua script language that does not match what we "define" to be the language supported. :)

kubu added a comment.Aug 11 2016, 2:43 PM

Thanks for this detailed answer!

Indeed "normal" Lua seems pretty much "unstable" to be used/maintained on the long term...

With these clarifications "normal" Lua would quickly becomes an obstacle to further developments...
So let's drop it if other core developers are agree with that ;-)

About the aarch64 luajit support, this architecture is not supported by the current stable version Luajit 2.0.4.
aarch64 will be only available with the upcoming Luajit 2.1.

Best regards

aaaah ok. but aarch64 is coming soon enough. that's good enough for me. luajit 2.1. :) i would hope that's not too far away.

stefan_schmidt triaged this task as Normal priority.Feb 10 2017, 6:54 AM
vcunat added a subscriber: vcunat.May 2 2017, 4:50 AM

Now luajit-2.1.0-beta3 breaks compilation of EFL, but that's easily fixable by providing the removed alias again, e.g. via -DluaL_reg=luaL_Reg compile flag.

Anyone seen this?

ERR<16015>:elua /home/glchaves/devel/enlightenment/efl/src/lib/elua/elua.c:714 _elua_errmsg() /home/glchaves/devel/enlightenment/build/efl/src/bin/elua/.libs/lt-elua: /home/glchaves/devel/enlightenment/efl/src/scripts/elua/core/module.lua: cannot load incompatible bytecode
ERR<16038>:elua /home/glchaves/devel/enlightenment/efl/src/lib/elua/elua.c:714 _elua_errmsg() /home/glchaves/devel/enlightenment/build/efl/src/bin/elua/.libs/lt-elua: /home/glchaves/devel/enlightenment/efl/src/scripts/elua/core/module.lua: cannot load incompatible bytecode

$ lua -v
Lua 5.3.4 Copyright (C) 1994-2017 Lua.org, PUC-Rio

$ luajit -v
LuaJIT 2.1.0-beta3 -- Copyright (C) 2005-2017 Mike Pall. http://luajit.org/

make[4]: *** [Makefile:54431: lib/ecore_audio/ecore_audio.eo.lua] Error 1

to be more complete on the point of failure.

no - but i build with luajit. our bindings totally depend on luajit, so normal lua support is really kind of on "life support" as a "until luajit supports your architecture". it supports all the major ones now. are you building with luajit (default) or lua-old?

--disable-doc --enable-systemd --enable-elput

here as config.

s you're using luajit then? ehhhh errr..... WAT? LuaJIT 2.0.5 here on arch... that error message will then be coming from luajit itself... maybe it cached some bytecoee somewhere from 2.0.x days and now you upgraded it's not happy? i have never head of it doing this kind of thing before... but i'm still on 2.0.5

By experience, 2.1.x bytecode is incompatible with 2.0.x and vice versa. (Interestingly, bytecode should be compatible when changing hardware architecture.)

limachaves reopened this task as Open.Aug 18 2017, 8:15 AM

Reopening this, then.

q66 added a comment.Aug 18 2017, 8:34 AM

looks like leftover cached bytecode made with previous version of luajit...

Removing src/scripts/elua/apps/lualian.luac
Removing src/scripts/elua/core/gettext.luac
Removing src/scripts/elua/core/module.luac
Removing src/scripts/elua/core/util.luac
Removing src/scripts/elua/modules/getopt.luac
Removing src/scripts/elua/modules/lualian.luac

Oops, after a git clean -xdf, now building again. If it succeeds, may I ask not to leave those at *source dir*, please? :P

limachaves closed this task as Resolved.Aug 18 2017, 8:55 AM
q66 added a comment.Aug 18 2017, 8:57 AM

The right way really is to remove the bytecode if it fails loading and try loading the source file... so I guess I will do just that

Thanks a lot.

shouldn't a make clean nuke these too?

q66 added a comment.Aug 30 2017, 11:23 AM

I implemented a fix for the bytecode confusion in 67d1c0e51c01ba159f88adc6446cc31cee79c808 so similar cases should not happen again.

did you ensure make clean removes them? it doesn't. it absolutely should... :)