diff mbox

[1/5] lua : fix autobuild.buildroot

Message ID 1342446352-31231-1-git-send-email-francois.perrad@gadz.org
State Rejected
Headers show

Commit Message

Francois Perrad July 16, 2012, 1:45 p.m. UTC
BR2_PACKAGE_LUA_SHARED_LIBRARY is an option for install, not for build
---
 package/lua/lua.mk |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Thomas Petazzoni July 16, 2012, 3:39 p.m. UTC | #1
Le Mon, 16 Jul 2012 15:45:48 +0200,
Francois Perrad <fperrad@gmail.com> a écrit :

> BR2_PACKAGE_LUA_SHARED_LIBRARY is an option for install, not for build
> ---
>  package/lua/lua.mk |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/package/lua/lua.mk b/package/lua/lua.mk
> index baa459c..2fc838b 100644
> --- a/package/lua/lua.mk
> +++ b/package/lua/lua.mk
> @@ -8,10 +8,7 @@ LUA_VERSION = 5.1.5
>  LUA_SITE = http://www.lua.org/ftp
>  LUA_INSTALL_STAGING = YES
>  
> -ifeq ($(BR2_PACKAGE_LUA_SHARED_LIBRARY),y)
> -	LUA_MYCFLAGS += -fPIC
> -endif
> -
> +LUA_MYCFLAGS += -fPIC
>  LUA_MYLIBS += -ldl
>  
>  ifeq ($(BR2_PACKAGE_LUA_INTERPRETER_READLINE),y)

If Lua is not build as a shared library, there is no point in building
the object files with the -fPIC argument, so the existing code seems to
make sense to me. The BR2_PACKAGE_LUA_SHARED_LIBRARY affects both the
build time (we don't build with -fPIC) and the install time (we install
the shared libraries).

But I'm confused: during the Libre Software Meeting, didn't we discussed
that the BR2_PACKAGE_LUA_SHARED_LIBRARY option should go away, and that
the BR2_PACKAGE_LUA option should unconditionally install the shared
libraries, and then there would be sub-options for the interpreter and
compiler?

Thanks!

Thomas
François Perrad July 17, 2012, 8:35 a.m. UTC | #2
2012/7/16 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> Le Mon, 16 Jul 2012 15:45:48 +0200,
> Francois Perrad <fperrad@gmail.com> a écrit :
>
>> BR2_PACKAGE_LUA_SHARED_LIBRARY is an option for install, not for build
>> ---
>>  package/lua/lua.mk |    5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/package/lua/lua.mk b/package/lua/lua.mk
>> index baa459c..2fc838b 100644
>> --- a/package/lua/lua.mk
>> +++ b/package/lua/lua.mk
>> @@ -8,10 +8,7 @@ LUA_VERSION = 5.1.5
>>  LUA_SITE = http://www.lua.org/ftp
>>  LUA_INSTALL_STAGING = YES
>>
>> -ifeq ($(BR2_PACKAGE_LUA_SHARED_LIBRARY),y)
>> -     LUA_MYCFLAGS += -fPIC
>> -endif
>> -
>> +LUA_MYCFLAGS += -fPIC
>>  LUA_MYLIBS += -ldl
>>
>>  ifeq ($(BR2_PACKAGE_LUA_INTERPRETER_READLINE),y)
>
> If Lua is not build as a shared library, there is no point in building
> the object files with the -fPIC argument, so the existing code seems to
> make sense to me. The BR2_PACKAGE_LUA_SHARED_LIBRARY affects both the
> build time (we don't build with -fPIC) and the install time (we install
> the shared libraries).
>
> But I'm confused: during the Libre Software Meeting, didn't we discussed
> that the BR2_PACKAGE_LUA_SHARED_LIBRARY option should go away, and that
> the BR2_PACKAGE_LUA option should unconditionally install the shared
> libraries, and then there would be sub-options for the interpreter and
> compiler?
>

I want remove BR2_PACKAGE_LUA_SHARED_LIBRARY only in Lua modules,
because it is an internal of the lua package, and Lua modules must be
used with Lua or LuaJIT.

see http://patchwork.ozlabs.org/patch/162294/

François

> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni July 17, 2012, 8:40 a.m. UTC | #3
Le Tue, 17 Jul 2012 10:35:15 +0200,
François Perrad <francois.perrad@gadz.org> a écrit :

> I want remove BR2_PACKAGE_LUA_SHARED_LIBRARY only in Lua modules,
> because it is an internal of the lua package, and Lua modules must be
> used with Lua or LuaJIT.
> 
> see http://patchwork.ozlabs.org/patch/162294/

Yes, you said that to me during the Libre Software Meeting, but this
modification isn't part of the 5 patches patch set you have sent
yesterday, so I was a bit confused.

Can we have a complete patch set, with *all* your proposed changes to
the Lua packaging?

It's really complex to review patches for which you say that anyway
such or such option is removed by a later patch, which is not in the
same patch series.

Thanks!

