Message ID | 20171103195343.29670-1-fontaine.fabrice@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [1/1] duktape: new package | expand |
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"><<a href="mailto:thomas.petazzoni@free-electrons.com" target="_blank">thomas.petazzoni@free-electrons.com</a>></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> > 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> > new file mode 100644<br> > index 000000000..391a89f6d<br> > --- /dev/null<br> > +++ b/package/duktape/0002-Don-t-<wbr>copy-headers-in-install-<wbr>target.patch<br> > @@ -0,0 +1,28 @@<br> > +From e5e839b96332229a860424b451fe3d<wbr>678ff2d0f2 Mon Sep 17 00:00:00 2001<br> > +From: Fabrice Fontaine <<a href="mailto:fontaine.fabrice@gmail.com">fontaine.fabrice@gmail.com</a>><br> > +Date: Fri, 3 Nov 2017 20:10:41 +0100<br> > +Subject: [PATCH] Don't copy headers in install target<br> > +<br> > +Don't copy headers in install target to be able to use this Makefile to<br> > +install duktape library in staging and target directory<br> <br> </span>I'm not sure what you'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't want to hack each and every package so that it doesn't install<br> its headers or static library to $(TARGET_DIR). We very much prefer to<br> use the existing "make install" 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 "-a" 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
diff --git a/DEVELOPERS b/DEVELOPERS index 154a3a784..446ec2dc3 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -587,6 +587,7 @@ F: package/alljoyn-base/ F: package/alljoyn-tcl/ F: package/alljoyn-tcl-base/ F: package/boinc/ +F: package/duktape/ F: package/gtksourceview/ F: package/gssdp/ F: package/gupnp/ diff --git a/package/Config.in b/package/Config.in index ce389ce6a..421b6635f 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1211,6 +1211,7 @@ menu "External AngularJS plugins" endmenu endif source "package/bootstrap/Config.in" + source "package/duktape/Config.in" source "package/explorercanvas/Config.in" source "package/flot/Config.in" source "package/jquery/Config.in" diff --git a/package/duktape/0001-Replace-gcc-by-CC.patch b/package/duktape/0001-Replace-gcc-by-CC.patch new file mode 100644 index 000000000..01ac5e044 --- /dev/null +++ b/package/duktape/0001-Replace-gcc-by-CC.patch @@ -0,0 +1,46 @@ +From 7e055e36b2822eb2d2acd254c7053c37a24d6e6d Mon Sep 17 00:00:00 2001 +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> +Date: Fri, 3 Nov 2017 15:31:09 +0100 +Subject: [PATCH] Replace gcc by $(CC) + +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +--- + Makefile.sharedlibrary | 8 +++++--- + 1 file changed, 5 insertions(+), 3 deletions(-) + +diff --git a/Makefile.sharedlibrary b/Makefile.sharedlibrary +index 56f82f9..9a110aa 100644 +--- a/Makefile.sharedlibrary ++++ b/Makefile.sharedlibrary +@@ -36,6 +36,8 @@ INSTALL_PREFIX=/usr/local + DUKTAPE_SRCDIR=./src + #DUKTAPE_SRCDIR=./src-noline + ++CC:=gcc ++ + .PHONY: all + all: libduktape.so.$(REAL_VERSION) libduktaped.so.$(REAL_VERSION) + +@@ -44,11 +46,11 @@ all: libduktape.so.$(REAL_VERSION) libduktaped.so.$(REAL_VERSION) + # to $INSTALL_PREFIX/include on installation. + + libduktape.so.$(REAL_VERSION): +- gcc -shared -fPIC -Wall -Wextra -Os -Wl,-soname,libduktape.so.$(SONAME_VERSION) \ ++ $(CC) -shared -fPIC -Wall -Wextra -Os -Wl,-soname,libduktape.so.$(SONAME_VERSION) \ + -o $@ $(DUKTAPE_SRCDIR)/duktape.c + + libduktaped.so.$(REAL_VERSION): +- gcc -shared -fPIC -g -Wall -Wextra -Os -Wl,-soname,libduktaped.so.$(SONAME_VERSION) \ ++ $(CC) -shared -fPIC -g -Wall -Wextra -Os -Wl,-soname,libduktaped.so.$(SONAME_VERSION) \ + -o $@ $(DUKTAPE_SRCDIR)/duktape.c + + # Symlinks depend on platform conventions. +@@ -68,4 +70,4 @@ install: libduktape.so.$(REAL_VERSION) libduktaped.so.$(REAL_VERSION) + #CCOPTS=-I/usr/local/include -L/usr/local/lib + CCOPTS=-I./examples/cmdline + duk: +- gcc $(CCOPTS) -Wall -Wextra -Os -o $@ ./examples/cmdline/duk_cmdline.c -lduktape -lm ++ $(CC) $(CCOPTS) -Wall -Wextra -Os -o $@ ./examples/cmdline/duk_cmdline.c -lduktape -lm +-- +2.14.1 + 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 + +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +--- + Makefile.sharedlibrary | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/Makefile.sharedlibrary b/Makefile.sharedlibrary +index 9a110aa..dfc63f3 100644 +--- a/Makefile.sharedlibrary ++++ b/Makefile.sharedlibrary +@@ -63,7 +63,6 @@ install: libduktape.so.$(REAL_VERSION) libduktaped.so.$(REAL_VERSION) + rm -f $(INSTALL_PREFIX)/lib/libduktaped.so $(INSTALL_PREFIX)/lib/libduktaped.so.$(SONAME_VERSION) + ln -s libduktaped.so.$(REAL_VERSION) $(INSTALL_PREFIX)/lib/libduktaped.so + ln -s libduktaped.so.$(REAL_VERSION) $(INSTALL_PREFIX)/lib/libduktaped.so.$(SONAME_VERSION) +- cp $(DUKTAPE_SRCDIR)/duktape.h $(DUKTAPE_SRCDIR)/duk_config.h $(INSTALL_PREFIX)/include/ + + # Note: assumes /usr/local/include/ and /usr/local/lib/ are in include/link + # path which may not be the case for all distributions. +-- +2.14.1 + diff --git a/package/duktape/Config.in b/package/duktape/Config.in new file mode 100644 index 000000000..b422afb58 --- /dev/null +++ b/package/duktape/Config.in @@ -0,0 +1,11 @@ +config BR2_PACKAGE_DUKTAPE + bool "duktape" + help + 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 diff --git a/package/duktape/duktape.hash b/package/duktape/duktape.hash new file mode 100644 index 000000000..37eb942e6 --- /dev/null +++ b/package/duktape/duktape.hash @@ -0,0 +1,3 @@ +# Locally computed: +sha256 794805992a5bf6d47c3f244bb1df41c528f4a1f2d1e2f81dfa33f0920ab016ef duktape-v2.2.0.tar.gz +sha256 5358498534dac625c89a69c10becf3dcc40f9af58e6b69ee358ebdf6934f49c6 LICENSE.txt diff --git a/package/duktape/duktape.mk b/package/duktape/duktape.mk new file mode 100644 index 000000000..7cb2f18ab --- /dev/null +++ b/package/duktape/duktape.mk @@ -0,0 +1,65 @@ +################################################################################ +# +# duktape +# +################################################################################ + +DUKTAPE_VERSION = v2.2.0 +DUKTAPE_SITE = $(call github,svaarala,duktape-releases,$(DUKTAPE_VERSION)) +DUKTAPE_LICENSE = MIT +DUKTAPE_LICENSE_FILES = LICENSE.txt +DUKTAPE_INSTALL_STAGING = YES + +DUKTAPE_MAKE_OPTS = \ + CC=$(TARGET_CC) + +# For dynamic library, use Makefile.sharedlibrary provided by duktape +ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y) +define DUKTAPE_BUILD_SHARED + $(TARGET_MAKE_ENV) $(MAKE) -f Makefile.sharedlibrary \ + $(DUKTAPE_MAKE_OPTS) -C $(@D) +endef + +# $1: destination directory +define DUKTAPE_INSTALL_SHARED + $(TARGET_MAKE_ENV) $(MAKE) -f Makefile.sharedlibrary \ + $(DUKTAPE_MAKE_OPTS) -C $(@D) INSTALL_PREFIX=$(1)/usr install +endef +endif + +# 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 + +define DUKTAPE_BUILD_CMDS + $(DUKTAPE_BUILD_SHARED) + $(DUKTAPE_BUILD_STATIC) +endef + +define DUKTAPE_INSTALL_STAGING_CMDS + $(INSTALL) -m 0644 -D $(@D)/src/duk_config.h \ + $(STAGING_DIR)/usr/include/duk_config.h + $(INSTALL) -m 0644 -D $(@D)/src/duktape.h \ + $(STAGING_DIR)/usr/include/duktape.h + $(call DUKTAPE_INSTALL_SHARED,$(STAGING_DIR)) + $(DUKTAPE_INSTALL_STATIC) +endef + +define DUKTAPE_INSTALL_TARGET_CMDS + $(call DUKTAPE_INSTALL_SHARED,$(TARGET_DIR)) +endef + +$(eval $(generic-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