diff mbox

[2/2] swupdate: support for Lua 5.1 is broken

Message ID 20170731211800.5888-2-joerg.krause@embedded.rocks
State Accepted
Headers show

Commit Message

Jörg Krause July 31, 2017, 9:18 p.m. UTC
Before commit 87b6ac1478821351c92f7ca1c3154550e4713b28 support for Lua
was always disabled by the default config file:

```
CONFIG_LUA is not set
```

The commit removed this setting and Lua support is now enabled if a Lua
interpreter is enabled. As the swupdate build system uses pkg-config to check
for the lua library by default (LUAPKG="lua") this throws a lot of
undefined referenced in case LuaJIT is uses as Lua interpreter, e.g.:

```
corelib/lib.a(lua_interface.o): In function `l_info':
lua_interface.c:(.text.l_info+0x14): undefined reference to `luaL_checklstring'
```

However, since version 2017.07 support for Lua 5.1 is broken in swupdate. Therefore,
remove support for Lua 5.1 and LuaJIT in the swupdate package for now
until upstream might fix this issue.

Reported upstream:
https://groups.google.com/forum/#!topic/swupdate/WAm8npAOd6o

Fixes:
http://autobuild.buildroot.net/results/df2/df2a71efe5af52d7b8721a355c49934b1be197a3/
http://autobuild.buildroot.net/results/400/4006225c8a47eb0b56399c83bd6d00406a0f62c2/
http://autobuild.buildroot.net/results/098/098f9d6cd905359adac4fb15e1d54c5022757325/
http://autobuild.buildroot.net/results/e5f/e5f6f99d96d9c661454335e7f931a03c3ae6a9e2/
http://autobuild.buildroot.net/results/f3a/f3a0abe8d5e35c85da40d20dab260c28506a0d4c/

Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks>
---
 package/swupdate/Config.in   | 7 ++-----
 package/swupdate/swupdate.mk | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

Comments

Thomas Petazzoni Aug. 1, 2017, 5:10 p.m. UTC | #1
Hello,

On Mon, 31 Jul 2017 23:18:00 +0200, Jörg Krause wrote:
> Before commit 87b6ac1478821351c92f7ca1c3154550e4713b28 support for Lua
> was always disabled by the default config file:
> 
> ```
> CONFIG_LUA is not set
> ```
> 
> The commit removed this setting and Lua support is now enabled if a Lua
> interpreter is enabled. As the swupdate build system uses pkg-config to check
> for the lua library by default (LUAPKG="lua") this throws a lot of
> undefined referenced in case LuaJIT is uses as Lua interpreter, e.g.:
> 
> ```
> corelib/lib.a(lua_interface.o): In function `l_info':
> lua_interface.c:(.text.l_info+0x14): undefined reference to `luaL_checklstring'
> ```
> 
> However, since version 2017.07 support for Lua 5.1 is broken in swupdate. Therefore,
> remove support for Lua 5.1 and LuaJIT in the swupdate package for now
> until upstream might fix this issue.

Isn't it better to revert the bump to 2017.07 instead of removing
functionality that people could be relying on ?

Thomas
Jörg Krause Aug. 2, 2017, 6:42 a.m. UTC | #2
Hi Thomas,

On Tue, 2017-08-01 at 19:10 +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 31 Jul 2017 23:18:00 +0200, Jörg Krause wrote:
> > Before commit 87b6ac1478821351c92f7ca1c3154550e4713b28 support for
> > Lua
> > was always disabled by the default config file:
> > 
> > ```
> > CONFIG_LUA is not set
> > ```
> > 
> > The commit removed this setting and Lua support is now enabled if a
> > Lua
> > interpreter is enabled. As the swupdate build system uses pkg-
> > config to check
> > for the lua library by default (LUAPKG="lua") this throws a lot of
> > undefined referenced in case LuaJIT is uses as Lua interpreter,
> > e.g.:
> > 
> > ```
> > corelib/lib.a(lua_interface.o): In function `l_info':
> > lua_interface.c:(.text.l_info+0x14): undefined reference to
> > `luaL_checklstring'
> > ```
> > 
> > However, since version 2017.07 support for Lua 5.1 is broken in
> > swupdate. Therefore,
> > remove support for Lua 5.1 and LuaJIT in the swupdate package for
> > now
> > until upstream might fix this issue.
> 
> Isn't it better to revert the bump to 2017.07 instead of removing
> functionality that people could be relying on ?

