diff mbox

package/middleclass: new package

Message ID 20160606130409.19533-1-m.niestroj@grinn-global.com
State Changes Requested
Headers show

Commit Message

Marcin Niestroj June 6, 2016, 1:04 p.m. UTC 
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
---
 package/Config.in                    |  1 +
 package/middleclass/Config.in        |  9 +++++++++
 package/middleclass/middleclass.hash |  2 ++
 package/middleclass/middleclass.mk   | 18 ++++++++++++++++++
 4 files changed, 30 insertions(+)
 create mode 100644 package/middleclass/Config.in
 create mode 100644 package/middleclass/middleclass.hash
 create mode 100644 package/middleclass/middleclass.mk

Comments

Thomas Petazzoni June 7, 2016, 11:25 a.m. UTC | #1 
Hello,

On Mon,  6 Jun 2016 15:04:09 +0200, Marcin Niestroj wrote:
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>

Thanks for this contribution! However I have a few comments, see below.

> diff --git a/package/middleclass/Config.in b/package/middleclass/Config.in
> new file mode 100644
> index 0000000..712064a
> --- /dev/null
> +++ b/package/middleclass/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_MIDDLECLASS
> +	bool "middleclass"
> +	depends on BR2_PACKAGE_HAS_LUAINTERPRETER

This dependency is not necessary, since you're already including
package/middleclass/Config.in within the following condition:

if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS

> diff --git a/package/middleclass/middleclass.mk b/package/middleclass/middleclass.mk
> new file mode 100644
> index 0000000..3e21452
> --- /dev/null
> +++ b/package/middleclass/middleclass.mk
> @@ -0,0 +1,18 @@
> +################################################################################
> +#
> +# middleclass
> +#
> +################################################################################
> +
> +MIDDLECLASS_VERSION = v4.0.0
> +MIDDLECLASS_SITE = $(call github,kikito,middleclass,$(MIDDLECLASS_VERSION))
> +MIDDLECLASS_DEPENDENCIES = luainterpreter
> +MIDDLECLASS_LICENSE = MIT
> +MIDDLECLASS_LICENSE_FILES = MIT-LICENSE.txt
> +
> +define MIDDLECLASS_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0644 -D $(@D)/middleclass.lua \
> +		$(TARGET_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/middleclass.lua
> +endef
> +
> +$(eval $(generic-package))

Could you use the luarocks package infrastructure instead? The
middleclass package is available in luarocks, and it greatly simplifies
the Lua packages. See the luaexpat package for an example. The
Buildroot manual also has extensive documentation about the luarocks
package infrastructure.

Thanks a lot!

Thomas
Marcin Niestroj June 7, 2016, 11:57 a.m. UTC | #2 
Hi,

On 07.06.2016 13:25, Thomas Petazzoni wrote:
> Hello,
>
> On Mon,  6 Jun 2016 15:04:09 +0200, Marcin Niestroj wrote:
>> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
>
> Thanks for this contribution! However I have a few comments, see below.
>
>> diff --git a/package/middleclass/Config.in b/package/middleclass/Config.in
>> new file mode 100644
>> index 0000000..712064a
>> --- /dev/null
>> +++ b/package/middleclass/Config.in
>> @@ -0,0 +1,9 @@
>> +config BR2_PACKAGE_MIDDLECLASS
>> +	bool "middleclass"
>> +	depends on BR2_PACKAGE_HAS_LUAINTERPRETER
>
> This dependency is not necessary, since you're already including
> package/middleclass/Config.in within the following condition:
>
> if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS

Thanks for pointing it. I used some other package as reference, so I 
missed that.

>
>> diff --git a/package/middleclass/middleclass.mk b/package/middleclass/middleclass.mk
>> new file mode 100644
>> index 0000000..3e21452
>> --- /dev/null
>> +++ b/package/middleclass/middleclass.mk
>> @@ -0,0 +1,18 @@
>> +################################################################################
>> +#
>> +# middleclass
>> +#
>> +################################################################################
>> +
>> +MIDDLECLASS_VERSION = v4.0.0
>> +MIDDLECLASS_SITE = $(call github,kikito,middleclass,$(MIDDLECLASS_VERSION))
>> +MIDDLECLASS_DEPENDENCIES = luainterpreter
>> +MIDDLECLASS_LICENSE = MIT
>> +MIDDLECLASS_LICENSE_FILES = MIT-LICENSE.txt
>> +
>> +define MIDDLECLASS_INSTALL_TARGET_CMDS
>> +	$(INSTALL) -m 0644 -D $(@D)/middleclass.lua \
>> +		$(TARGET_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/middleclass.lua
>> +endef
>> +
>> +$(eval $(generic-package))
>
> Could you use the luarocks package infrastructure instead? The
> middleclass package is available in luarocks, and it greatly simplifies
> the Lua packages. See the luaexpat package for an example. The
> Buildroot manual also has extensive documentation about the luarocks
> package infrastructure.

I tried to use luarocks for that package. However I encountered two
issues with that approach:

1) middleclass is not consistent with it's version. In most places it
is 4.0.0. However in rockspec [1] it defines it's version as 4.0-0. I
guess it should be 4.0.0-0 instead. This inconsistency needs some
tweaks in Buildroot package makefile.

2) When middleclass is installed with luarocks on my host, it downloads
*.rockspec directly instead of *.src.rock. I think that Buildroot is
not supporting that case, but please correct me if I am wrong. It was 
not straightforward to make it work for me.