Thomas
François Perrad July 17, 2012, 2:46 p.m. UTC | #4
2012/7/17 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> Le Tue, 17 Jul 2012 10:35:15 +0200,
> François Perrad <francois.perrad@gadz.org> a écrit :
>
>> I want remove BR2_PACKAGE_LUA_SHARED_LIBRARY only in Lua modules,
>> because it is an internal of the lua package, and Lua modules must be
>> used with Lua or LuaJIT.
>>
>> see http://patchwork.ozlabs.org/patch/162294/
>
> Yes, you said that to me during the Libre Software Meeting, but this
> modification isn't part of the 5 patches patch set you have sent
> yesterday, so I was a bit confused.
>
> Can we have a complete patch set, with *all* your proposed changes to
> the Lua packaging?
>
> It's really complex to review patches for which you say that anyway
> such or such option is removed by a later patch, which is not in the
> same patch series.
>

There are 3 series of patches :

a serie for luajit
[1/2] luajit: new package
      work in progress
[2/2] luajit: fix dependencies of Lua modules

a serie for refactor lua
[PATCH 1/5] lua : fix autobuild.buildroot
[PATCH 2/5] lua : don't install static library in target
[PATCH 3/5] lua : don't install shared library in staging
[PATCH 4/5] lua : refactor with POST_PATH_HOOKS
[PATCH 5/5] lua: split and rename patches
trash them, only the last one deserves to be saved by a new version

a new serie for refactor lua
[PATCH 1/2] lua: split and rename patches v2
[PATCH 2/2] lua: refactor root path

François

> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
Thomas Petazzoni July 17, 2012, 2:57 p.m. UTC | #5
Le Tue, 17 Jul 2012 16:46:32 +0200,
François Perrad <francois.perrad@gadz.org> a écrit :

> There are 3 series of patches :
> 
> a serie for luajit
> [1/2] luajit: new package
>       work in progress
> [2/2] luajit: fix dependencies of Lua modules

Ok, so I need to wait for both of these that you prepare a nice patch
set that solves the bitness building problem for luajit. Correct?

> a serie for refactor lua
> [PATCH 1/5] lua : fix autobuild.buildroot
> [PATCH 2/5] lua : don't install static library in target
> [PATCH 3/5] lua : don't install shared library in staging
> [PATCH 4/5] lua : refactor with POST_PATH_HOOKS
> [PATCH 5/5] lua: split and rename patches
> trash them, only the last one deserves to be saved by a new version

Ok, so I ignore those 5 patches, correct?

> a new serie for refactor lua
> [PATCH 1/2] lua: split and rename patches v2
> [PATCH 2/2] lua: refactor root path

Ok, I will merge them.

Thanks,

Thomas
François Perrad July 17, 2012, 3:20 p.m. UTC | #6
2012/7/17 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>:
> Le Tue, 17 Jul 2012 16:46:32 +0200,
> François Perrad <francois.perrad@gadz.org> a écrit :
>
>> There are 3 series of patches :
>>
>> a serie for luajit
>> [1/2] luajit: new package
>>       work in progress
>> [2/2] luajit: fix dependencies of Lua modules
>
> Ok, so I need to wait for both of these that you prepare a nice patch
> set that solves the bitness building problem for luajit. Correct?
>

as you suggest, I use (with success) the proposal of Jean-Christophe
Plagnol-Villard
(http://patchwork.ozlabs.org/patch/153235/)
so now, in fact, I wait you integrate it.

>> a serie for refactor lua
>> [PATCH 1/5] lua : fix autobuild.buildroot
>> [PATCH 2/5] lua : don't install static library in target
>> [PATCH 3/5] lua : don't install shared library in staging
>> [PATCH 4/5] lua : refactor with POST_PATH_HOOKS
>> [PATCH 5/5] lua: split and rename patches
>> trash them, only the last one deserves to be saved by a new version
>
> Ok, so I ignore those 5 patches, correct?

ok
>
>> a new serie for refactor lua
>> [PATCH 1/2] lua: split and rename patches v2
>> [PATCH 2/2] lua: refactor root path
>
> Ok, I will merge them.
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
Thomas Petazzoni July 17, 2012, 9:35 p.m. UTC | #7
Le Tue, 17 Jul 2012 17:20:58 +0200,
François Perrad <francois.perrad@gadz.org> a écrit :

> as you suggest, I use (with success) the proposal of Jean-Christophe
> Plagnol-Villard
> (http://patchwork.ozlabs.org/patch/153235/)
> so now, in fact, I wait you integrate it.

Please send a proper patch set with all the patches that are needed. If
you want your changes to be integrated, they need to be easy to review
and integrate. And for the moment, you're just sending multiple
individual patches, hoping that someone will figure out which patch
still makes sense, how the patches relate between each other, etc.

I have applied your patches that rework the Lua interpreter patches,
but for the luajit package, I'm waiting for a clean patch series from
you in order to merge things. And when you send updated versions of
your patches, please send a new complete series of patches.

Thanks!

Thomas
diff mbox

Patch

diff --git a/package/lua/lua.mk b/package/lua/lua.mk
index baa459c..2fc838b 100644
--- a/package/lua/lua.mk
+++ b/package/lua/lua.mk
@@ -8,10 +8,7 @@  LUA_VERSION = 5.1.5
 LUA_SITE = http://www.lua.org/ftp
 LUA_INSTALL_STAGING = YES
 
-ifeq ($(BR2_PACKAGE_LUA_SHARED_LIBRARY),y)
-	LUA_MYCFLAGS += -fPIC
-endif
-
+LUA_MYCFLAGS += -fPIC
 LUA_MYLIBS += -ldl
 
 ifeq ($(BR2_PACKAGE_LUA_INTERPRETER_READLINE),y)