[Buildroot] [PATCH v1 3/4] openDHT: new package
Thomas Petazzoni
thomas.petazzoni at free-electrons.com
Fri Apr 22 21:30:54 UTC 2016
Hello,
Thanks for this contribution.
On Fri, 22 Apr 2016 16:57:07 -0400, Patrick Keroulas wrote:
> A lightweight C++11 Distributed Hash Table implementation originally
> based on https://github.com/jech/dht by Juliusz Chroboczek.
Please add your Signed-off-by here.
Also, the commit title should be in lower-case, to match the package
name:
opendht: new package
> diff --git a/package/opendht/Config.in b/package/opendht/Config.in
> new file mode 100644
> index 0000000..10bfd23
> --- /dev/null
> +++ b/package/opendht/Config.in
> @@ -0,0 +1,21 @@
> +config BR2_PACKAGE_OPENDHT
> + bool "opendht"
> + depends on BR2_USE_WCHAR #gnutls
Space after #
> + depends on BR2_INSTALL_LIBSTDCPP
> + depends on !BR2_STATIC_LIBS
> + depends on BR2_TOOLCHAIN_HAS_THREADS
> + depends on BR2_TOOLCHAIN_HAS_SYNC_4 #msgpack
Ditto.
> + select BR2_PACKAGE_GNUTLS
> + select BR2_PACKAGE_READLINE
> + select BR2_PACKAGE_MSGPACK
> + help
> + OpenDHT is lightweight C++11 Distributed Hash Table implementation.
> +
> + https://github.com/savoirfairelinux/opendht
Indentation for the help text is one tab + 2 spaces.
> +comment "opendht needs a toolchain w/ C++, thread, dynamic library"
> + depends on !BR2_INSTALL_LIBSTDCPP \
> + || !BR2_USE_WCHAR \
> + || !BR2_TOOLCHAIN_HAS_THREADS \
> + || !BR2_TOOLCHAIN_HAS_SYNC_4 \
> + || BR2_STATIC_LIBS
Please indent the follow-up lines with just one tab, and separate the \
with just one space. Also, the BR2_TOOLCHAIN_HAS_SYNC_4 is an
architecture dependency, so it needs to be handled differently.
Finally, you forgot the wchar dependency in the comment.
You want to have something like:
comment "opendht needs a toolchain w/ C++, thread, dynamic library, wchar"
depends on BR2_TOOLCHAIN_HAS_SYNC_4
depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
!BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
> diff --git a/package/opendht/opendht.hash b/package/opendht/opendht.hash
> new file mode 100644
> index 0000000..683957c
> --- /dev/null
> +++ b/package/opendht/opendht.hash
> @@ -0,0 +1,2 @@
> +# from local calculation
We normally write:
# Locally calculated
> +sha256 74ce5a5fc449509cf85912120d4243f93a006465e43ff3100c9c4fbd3001dc41 opendht-468331093345e718b5d269a71b3c52a2eea4797e.tar.gz
> diff --git a/package/opendht/opendht.mk b/package/opendht/opendht.mk
> new file mode 100644
> index 0000000..5bab245
> --- /dev/null
> +++ b/package/opendht/opendht.mk
> @@ -0,0 +1,26 @@
> +################################################################################
> +#
> +# opendht
> +#
> +################################################################################
> +
> +OPENDHT_VERSION = 468331093345e718b5d269a71b3c52a2eea4797e
> +OPENDHT_SITE_METHOD = git
> +#OPENDHT_SITE = $(call github,savoirfairelinux,opendht,$(OPENDHT_VERSION))
> +OPENDHT_SITE = https://github.com/savoirfairelinux/opendht.git
Please use the github function call that you commented out, and remove
the _SITE_METHOD variable.
> +OPENDHT_LICENSE = GPLv3, X11
Which license applies to what part?
> +OPENDHT_LICENSE_FILES = COPYING
> +OPENDHT_AUTORECONF = YES
> +OPENDHT_INSTALL_STAGING = YES
> +OPENDHT_INSTALL_TARGET = YES
This last line is not needed, as it is the default.
> +OPENDHT_CONF_OPTS = --disable-static
Don't do this, let Buildroot pass --{enable,disable}-{static,shared}
depending on the configuration.
> +OPENDHT_CONF_ENV = \
> + GNUTLS_CFLAGS="-I$(STAGING_DIR)/usr/include" \
> + GNUTLS_LIBS="-I$(STAGING_DIR)/usr/lib -lgnutls -lnettle -lhogweed"
Why is this needed? The configure.ac is using pkg-config to find the
details about gnutls and other libraries, so there should really be no
need for such environment variables.
> +OPENDHT_DEPENDENCIES = \
> + host-pkgconf \
> + gnutls \
> + readline \
> + msgpack
> +
> +$(eval $(autotools-package))
Could you rework your patch to take into account those comments, and
send an updated version?
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
More information about the buildroot
mailing list