Above issues made me unable to add support for middleclass using
luarocks in a reasonable time. It was much simpler with current
approach.

[1] 
https://github.com/kikito/middleclass/blob/master/rockspecs/middleclass-4.0-0.rockspec

>
> Thanks a lot!
>
> Thomas
>
Thomas Petazzoni June 7, 2016, 12:26 p.m. UTC | #3 
Hello,

On Tue, 7 Jun 2016 13:57:20 +0200, Marcin Niestroj wrote:

> > if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS  
> 
> Thanks for pointing it. I used some other package as reference, so I 
> missed that.

Other packages might be wrong, do not hesitate to send patches to fix
them if you find such issues.

> > Could you use the luarocks package infrastructure instead? The
> > middleclass package is available in luarocks, and it greatly simplifies
> > the Lua packages. See the luaexpat package for an example. The
> > Buildroot manual also has extensive documentation about the luarocks
> > package infrastructure.  
> 
> I tried to use luarocks for that package. However I encountered two
> issues with that approach:

OK. François Perrad, who wrote the luarocks package infrastructure, is
in Cc of the discussion. Hopefully he can participate to the discussion
and give some feedback.

> 1) middleclass is not consistent with it's version. In most places it
> is 4.0.0. However in rockspec [1] it defines it's version as 4.0-0. I
> guess it should be 4.0.0-0 instead. This inconsistency needs some
> tweaks in Buildroot package makefile.

Hm, I'm not sure here, François can comment I guess.

> 
> 2) When middleclass is installed with luarocks on my host, it downloads
> *.rockspec directly instead of *.src.rock. I think that Buildroot is
> not supporting that case, but please correct me if I am wrong. It was 
> not straightforward to make it work for me.

Hum, Buildroot does download the .src.rock, since the luarocks package
infrastructure defines:

$(2)_SOURCE             ?= $(1)-$$($(3)_VERSION).src.rock

> Above issues made me unable to add support for middleclass using
> luarocks in a reasonable time. It was much simpler with current
> approach.

Right, but not the most appropriate approach. If the package is on
luarocks.org, it should work with our infrastructure. Let's see what
François can say about this.

Thanks!

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index 9f6e0d9..b48948e 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -544,6 +544,7 @@  menu "Lua libraries/modules"
 	source "package/luv/Config.in"
 	source "package/luvi/Config.in"
 	source "package/lzlib/Config.in"
+	source "package/middleclass/Config.in"
 	source "package/orbit/Config.in"
 	source "package/rings/Config.in"
 	source "package/turbolua/Config.in"
diff --git a/package/middleclass/Config.in b/package/middleclass/Config.in
new file mode 100644
index 0000000..712064a
--- /dev/null
+++ b/package/middleclass/Config.in
@@ -0,0 +1,9 @@ 
+config BR2_PACKAGE_MIDDLECLASS
+	bool "middleclass"
+	depends on BR2_PACKAGE_HAS_LUAINTERPRETER
+	help
+	  middleclass is a simple OOP library for Lua. It has
+	  inheritance, metamethods (operators), class variables
+	  and weak mixin support.
+
+	  https://github.com/kikito/middleclass
diff --git a/package/middleclass/middleclass.hash b/package/middleclass/middleclass.hash
new file mode 100644
index 0000000..3fc794a
--- /dev/null
+++ b/package/middleclass/middleclass.hash
@@ -0,0 +1,2 @@ 
+# Locally calculated
+sha256	fc7f3e79a77d4733fd80b2e310f7f34092bdb5888549f304a24efd624c56482e  middleclass-v4.0.0.tar.gz
diff --git a/package/middleclass/middleclass.mk b/package/middleclass/middleclass.mk
new file mode 100644
index 0000000..3e21452
--- /dev/null
+++ b/package/middleclass/middleclass.mk
@@ -0,0 +1,18 @@ 
+################################################################################
+#
+# middleclass
+#
+################################################################################
+
+MIDDLECLASS_VERSION = v4.0.0
+MIDDLECLASS_SITE = $(call github,kikito,middleclass,$(MIDDLECLASS_VERSION))
+MIDDLECLASS_DEPENDENCIES = luainterpreter
+MIDDLECLASS_LICENSE = MIT
+MIDDLECLASS_LICENSE_FILES = MIT-LICENSE.txt
+
+define MIDDLECLASS_INSTALL_TARGET_CMDS
+	$(INSTALL) -m 0644 -D $(@D)/middleclass.lua \
+		$(TARGET_DIR)/usr/share/lua/$(LUAINTERPRETER_ABIVER)/middleclass.lua
+endef
+
+$(eval $(generic-package))