[Buildroot] [PATCH v2 1/3] package/lua-std-debug: new package

Thomas Petazzoni thomas.petazzoni at bootlin.com
Sun Dec 30 13:53:30 UTC 2018


Hello,

I have applied, but there were a few things to fix. I'm Cc'ing
François, who gave his Acked-by, so that he can hopefully take into
account these details for the next reviews. For me, it is *very* useful
to have the reviews of François on Lua patches, but it's even better
when such a review catches all the small details! :-)

On Sat, 29 Dec 2018 10:07:23 +0800, james.hilliard1 at gmail.com wrote:

>  package/Config.in                        |  1 +
>  package/lua-std-debug/Config.in          |  7 +++++++
>  package/lua-std-debug/lua-std-debug.hash |  2 ++
>  package/lua-std-debug/lua-std-debug.mk   | 16 ++++++++++++++++

First issue: no entry was added to the DEVELOPERS file. Even if
François entry covers all Lua modules, it's good when someone
contributes a new package that he gets referenced as such in the
DEVELOPERS file.


> diff --git a/package/lua-std-debug/Config.in b/package/lua-std-debug/Config.in
> new file mode 100644
> index 0000000..3774292
> --- /dev/null
> +++ b/package/lua-std-debug/Config.in
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_LUA_STD_DEBUG
> +	bool "lua-std-debug"
> +	depends on BR2_PACKAGE_HAS_LUAINTERPRETER

This dependency is not needed: all Lua modules inclusions in
package/Config.in are already enclosed in a
BR2_PACKAGE_HAS_LUAINTERPRETER & !BR2_STATIC_LIBS condition.

> diff --git a/package/lua-std-debug/lua-std-debug.hash b/package/lua-std-debug/lua-std-debug.hash
> new file mode 100644
> index 0000000..2c48711
> --- /dev/null
> +++ b/package/lua-std-debug/lua-std-debug.hash
> @@ -0,0 +1,2 @@
> +# Locally calculated
> +sha256 7f6b84283d4b78dafee17e7765dd5f1f8e75c3314169977f4dda0e7873616ce2  std._debug-1.0.1-1.src.rock

The hash for the license file was missing.

> diff --git a/package/lua-std-debug/lua-std-debug.mk b/package/lua-std-debug/lua-std-debug.mk
> new file mode 100644
> index 0000000..b9e6312
> --- /dev/null
> +++ b/package/lua-std-debug/lua-std-debug.mk
> @@ -0,0 +1,16 @@
> +################################################################################
> +#
> +# lua-std-debug
> +#
> +################################################################################
> +
> +LUA_STD_DEBUG_VERSION_UPSTREAM = 1.0.1
> +LUA_STD_DEBUG_VERSION = $(LUA_STD_DEBUG_VERSION_UPSTREAM)-1
> +LUA_STD_DEBUG_NAME_UPSTREAM = std._debug
> +LUA_STD_DEBUG_SUBDIR = _debug-$(LUA_STD_DEBUG_VERSION_UPSTREAM)
> +LUA_STD_DEBUG_ROCKSPEC = $(LUA_STD_DEBUG_NAME_UPSTREAM)-$(LUA_STD_DEBUG_VERSION).rockspec
> +LUA_STD_DEBUG_SOURCE = $(LUA_STD_DEBUG_NAME_UPSTREAM)-$(LUA_STD_DEBUG_VERSION).src.rock

I was a bit surprised that those values were not automatically inferred
by the luarocks package infrastructure. However indeed, due to the
infrastructure using $(call LOWERCASE) and the upstream name containing
a underscore, it gets turned into a dash, and it no longer matches.
I don't think it's worth fixing the infrastructure about this right
now, but it's worth keeping in mind, in case several other packages are
in the same situation, in which case we could think about improving the
common infrastructure logic.

> +LUA_STD_DEBUG_LICENSE = MIT
> +LUA_STD_DEBUG_LICENSE_FILES = LICENSE.md

This was clearly not tested with "make legal-info". Indeed LICENSE.md
is inside LUA_STD_DEBUG_SUBDIR.

I fixed those details, and applied. Thanks!

Best regards

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


More information about the buildroot mailing list