Lua 5.1 and LuaJIT were never supported if CONFIG_HANDLER_IN_LUA was
selected. Upstream changed the functionality that the Lua code used
when enabling CONFIG_HANDLER_IN_LUA is now used in more common places
and therefore enabled if CONFIG_LUA is enabled.

That's why, I should have reword the commit message to: "Lua 5.1 is not
supported" instead of "broken".

I am asking the Stefano Babic, the swupdate maintainer, if he is
interested in enabling Lua 5.1 support. For now, swupdate is explicitly
tested using Lua 5.3.

Best regards,
Jörg Krause
Jörg Krause Aug. 2, 2017, 7:46 a.m. UTC | #3
Hi Thomas,
Hi Jordan,

On Wed, 2017-08-02 at 08:42 +0200, Jörg Krause wrote:
> Hi Thomas,
> 
> On Tue, 2017-08-01 at 19:10 +0200, Thomas Petazzoni wrote:
> > Hello,
> > 
> > On Mon, 31 Jul 2017 23:18:00 +0200, Jörg Krause wrote:
> > > Before commit 87b6ac1478821351c92f7ca1c3154550e4713b28 support
> > > for
> > > Lua
> > > was always disabled by the default config file:
> > > 
> > > ```
> > > CONFIG_LUA is not set
> > > ```
> > > 
> > > The commit removed this setting and Lua support is now enabled if
> > > a
> > > Lua
> > > interpreter is enabled. As the swupdate build system uses pkg-
> > > config to check
> > > for the lua library by default (LUAPKG="lua") this throws a lot
> > > of
> > > undefined referenced in case LuaJIT is uses as Lua interpreter,
> > > e.g.:
> > > 
> > > ```
> > > corelib/lib.a(lua_interface.o): In function `l_info':
> > > lua_interface.c:(.text.l_info+0x14): undefined reference to
> > > `luaL_checklstring'
> > > ```
> > > 
> > > However, since version 2017.07 support for Lua 5.1 is broken in
> > > swupdate. Therefore,
> > > remove support for Lua 5.1 and LuaJIT in the swupdate package for
> > > now
> > > until upstream might fix this issue.
> > 
> > Isn't it better to revert the bump to 2017.07 instead of removing
> > functionality that people could be relying on ?
> 
> Lua 5.1 and LuaJIT were never supported if CONFIG_HANDLER_IN_LUA was
> selected. Upstream changed the functionality that the Lua code used
> when enabling CONFIG_HANDLER_IN_LUA is now used in more common places
> and therefore enabled if CONFIG_LUA is enabled.
> 
> That's why, I should have reword the commit message to: "Lua 5.1 is
> not
> supported" instead of "broken".
> 
> I am asking the Stefano Babic, the swupdate maintainer, if he is
> interested in enabling Lua 5.1 support. For now, swupdate is
> explicitly
> tested using Lua 5.3.

Stefano confirmed that he has no plans in adding support for Lua
5.1/LuaJIT himself, but patches are welcome [1].

@Jordan: You've added a patch to allow configuring swupdate with LuaJIT
last year. If you're still interested in support for LuaJIT, please
contact the swupdate maintainer on the swupdate mailing list.

[1] https://groups.google.com/d/msg/swupdate/WAm8npAOd6o/hPWMMkV_AAAJ

