Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] package/lua-gd: new package
Date: Mon, 25 Mar 2019 19:24:16 +0100	[thread overview]
Message-ID: <20190325192416.10177592@windsurf> (raw)
In-Reply-To: <20190323121150.20564-1-francois.perrad@gadz.org>

Hello Fran?ois,

Overall looks good, was about to apply, but I have a couple of
comments/questions, see below.

On Sat, 23 Mar 2019 13:11:50 +0100
Francois Perrad <fperrad@gmail.com> wrote:

> diff --git a/package/lua-gd/0001-Protect-declaration-of-LgdImageCreateFromPng-with-GD.patch b/package/lua-gd/0001-Protect-declaration-of-LgdImageCreateFromPng-with-GD.patch
> new file mode 100644
> index 000000000..0d507d603
> --- /dev/null
> +++ b/package/lua-gd/0001-Protect-declaration-of-LgdImageCreateFromPng-with-GD.patch
> @@ -0,0 +1,28 @@
> +Protect declaration of LgdImageCreateFromPng* with GD_PNG feature test macro
> +
> +If GD_PNG is false, neither LgdImageCreateFromPng nor
> +LgdImageCreateFromPngPtr would be implemented. We should avoid declaring
> +them too.
> +
> +Fetch from: https://github.com/ittner/lua-gd/pull/8
> +
> +Signed-off-by: Francois Perrad <francois.perrad@gadz.org>

Please use
https://github.com/ittner/lua-gd/commit/78afd1c5f1ceaed05b78ac42c297d87a493295fd.patch
and create a Git formatted patch, i.e:

	$ git clone https://github.com/ittner/lua-gd.git
	$ curl https://github.com/ittner/lua-gd/commit/78afd1c5f1ceaed05b78ac42c297d87a493295fd.patch | git am -s
	$ git format-patch HEAD^

> diff --git a/package/lua-gd/lua-gd.mk b/package/lua-gd/lua-gd.mk
> new file mode 100644
> index 000000000..77d63d4de
> --- /dev/null
> +++ b/package/lua-gd/lua-gd.mk
> @@ -0,0 +1,25 @@
> +################################################################################
> +#
> +# lua-gd
> +#
> +################################################################################
> +
> +LUA_GD_VERSION = e60b13b7977bb3424d7044976ccba5d42c256934
> +LUA_GD_SITE = $(call github,ittner,lua-gd,$(LUA_GD_VERSION))
> +LUA_GD_LICENSE = MIT
> +LUA_GD_LICENSE_FILES = COPYING
> +LUA_GD_DEPENDENCIES = luainterpreter gd
> +
> +define LUA_GD_BUILD_CMDS
> +	$(MAKE) -C $(@D) gd.so \
> +		GDLIBCONFIG="$(STAGING_DIR)/usr/bin/$(GD_CONFIG_SCRIPTS)" \

We don't typically use the <pkg>_CONFIG_SCRIPTS variable from another
package, because it may contains multiple entries, so please stick to:

	GDLIBCONFIG=$(STAGING_DIR)/usr/bin/gdlib-config

> +		CC=$(TARGET_CC) \
> +		CFLAGS="$(TARGET_CFLAGS) -DVERSION=\\\"$(GD_VERSION)r3\\\"" \

What is this $(GD_VERSION)r3 thing ? Why r3 ? If it's really needed
as-is, it deserves a small comment.

> +define LUA_GD_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D $(@D)/gd.so $(TARGET_DIR)/usr/lib/lua/$(LUAINTERPRETER_ABIVER)/gd.so

Please add an explicit -m option to define the permissions.

Thanks!

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

      reply	other threads:[~2019-03-25 18:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-23 12:11 [Buildroot] [PATCH] package/lua-gd: new package Francois Perrad
2019-03-25 18:24 ` Thomas Petazzoni [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190325192416.10177592@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox