diff mbox series

[1/1] package/minetest: don't depend on luajit

Message ID 20200727205621.2556101-1-james.hilliard1@gmail.com
State Rejected
Headers show
Series [1/1] package/minetest: don't depend on luajit | expand

Commit Message

James Hilliard July 27, 2020, 8:56 p.m. UTC
Since minetest has a fallback to a bundled lua when luajit is not
available we don't need to depend on luajit or any lua version at all.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 package/minetest/Config.in   |  2 --
 package/minetest/minetest.mk | 10 ++++++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

Comments

Thomas Petazzoni July 28, 2020, 7:09 a.m. UTC | #1
On Mon, 27 Jul 2020 14:56:21 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> Since minetest has a fallback to a bundled lua when luajit is not
> available we don't need to depend on luajit or any lua version at all.

In general, I think we try to not use bundled libraries even when they
exist. If minetest needs a Lua interpreter, it should always use an
external Lua interpreter, I believe.

Thomas
James Hilliard July 28, 2020, 10:17 a.m. UTC | #2
On Tue, Jul 28, 2020 at 1:09 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 27 Jul 2020 14:56:21 -0600
> James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > Since minetest has a fallback to a bundled lua when luajit is not
> > available we don't need to depend on luajit or any lua version at all.
>
> In general, I think we try to not use bundled libraries even when they
> exist. If minetest needs a Lua interpreter, it should always use an
> external Lua interpreter, I believe.
From my understanding luajit is the only supported external lua
interpreter for minetest. So I think it makes sense to not require
a specific lua interpreter for flexibility.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni July 28, 2020, 11:48 a.m. UTC | #3
On Tue, 28 Jul 2020 04:17:26 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> On Tue, Jul 28, 2020 at 1:09 AM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > On Mon, 27 Jul 2020 14:56:21 -0600
> > James Hilliard <james.hilliard1@gmail.com> wrote:
> >  
> > > Since minetest has a fallback to a bundled lua when luajit is not
> > > available we don't need to depend on luajit or any lua version at all.  
> >
> > In general, I think we try to not use bundled libraries even when they
> > exist. If minetest needs a Lua interpreter, it should always use an
> > external Lua interpreter, I believe.  
> From my understanding luajit is the only supported external lua
> interpreter for minetest. So I think it makes sense to not require
> a specific lua interpreter for flexibility.

I don't understand your reply, as I was not talking about using other
Lua interpreters.

What I'm saying is that if minetest has a mandatory dependency on
LuaJIT, then it should *always* use the external LuaJIT and never use
the bundled one. We prefer external libraries over bundled libraries in
general in the context of Buildroot.

Thomas
Yann E. MORIN July 28, 2020, 7:36 p.m. UTC | #4
James, All,

On 2020-07-28 13:48 +0200, Thomas Petazzoni spake thusly:
> On Tue, 28 Jul 2020 04:17:26 -0600
> James Hilliard <james.hilliard1@gmail.com> wrote:
> > On Tue, Jul 28, 2020 at 1:09 AM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> > > On Mon, 27 Jul 2020 14:56:21 -0600
> > > James Hilliard <james.hilliard1@gmail.com> wrote:
> > > > Since minetest has a fallback to a bundled lua when luajit is not
> > > > available we don't need to depend on luajit or any lua version at all.  
> > > In general, I think we try to not use bundled libraries even when they
> > > exist. If minetest needs a Lua interpreter, it should always use an
> > > external Lua interpreter, I believe.  
> > From my understanding luajit is the only supported external lua
> > interpreter for minetest. So I think it makes sense to not require
> > a specific lua interpreter for flexibility.
> I don't understand your reply, as I was not talking about using other
> Lua interpreters.
> What I'm saying is that if minetest has a mandatory dependency on
> LuaJIT, then it should *always* use the external LuaJIT and never use
> the bundled one. We prefer external libraries over bundled libraries in
> general in the context of Buildroot.

James, I think both Thomas and I understand your point of view. However,
I do agree with Thomas on that topic, that we do use external libraries
over bundled ones, even if that restricts the possibilities.

Besides, minetest is a game; it is not a critical component for which we
could consider bending the rules. Restricting minetest to use the
external luajit will not be a major hindrance for the overwhelming
majority of bBuildroot users, if at all (sorry Romain! ;-] ).

So I've marked this patch as rejected in patchwork.

Regards,
Yann E. MORIN.
James Hilliard July 28, 2020, 8:44 p.m. UTC | #5
On Tue, Jul 28, 2020 at 5:48 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Tue, 28 Jul 2020 04:17:26 -0600
> James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > On Tue, Jul 28, 2020 at 1:09 AM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> > >
> > > On Mon, 27 Jul 2020 14:56:21 -0600
> > > James Hilliard <james.hilliard1@gmail.com> wrote:
> > >
> > > > Since minetest has a fallback to a bundled lua when luajit is not
> > > > available we don't need to depend on luajit or any lua version at all.
> > >
> > > In general, I think we try to not use bundled libraries even when they
> > > exist. If minetest needs a Lua interpreter, it should always use an
> > > external Lua interpreter, I believe.
> > From my understanding luajit is the only supported external lua
> > interpreter for minetest. So I think it makes sense to not require
> > a specific lua interpreter for flexibility.
>
> I don't understand your reply, as I was not talking about using other
> Lua interpreters.
>
> What I'm saying is that if minetest has a mandatory dependency on
> LuaJIT, then it should *always* use the external LuaJIT and never use
> the bundled one. We prefer external libraries over bundled libraries in
> general in the context of Buildroot.
Oh, it doesn't have a mandatory LuaJIT dependency, the internal version
is not LuaJIT but some bundled regular Lua 5.1 variant AFAIK.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Romain Naour July 28, 2020, 9:47 p.m. UTC | #6
Hi James, all,

