[1/1] duktape: new package

Duktape is an embeddable Javascript engine, with a focus on
portability and compact footprint.

Duktape is easy to integrate into a C/C++ project: add duktape.c,
duktape.h, and duk_config.h to your build, and use the Duktape API
to call Ecmascript functions from C code and vice versa.

http://www.duktape.org

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 DEVELOPERS                                         |  1 +
 package/Config.in                                  |  1 +
 package/duktape/0001-Replace-gcc-by-CC.patch       | 46 +++++++++++++++
 ...0002-Don-t-copy-headers-in-install-target.patch | 28 ++++++++++
 package/duktape/Config.in                          | 11 ++++
 package/duktape/duktape.hash                       |  3 +
 package/duktape/duktape.mk                         | 65 ++++++++++++++++++++++
 7 files changed, 155 insertions(+)
 create mode 100644 package/duktape/0001-Replace-gcc-by-CC.patch
 create mode 100644 package/duktape/0002-Don-t-copy-headers-in-install-target.patch
 create mode 100644 package/duktape/Config.in
 create mode 100644 package/duktape/duktape.hash
 create mode 100644 package/duktape/duktape.mk

Hello,

Thanks for this new contribution!

On Fri,  3 Nov 2017 20:53:43 +0100, Fabrice Fontaine wrote:

> diff --git a/package/duktape/0002-Don-t-copy-headers-in-install-target.patch b/package/duktape/0002-Don-t-copy-headers-in-install-target.patch
> new file mode 100644
> index 000000000..391a89f6d
> --- /dev/null
> +++ b/package/duktape/0002-Don-t-copy-headers-in-install-target.patch
> @@ -0,0 +1,28 @@
> +From e5e839b96332229a860424b451fe3d678ff2d0f2 Mon Sep 17 00:00:00 2001
> +From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> +Date: Fri, 3 Nov 2017 20:10:41 +0100
> +Subject: [PATCH] Don't copy headers in install target
> +
> +Don't copy headers in install target to be able to use this Makefile to
> +install duktape library in staging and target directory

I'm not sure what you're trying to do here. Install headers to
$(TARGET_DIR) is perfectly fine, they get cleaned up at the end of the
build during the target-finalize step. Essentially all libraries
install everything to $(TARGET_DIR): headers, static library, shared
library, pkg-config files, documentation, and what is not needed on the
target gets cleaned up by the logic in target-finalize.

We don't want to hack each and every package so that it doesn't install
its headers or static library to $(TARGET_DIR). We very much prefer to
use the existing "make install" logic of packages, and have a common,
shared, logic in target-finalize to do the cleanup.

Or did I miss the purpose of this particular patch?

Thanks!

Thomas

Dear Thomas,

Thanks for this quick review.

2017-11-03 22:35 GMT+01:00 Thomas Petazzoni <
thomas.petazzoni@free-electrons.com>:

> Hello,
>
> Thanks for this new contribution!
>
> On Fri,  3 Nov 2017 20:53:43 +0100, Fabrice Fontaine wrote:
>
> > diff --git a/package/duktape/0002-Don-t-copy-headers-in-install-target.patch
> b/package/duktape/0002-Don-t-copy-headers-in-install-target.patch
> > new file mode 100644
> > index 000000000..391a89f6d
> > --- /dev/null
> > +++ b/package/duktape/0002-Don-t-copy-headers-in-install-target.patch
> > @@ -0,0 +1,28 @@
> > +From e5e839b96332229a860424b451fe3d678ff2d0f2 Mon Sep 17 00:00:00 2001
> > +From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > +Date: Fri, 3 Nov 2017 20:10:41 +0100
> > +Subject: [PATCH] Don't copy headers in install target
> > +
> > +Don't copy headers in install target to be able to use this Makefile to
> > +install duktape library in staging and target directory
>
> I'm not sure what you're trying to do here. Install headers to
> $(TARGET_DIR) is perfectly fine, they get cleaned up at the end of the
> build during the target-finalize step. Essentially all libraries
> install everything to $(TARGET_DIR): headers, static library, shared
> library, pkg-config files, documentation, and what is not needed on the
> target gets cleaned up by the logic in target-finalize.
>
> We don't want to hack each and every package so that it doesn't install
> its headers or static library to $(TARGET_DIR). We very much prefer to
> use the existing "make install" logic of packages, and have a common,
> shared, logic in target-finalize to do the cleanup.
>
> Or did I miss the purpose of this particular patch?
>
The issue with the original Makefile is that it tries to copy the two
header files in $(TARGET_DIR)/usr/include which does not exist so I had
several choices:
- create this directory in duktape.mk
- add "-a" option to the cp in the original Makefile
- remove the cp

I chose the third option but I can go with the first or second one. Which
one do you prefer?

>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
Best Regards,