Jörg
Arnout Vandecappelle Aug. 10, 2017, 12:51 p.m. UTC | #4
On 31-07-17 23:18, Jörg Krause wrote:
> Before commit 87b6ac1478821351c92f7ca1c3154550e4713b28 support for Lua
> was always disabled by the default config file:
> 
> ```
> CONFIG_LUA is not set
> ```
> 
> The commit removed this setting and Lua support is now enabled if a Lua
> interpreter is enabled. As the swupdate build system uses pkg-config to check
> for the lua library by default (LUAPKG="lua") this throws a lot of
> undefined referenced in case LuaJIT is uses as Lua interpreter, e.g.:
> 
> ```
> corelib/lib.a(lua_interface.o): In function `l_info':
> lua_interface.c:(.text.l_info+0x14): undefined reference to `luaL_checklstring'
> ```
> 
> However, since version 2017.07 support for Lua 5.1 is broken in swupdate. Therefore,
> remove support for Lua 5.1 and LuaJIT in the swupdate package for now
> until upstream might fix this issue.
> 
> Reported upstream:
> https://groups.google.com/forum/#!topic/swupdate/WAm8npAOd6o
> 
> Fixes:
> http://autobuild.buildroot.net/results/df2/df2a71efe5af52d7b8721a355c49934b1be197a3/
> http://autobuild.buildroot.net/results/400/4006225c8a47eb0b56399c83bd6d00406a0f62c2/
> http://autobuild.buildroot.net/results/098/098f9d6cd905359adac4fb15e1d54c5022757325/
> http://autobuild.buildroot.net/results/e5f/e5f6f99d96d9c661454335e7f931a03c3ae6a9e2/
> http://autobuild.buildroot.net/results/f3a/f3a0abe8d5e35c85da40d20dab260c28506a0d4c/
> 
> Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks>

 Applied to master after updating the commit message as you suggested, thanks.

 Regards,
 Arnout
diff mbox

Patch

diff --git a/package/swupdate/Config.in b/package/swupdate/Config.in
index dbc69f1fe8..7a0f9d12e5 100644
--- a/package/swupdate/Config.in
+++ b/package/swupdate/Config.in
@@ -4,7 +4,7 @@  config BR2_PACKAGE_SWUPDATE
 	depends on BR2_USE_MMU # fork()
 	# swupdate requires a parser and uses libconfig as default
 	select BR2_PACKAGE_LIBCONFIG if !BR2_PACKAGE_JSON_C && \
-		!BR2_PACKAGE_HAS_LUAINTERPRETER
+		!BR2_PACKAGE_LUA_5_2 && !BR2_PACKAGE_LUA_5_3
 	help
 	  swupdate provides a reliable way to update the software on an
 	  embedded system.
@@ -19,11 +19,8 @@  config BR2_PACKAGE_SWUPDATE
 	  use your own modified configuration, you have to select the
 	  necessary packages manually:
 
-	  * Select BR2_PACKAGE_LUA or BR2_PACKAGE_LUAJIT if you want
+	  * Select BR2_PACKAGE_LUA_5_2 or BR2_PACKAGE_LUA_5_3 if you want
 	    to have Lua support.
-	    CONFIG_HANDLER_IN_LUA is not supported in LuaJIT or Lua 5.1.
-	    Note that for LuaJIT support, you need to set
-	    CONFIG_LUAVERSION="jit-5.1".
 	  * Select BR2_LIBCURL if you want to use the download feature.
 	  * Select BR2_PACKAGE_OPENSSL is you want to add encryption support.
 	  * Select BR2_PACKAGE_MTD if you want to use swupdate with UBI
diff --git a/package/swupdate/swupdate.mk b/package/swupdate/swupdate.mk
index ee92adddf0..fbb092da05 100644
--- a/package/swupdate/swupdate.mk
+++ b/package/swupdate/swupdate.mk
@@ -39,8 +39,8 @@  else
 SWUPDATE_MAKE_ENV += HAVE_LIBCURL=n
 endif
 
-ifeq ($(BR2_PACKAGE_HAS_LUAINTERPRETER),y)
-SWUPDATE_DEPENDENCIES += luainterpreter host-pkgconf
+ifeq ($(BR2_PACKAGE_LUA_5_2)$(BR2_PACKAGE_LUA_5_3),y)
+SWUPDATE_DEPENDENCIES += lua host-pkgconf
 SWUPDATE_MAKE_ENV += HAVE_LUA=y
 else
 SWUPDATE_MAKE_ENV += HAVE_LUA=n