Le 28/07/2020 à 22:44, James Hilliard a écrit :
> On Tue, Jul 28, 2020 at 5:48 AM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
>>
>> On Tue, 28 Jul 2020 04:17:26 -0600
>> James Hilliard <james.hilliard1@gmail.com> wrote:
>>
>>> On Tue, Jul 28, 2020 at 1:09 AM Thomas Petazzoni
>>> <thomas.petazzoni@bootlin.com> wrote:
>>>>
>>>> On Mon, 27 Jul 2020 14:56:21 -0600
>>>> James Hilliard <james.hilliard1@gmail.com> wrote:
>>>>
>>>>> Since minetest has a fallback to a bundled lua when luajit is not
>>>>> available we don't need to depend on luajit or any lua version at all.
>>>>
>>>> In general, I think we try to not use bundled libraries even when they
>>>> exist. If minetest needs a Lua interpreter, it should always use an
>>>> external Lua interpreter, I believe.
>>> From my understanding luajit is the only supported external lua
>>> interpreter for minetest. So I think it makes sense to not require
>>> a specific lua interpreter for flexibility.
>>
>> I don't understand your reply, as I was not talking about using other
>> Lua interpreters.
>>
>> What I'm saying is that if minetest has a mandatory dependency on
>> LuaJIT, then it should *always* use the external LuaJIT and never use
>> the bundled one. We prefer external libraries over bundled libraries in
>> general in the context of Buildroot.
> Oh, it doesn't have a mandatory LuaJIT dependency, the internal version
> is not LuaJIT but some bundled regular Lua 5.1 variant AFAIK.

When I packaged minetest, I looked at distribution packaging:

https://www.archlinux.org/packages/community/x86_64/minetest/
https://src.fedoraproject.org/rpms/minetest/blob/master/f/minetest.spec#_38
https://salsa.debian.org/games-team/minetest/-/blob/master/debian/control#L22

minetest use luajit when available.
Only Debian support liblua5.1 when luajit is not supported.

I'm not sure it worth the effort since minetest require x11 and libgl.

Best regards,
Romain


>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
diff mbox series

Patch

diff --git a/package/minetest/Config.in b/package/minetest/Config.in
index b8ee175d52..19da290df4 100644
--- a/package/minetest/Config.in
+++ b/package/minetest/Config.in
@@ -5,7 +5,6 @@  config BR2_PACKAGE_MINETEST
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on BR2_PACKAGE_XORG7 # irrlicht
 	depends on BR2_PACKAGE_HAS_LIBGL # irrlicht
-	depends on BR2_PACKAGE_LUAJIT
 	select BR2_PACKAGE_IRRLICHT
 	select BR2_PACKAGE_GMP
 	select BR2_PACKAGE_JSONCPP
@@ -52,7 +51,6 @@  comment "sound support needs a toolchain w/ threads NPTL"
 endif
 
 comment "minetest needs a toolchain w/ C++, gcc >= 4.9, threads"
-	depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS
 	depends on !BR2_INSTALL_LIBSTDCPP \
 		|| !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 \
 		|| !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/minetest/minetest.mk b/package/minetest/minetest.mk
index e8fe5b45ce..c0301273be 100644
--- a/package/minetest/minetest.mk
+++ b/package/minetest/minetest.mk
@@ -9,12 +9,11 @@  MINETEST_SITE = $(call github,minetest,minetest,$(MINETEST_VERSION))
 MINETEST_LICENSE = LGPL-2.1+ (code), CC-BY-SA-3.0 (textures and sounds)
 MINETEST_LICENSE_FILES = LICENSE.txt
 
-MINETEST_DEPENDENCIES = gmp irrlicht jsoncpp luajit sqlite zlib
+MINETEST_DEPENDENCIES = gmp irrlicht jsoncpp sqlite zlib
 
 MINETEST_CONF_OPTS = \
 	-DDEFAULT_RUN_IN_PLACE=OFF \
 	-DENABLE_GLES=OFF \
-	-DENABLE_LUAJIT=ON \
 	-DENABLE_CURSES=OFF \
 	-DAPPLY_LOCALE_BLACKLIST=OFF \
 	-DENABLE_SYSTEM_GMP=ON \
@@ -75,6 +74,13 @@  else
 MINETEST_CONF_OPTS += -DENABLE_SPATIAL=OFF
 endif
 
+ifeq ($(BR2_PACKAGE_LUAJIT),y)
+MINETEST_DEPENDENCIES += luajit
+MINETEST_CONF_OPTS += -DENABLE_LUAJIT=ON
+else
+MINETEST_CONF_OPTS += -DENABLE_LUAJIT=OFF
+endif
+
 ifeq ($(BR2_PACKAGE_POSTGRESQL),y)
 MINETEST_DEPENDENCIES += postgresql
 MINETEST_CONF_OPTS += -DENABLE_POSTGRESQL=ON