| Message ID | 201707061544.v66FiZQQ020306@mx1.sonologic.net |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Thu, 6 Jul 2017 17:40:01 +0200, Koen Martens wrote: > Cap'n Proto is an insanely fast data interchange format > and capability-based RPC system. Think JSON, except > binary. Or think Protocol Buffers, except faster. In > fact, in benchmarks, Cap’n Proto is INFINITY TIMES > faster than Protocol Buffers. > > https://capnproto.org/index.html > > Signed-off-by: Koen Martens <gmc@sonologic.nl> Thanks for this contribution, the patch looks mostly good. Only a few comments below. > diff --git a/package/capnproto/0001-execinfo.patch b/package/capnproto/0001-execinfo.patch > new file mode 100644 > index 0000000..85ef716 > --- /dev/null > +++ b/package/capnproto/0001-execinfo.patch > @@ -0,0 +1,85 @@ > +Capnproto assumes that if __GLIBC__ is defined then > +execinfo.h is available. uClibc does not implement > +execinfo.h, but does define __GLIBC__ in features.h > +(documented as an acceptable lie). > + > +We add an explicit check for existence of execinfo.h > +to configure and use HAVE_EXECINFO_H in the code > +that includes execinfo.h > + > +A more permanent patch has been submitted upstream > +as a pull-request. > + > +https://github.com/capnproto/capnproto/pull/511 > + > +Signed-off-by: Koen Martens <gmc@sonologic.nl> Since upstream uses Git, we prefer to have a Git-formatted patch, i.e generated with "git format-patch -N". It should also have a proper commit title, i.e a single line at the beginning that summarizes the commit, separated from the rest of the commit log by one empty line. > + > +diff -Naur a/config.h.in b/config.h.in > +--- a/config.h.in 2017-06-08 20:03:58.000000000 +0200 > ++++ b/config.h.in 2017-07-06 12:26:45.559406000 +0200 > +diff -Naur a/configure b/configure > +--- a/configure 2017-06-08 20:03:58.000000000 +0200 > ++++ b/configure 2017-07-06 12:26:45.559406000 +0200 Please keep only the modifications on configure.ac and exception.c++, and enable CAPNPROTO_AUTORECONF = YES. This will re-generate the configure script. > diff --git a/package/capnproto/Config.in b/package/capnproto/Config.in > new file mode 100644 > index 0000000..d25ccf7 > --- /dev/null > +++ b/package/capnproto/Config.in > @@ -0,0 +1,16 @@ > +config BR2_PACKAGE_CAPNPROTO > + bool "capnproto" > + depends on BR2_HOST_GCC_AT_LEAST_4_8 # C++11 > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # C++11 > + help > + Cap'n Proto is an insanely fast data interchange format > + and capability-based RPC system. Think JSON, except > + binary. Or think Protocol Buffers, except faster. In > + fact, in benchmarks, Cap’n Proto is INFINITY TIMES > + faster than Protocol Buffers. > + > + https://capnproto.org/index.html > + > +comment "capnproto needs host and target gcc >= 4.8" > + depends on !BR2_HOST_GCC_AT_LEAST_4_8 || !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 > + > diff --git a/package/capnproto/capnproto.mk b/package/capnproto/capnproto.mk > new file mode 100644 > index 0000000..510a3b2 > --- /dev/null > +++ b/package/capnproto/capnproto.mk > @@ -0,0 +1,17 @@ > +################################################################################ > +# > +# capnproto > +# > +################################################################################ > + > +CAPNPROTO_VERSION = 0.6.1 > +CAPNPROTO_SOURCE = capnproto-c++-$(CAPNPROTO_VERSION).tar.gz > +CAPNPROTO_SITE = https://capnproto.org > +CAPNPROTO_LICENSE = MIT > +CAPNPROTO_LICENSE_FILES = LICENSE.txt > +CAPNPROTO_INSTALL_STAGING = YES > +CAPNPROTO_CONF_OPTS = --with-external-capnp > +CAPNPROTO_DEPENDENCIES = host-capnproto Could you give some details as to why the host capnproto is necessary to build the target capnproto ? Also, you forgot the hash file for this package. If possible, make sure to run your new package through utils/check-package and utils/test-pkg. Thanks! Thomas
On 06-07-17 17:40, Koen Martens wrote: [snip] > diff --git a/package/capnproto/Config.in b/package/capnproto/Config.in > new file mode 100644 > index 0000000..d25ccf7 > --- /dev/null > +++ b/package/capnproto/Config.in > @@ -0,0 +1,16 @@ > +config BR2_PACKAGE_CAPNPROTO > + bool "capnproto" > + depends on BR2_HOST_GCC_AT_LEAST_4_8 # C++11 > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # C++11 Something tells me that this needs depends on BR2_INSTALL_LIBSTDCPP Regards, Arnout > + help > + Cap'n Proto is an insanely fast data interchange format > + and capability-based RPC system. Think JSON, except > + binary. Or think Protocol Buffers, except faster. In > + fact, in benchmarks, Cap’n Proto is INFINITY TIMES > + faster than Protocol Buffers. > + > + https://capnproto.org/index.html > + > +comment "capnproto needs host and target gcc >= 4.8" > + depends on !BR2_HOST_GCC_AT_LEAST_4_8 || !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 > + [snip]
On 6 July 2017 21:16:12 CEST, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: >Hello, > >On Thu, 6 Jul 2017 17:40:01 +0200, Koen Martens wrote: >> diff --git a/package/capnproto/0001-execinfo.patch >b/package/capnproto/0001-execinfo.patch >> new file mode 100644 >> index 0000000..85ef716 >> --- /dev/null >> +++ b/package/capnproto/0001-execinfo.patch >> @@ -0,0 +1,85 @@ >> +Capnproto assumes that if __GLIBC__ is defined then >> +execinfo.h is available. uClibc does not implement >> +execinfo.h, but does define __GLIBC__ in features.h >> +(documented as an acceptable lie). >> + >> +We add an explicit check for existence of execinfo.h >> +to configure and use HAVE_EXECINFO_H in the code >> +that includes execinfo.h >> + >> +A more permanent patch has been submitted upstream >> +as a pull-request. >> + >> +https://github.com/capnproto/capnproto/pull/511 >> + >> +Signed-off-by: Koen Martens <gmc@sonologic.nl> > >Since upstream uses Git, we prefer to have a Git-formatted patch, i.e >generated with "git format-patch -N". It should also have a proper >commit title, i.e a single line at the beginning that summarizes the >commit, separated from the rest of the commit log by one empty line. Sure, except the published tarball is from a subdirectory of the upstream git repo. So creating a git patch with format-patch results in a patch that needs 'patch -p2' to apply in buildroot. I understand this is impossible. What do you prefer i do? Manually modify the patch to remove the subdir that should be stripped? >> +CAPNPROTO_DEPENDENCIES = host-capnproto > >Could you give some details as to why the host capnproto is necessary >to build the target capnproto ? Capnproto provides a compiler to compile capnproto message definitions to c++ code. The library uses capnproto messages itself, and thus needs the capnproto compiler to compile. This obviously must be the host capnproto compiler, because the target compiler will not run on the compiling host. Is that sufficient detail, or do you want this in the package files somewhere? If so, where? - Koen
Hello, On Fri, 07 Jul 2017 08:58:08 +0200, Koen Martens wrote: > >Since upstream uses Git, we prefer to have a Git-formatted patch, i.e > >generated with "git format-patch -N". It should also have a proper > >commit title, i.e a single line at the beginning that summarizes the > >commit, separated from the rest of the commit log by one empty line. > > Sure, except the published tarball is from a subdirectory of the > upstream git repo. So creating a git patch with format-patch results > in a patch that needs 'patch -p2' to apply in buildroot. I understand > this is impossible. What do you prefer i do? Manually modify the > patch to remove the subdir that should be stripped? Gah, OK. Then perhaps a quick comment in the patch description would be nice, like: [Patch cannot be generated with git format-patch, as the upstream Git repository directory layout doesn't match the tarball directory layout] > >> +CAPNPROTO_DEPENDENCIES = host-capnproto > > > >Could you give some details as to why the host capnproto is necessary > >to build the target capnproto ? > > Capnproto provides a compiler to compile capnproto message > definitions to c++ code. The library uses capnproto messages itself, > and thus needs the capnproto compiler to compile. This obviously must > be the host capnproto compiler, because the target compiler will not > run on the compiling host. > > Is that sufficient detail, or do you want this in the package files > somewhere? If so, where? A quick comment above the dependency would be nice, like: # Needs the capnproto compiler on the host to generate C++ code from # message definitions Thanks! Thomas
Hi, On Fri, Jul 07, 2017 at 09:44:43AM +0200, Thomas Petazzoni wrote: > On Fri, 07 Jul 2017 08:58:08 +0200, Koen Martens wrote: > > >Since upstream uses Git, we prefer to have a Git-formatted patch, i.e > > >generated with "git format-patch -N". It should also have a proper > > >commit title, i.e a single line at the beginning that summarizes the > > >commit, separated from the rest of the commit log by one empty line. > > > > Sure, except the published tarball is from a subdirectory of the > > upstream git repo. So creating a git patch with format-patch results > > in a patch that needs 'patch -p2' to apply in buildroot. I understand > > this is impossible. What do you prefer i do? Manually modify the > > patch to remove the subdir that should be stripped? > > Gah, OK. Then perhaps a quick comment in the patch description would be > nice, like: > > [Patch cannot be generated with git format-patch, as the upstream Git > repository directory layout doesn't match the tarball directory layout] It's a temporary patch anyway, i have an open merge request with upstream. Anyway, i'm trying to utils/test-pkg capnproto with CAPNPROTO_AUTORECONF = YES, but get failures such as: /bin/bash: /home/gmc/br-test-pkg/armv5-ctng-linux-gnueabi/host/bin/autoreconf: No such file or directory Interestingly, /home/gmc/br-test-pkg/armv5-ctng-linux-gnueabi/host/usr/bin/autoreconf does exist, so with usr/bin instead of bin. I have a one-line capnproto.conf i feed to test-pkg: BR2_PACKAGE_CAPNPROTO=y What am i doing wrong? - Koen
Hello, On Fri, 7 Jul 2017 19:39:11 +0200, Koen Martens wrote: > It's a temporary patch anyway, i have an open merge request with > upstream. > > Anyway, i'm trying to utils/test-pkg capnproto with > CAPNPROTO_AUTORECONF = YES, but get failures such as: > > /bin/bash: > /home/gmc/br-test-pkg/armv5-ctng-linux-gnueabi/host/bin/autoreconf: No > such file or directory > > Interestingly, > /home/gmc/br-test-pkg/armv5-ctng-linux-gnueabi/host/usr/bin/autoreconf > does exist, so with usr/bin instead of bin. Are you doing completely clean builds here? If you have already done a run of test-pkg before, make sure to clean up the build folders. We have recently moved $(HOST_DIR)/usr to $(HOST_DIR), and it seems like you have a mix of things before and after this change. If that still doesn't work after a full rebuild, let us know, there might be a real bug somewhere, though I would assume the autobuilders would have caught such an obvious problem. Thomas
Hi, On Fri, Jul 07, 2017 at 08:16:43PM +0200, Thomas Petazzoni wrote: > On Fri, 7 Jul 2017 19:39:11 +0200, Koen Martens wrote: > > Anyway, i'm trying to utils/test-pkg capnproto with > > CAPNPROTO_AUTORECONF = YES, but get failures such as: > > > > /bin/bash: > > /home/gmc/br-test-pkg/armv5-ctng-linux-gnueabi/host/bin/autoreconf: No > > such file or directory > > > > Interestingly, > > /home/gmc/br-test-pkg/armv5-ctng-linux-gnueabi/host/usr/bin/autoreconf > > does exist, so with usr/bin instead of bin. > > Are you doing completely clean builds here? If you have already done a > run of test-pkg before, make sure to clean up the build folders. > > We have recently moved $(HOST_DIR)/usr to $(HOST_DIR), and it seems > like you have a mix of things before and after this change. > > If that still doesn't work after a full rebuild, let us know, there > might be a real bug somewhere, though I would assume the autobuilders > would have caught such an obvious problem. *doh*, indeed I had some old stuff in ~/br-test-pkg, thanks! - Koen
diff --git a/DEVELOPERS b/DEVELOPERS index c409f28..6ad12d2 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1023,6 +1023,7 @@ N: Kevin Joly <kevin.joly@sensefly.com> F: package/libgphoto2/ N: Koen Martens <gmc@sonologic.nl> +F: package/capnproto/ F: package/linuxconsoletools/ N: Laurent Cans <laurent.cans@gmail.com> diff --git a/package/Config.in b/package/Config.in index 46c78a0..0619103 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1375,6 +1375,7 @@ menu "Other" source "package/bctoolbox/Config.in" source "package/bdwgc/Config.in" source "package/boost/Config.in" + source "package/capnproto/Config.in" source "package/clapack/Config.in" source "package/classpath/Config.in" source "package/cppcms/Config.in" diff --git a/package/capnproto/0001-execinfo.patch b/package/capnproto/0001-execinfo.patch new file mode 100644 index 0000000..85ef716 --- /dev/null +++ b/package/capnproto/0001-execinfo.patch @@ -0,0 +1,85 @@ +Capnproto assumes that if __GLIBC__ is defined then +execinfo.h is available. uClibc does not implement +execinfo.h, but does define __GLIBC__ in features.h +(documented as an acceptable lie). + +We add an explicit check for existence of execinfo.h +to configure and use HAVE_EXECINFO_H in the code +that includes execinfo.h + +A more permanent patch has been submitted upstream +as a pull-request. + +https://github.com/capnproto/capnproto/pull/511 + +Signed-off-by: Koen Martens <gmc@sonologic.nl> + +diff -Naur a/config.h.in b/config.h.in +--- a/config.h.in 2017-06-08 20:03:58.000000000 +0200 ++++ b/config.h.in 2017-07-06 12:26:45.559406000 +0200 +@@ -3,6 +3,9 @@ + /* Define to 1 if you have the <dlfcn.h> header file. */ + #undef HAVE_DLFCN_H + ++/* Define to 1 if you have the <execinfo.h> header file. */ ++#undef HAVE_EXECINFO_H ++ + /* Define to 1 if you have the <inttypes.h> header file. */ + #undef HAVE_INTTYPES_H + +diff -Naur a/configure b/configure +--- a/configure 2017-06-08 20:03:58.000000000 +0200 ++++ b/configure 2017-07-06 12:26:45.559406000 +0200 +@@ -9157,6 +9157,20 @@ + done + + ++for ac_header in execinfo.h ++do : ++ ac_fn_c_check_header_compile "$LINENO" "execinfo.h" "ac_cv_header_execinfo_h" "$ac_includes_default ++" ++if test "x$ac_cv_header_execinfo_h" = xyes; then : ++ cat >>confdefs.h <<_ACEOF ++#define HAVE_EXECINFO_H 1 ++_ACEOF ++ ++fi ++ ++done ++ ++ + + func_stripname_cnf () + { +diff -Naur a/configure.ac b/configure.ac +--- a/configure.ac 2017-06-08 20:03:50.000000000 +0200 ++++ b/configure.ac 2017-07-06 12:26:45.559406000 +0200 +@@ -52,6 +52,8 @@ + AC_LANG([C++]) + AX_CXX_COMPILE_STDCXX_11 + ++AC_CHECK_HEADERS(execinfo.h) ++ + AS_CASE("${host_os}", *mingw*, [ + # We don't use pthreads on MinGW. + PTHREAD_CFLAGS="-mthreads" +diff -Naur a/src/kj/exception.c++ b/src/kj/exception.c++ +--- a/src/kj/exception.c++ 2017-06-04 19:03:27.000000000 +0200 ++++ b/src/kj/exception.c++ 2017-07-06 12:28:07.472341999 +0200 +@@ -19,6 +19,7 @@ + // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + // THE SOFTWARE. + ++#include "config.h" + #include "exception.h" + #include "string.h" + #include "debug.h" +@@ -33,7 +34,7 @@ + #endif + #include "io.h" + +-#if (__linux__ && __GLIBC__) || __APPLE__ ++#if HAVE_EXECINFO_H + #define KJ_HAS_BACKTRACE 1 + #include <execinfo.h> + #endif diff --git a/package/capnproto/Config.in b/package/capnproto/Config.in new file mode 100644 index 0000000..d25ccf7 --- /dev/null +++ b/package/capnproto/Config.in @@ -0,0 +1,16 @@ +config BR2_PACKAGE_CAPNPROTO + bool "capnproto" + depends on BR2_HOST_GCC_AT_LEAST_4_8 # C++11 + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # C++11 + help + Cap'n Proto is an insanely fast data interchange format + and capability-based RPC system. Think JSON, except + binary. Or think Protocol Buffers, except faster. In + fact, in benchmarks, Cap’n Proto is INFINITY TIMES + faster than Protocol Buffers. + + https://capnproto.org/index.html + +comment "capnproto needs host and target gcc >= 4.8" + depends on !BR2_HOST_GCC_AT_LEAST_4_8 || !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 + diff --git a/package/capnproto/capnproto.mk b/package/capnproto/capnproto.mk new file mode 100644 index 0000000..510a3b2 --- /dev/null +++ b/package/capnproto/capnproto.mk @@ -0,0 +1,17 @@ +################################################################################ +# +# capnproto +# +################################################################################ + +CAPNPROTO_VERSION = 0.6.1 +CAPNPROTO_SOURCE = capnproto-c++-$(CAPNPROTO_VERSION).tar.gz +CAPNPROTO_SITE = https://capnproto.org +CAPNPROTO_LICENSE = MIT +CAPNPROTO_LICENSE_FILES = LICENSE.txt +CAPNPROTO_INSTALL_STAGING = YES +CAPNPROTO_CONF_OPTS = --with-external-capnp +CAPNPROTO_DEPENDENCIES = host-capnproto + +$(eval $(autotools-package)) +$(eval $(host-autotools-package))
Cap'n Proto is an insanely fast data interchange format and capability-based RPC system. Think JSON, except binary. Or think Protocol Buffers, except faster. In fact, in benchmarks, Cap’n Proto is INFINITY TIMES faster than Protocol Buffers. https://capnproto.org/index.html Signed-off-by: Koen Martens <gmc@sonologic.nl> --- DEVELOPERS | 1 + package/Config.in | 1 + package/capnproto/0001-execinfo.patch | 85 +++++++++++++++++++++++++++++++++++ package/capnproto/Config.in | 16 +++++++ package/capnproto/capnproto.mk | 17 +++++++ 5 files changed, 120 insertions(+) create mode 100644 package/capnproto/0001-execinfo.patch create mode 100644 package/capnproto/Config.in create mode 100644 package/capnproto/capnproto.mk