Fabrice
<div dir="ltr"><div>Dear Thomas,<br><br></div>Thanks for this quick review.<br><div class="gmail_extra"><br><div class="gmail_quote">2017-11-03 22:35 GMT+01:00 Thomas Petazzoni <span dir="ltr">&lt;<a href="mailto:thomas.petazzoni@free-electrons.com" target="_blank">thomas.petazzoni@free-electrons.com</a>&gt;</span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello,<br>
<br>
Thanks for this new contribution!<br>
<span class=""><br>
On Fri,  3 Nov 2017 20:53:43 +0100, Fabrice Fontaine wrote:<br>
<br>
&gt; diff --git a/package/duktape/0002-Don-t-<wbr>copy-headers-in-install-<wbr>target.patch b/package/duktape/0002-Don-t-<wbr>copy-headers-in-install-<wbr>target.patch<br>
&gt; new file mode 100644<br>
&gt; index 000000000..391a89f6d<br>
&gt; --- /dev/null<br>
&gt; +++ b/package/duktape/0002-Don-t-<wbr>copy-headers-in-install-<wbr>target.patch<br>
&gt; @@ -0,0 +1,28 @@<br>
&gt; +From e5e839b96332229a860424b451fe3d<wbr>678ff2d0f2 Mon Sep 17 00:00:00 2001<br>
&gt; +From: Fabrice Fontaine &lt;<a href="mailto:fontaine.fabrice@gmail.com">fontaine.fabrice@gmail.com</a>&gt;<br>
&gt; +Date: Fri, 3 Nov 2017 20:10:41 +0100<br>
&gt; +Subject: [PATCH] Don&#39;t copy headers in install target<br>
&gt; +<br>
&gt; +Don&#39;t copy headers in install target to be able to use this Makefile to<br>
&gt; +install duktape library in staging and target directory<br>
<br>
</span>I&#39;m not sure what you&#39;re trying to do here. Install headers to<br>
$(TARGET_DIR) is perfectly fine, they get cleaned up at the end of the<br>
build during the target-finalize step. Essentially all libraries<br>
install everything to $(TARGET_DIR): headers, static library, shared<br>
library, pkg-config files, documentation, and what is not needed on the<br>
target gets cleaned up by the logic in target-finalize.<br>
<br>
We don&#39;t want to hack each and every package so that it doesn&#39;t install<br>
its headers or static library to $(TARGET_DIR). We very much prefer to<br>
use the existing &quot;make install&quot; logic of packages, and have a common,<br>
shared, logic in target-finalize to do the cleanup.<br>
<br>
Or did I miss the purpose of this particular patch?<br></blockquote><div>The issue with the original Makefile is that it tries to copy the two header files in $(TARGET_DIR)/usr/include which does not exist so I had several choices:</div><div>- create this directory in <a href="http://duktape.mk">duktape.mk</a></div><div>- add &quot;-a&quot; option to the cp in the original Makefile<br></div><div>- remove the cp</div><div><br></div><div>I chose the third option but I can go with the first or second one. Which one do you prefer?<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
Thomas<br>
--<br>
Thomas Petazzoni, CTO, Free Electrons<br>
Embedded Linux and Kernel engineering<br>
<a href="http://free-electrons.com" rel="noreferrer" target="_blank">http://free-electrons.com</a><br>
</font></span></blockquote></div>Best Regards,</div><div class="gmail_extra"><br></div><div class="gmail_extra">Fabrice<br></div></div>

>>>>> "Fabrice" == Fabrice Fontaine <fontaine.fabrice@gmail.com> writes:

Hi,

 > The issue with the original Makefile is that it tries to copy the two
 > header files in $(TARGET_DIR)/usr/include which does not exist so I had
 > several choices:
 > - create this directory in duktape.mk
 > - add "-a" option to the cp in the original Makefile

But cp -a doesn't create missing directories?

> - remove the cp

 > I chose the third option but I can go with the first or second one. Which
 > one do you prefer?

I would prefer to see a mkdir -p to ensure the directory exists and/or
using install -D to copy the files, as such patches should be acceptable
upstream.

Hello,

On Fri,  3 Nov 2017 20:53:43 +0100, Fabrice Fontaine wrote:
> Duktape is an embeddable Javascript engine, with a focus on
> portability and compact footprint.
> 
> Duktape is easy to integrate into a C/C++ project: add duktape.c,
> duktape.h, and duk_config.h to your build, and use the Duktape API
> to call Ecmascript functions from C code and vice versa.
> 
> http://www.duktape.org
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

In addition to the comments already made by Peter and me, I have one
more comment below.


> +# For static library, nothing is provided by duktape so build and install it
> +ifeq ($(BR2_STATIC_LIBS)$(BR2_SHARED_STATIC_LIBS),y)
> +define DUKTAPE_BUILD_STATIC
> +	$(TARGET_CC) $(TARGET_CFLAGS) -c $(@D)/src/duktape.c \
> +		-o $(@D)/libduktape.a
> +	$(TARGET_CC) $(TARGET_CFLAGS) -g -c $(@D)/src/duktape.c \
> +		-o $(@D)/libduktaped.a
> +endef
> +
> +define DUKTAPE_INSTALL_STATIC
> +	$(INSTALL) -m 0644 -D $(@D)/libduktape.a \
> +		$(STAGING_DIR)/usr/lib/libduktape.a
> +	$(INSTALL) -m 0644 -D $(@D)/libduktaped.a \
> +		$(STAGING_DIR)/usr/lib/libduktaped.a
> +endef
> +endif

Is duktape build system doesn't support building/installing a static
library, then just don't support it (or submit a patch upstream to add
that support). In the mean time, make the package depends
on !BR2_STATIC_LIBS and just don't support static library building.

I'll mark your patch as Changes Requested in patchwork.

Best regards,

Thomas