[1/1] ibrdtn

Signed-off-by: Tom Sparks <tom_a_sparks@yahoo.com.au>
---
 package/ibrcommon/Config.in    | 13 +++++++++++++
 package/ibrcommon/ibrcommon.mk | 32 ++++++++++++++++++++++++++++++++
 package/ibrdtn/Config.in       | 13 +++++++++++++
 package/ibrdtn/ibrdtn.mk       | 32 ++++++++++++++++++++++++++++++++
 package/ibrdtnd/Config.in      | 23 +++++++++++++++++++++++
 package/ibrdtnd/ibrdtnd.mk     | 32 ++++++++++++++++++++++++++++++++
 6 files changed, 145 insertions(+)
 create mode 100644 package/ibrcommon/Config.in
 create mode 100644 package/ibrcommon/ibrcommon.mk
 create mode 100644 package/ibrdtn/Config.in
 create mode 100644 package/ibrdtn/ibrdtn.mk
 create mode 100644 package/ibrdtnd/Config.in
 create mode 100644 package/ibrdtnd/ibrdtnd.mk

Hello Tom,

I think it's better if you send separate patches for each new package 
you want to add, along with a good subject such as "new package: <pkg 
name>" or something similar.

Also you are not adding a new entry to package/Config.in in order to 
make your new packages able to be selected in the menu.

On 06/08/15 14:50, Tom Sparks wrote:
> +IBRCOMMON_CONF_OPTS = --with-openssl --with-lowpan   --with-xml

To many spaces between "--with-lowpan" and "--with-xml".

> +IBRCOMMON_INSTALL_STAGING_OPTS = 			\
> +	prefix=$(STAGING_DIR)/usr 		\
> +	exec_prefix=$(STAGING_DIR)/usr 		\
> +	PKG_DEVLIB_DIR=$(STAGING_DIR)/usr/lib	\
> +	install
> +
> +IBRCOMMON_INSTALL_TARGET_OPTS = 			\
> +	prefix=$(TARGET_DIR)/usr 		\
> +	exec_prefix=$(TARGET_DIR)/usr 		\
> +	install

I wouldn't try to align the back-slashes. Just a space and then the 
back-slash.

> +IBRDTN_INSTALL_STAGING_OPTS = 			\
> +	prefix=$(STAGING_DIR)/usr 		\
> +	exec_prefix=$(STAGING_DIR)/usr 		\
> +	PKG_DEVLIB_DIR=$(STAGING_DIR)/usr/lib	\
> +	install
> +
> +IBRDTN_INSTALL_TARGET_OPTS = 			\
> +	prefix=$(TARGET_DIR)/usr 		\
> +	exec_prefix=$(TARGET_DIR)/usr 		\
> +	install

Same here.

> +if BR2_PACKAGE_IBRDTND
> +
> +source "package/ibrcommon/Config.in"
> +source "package/ibrdtn/Config.in"
> +
> +endif

What are you trying to do with this? Making "ibrdtn" the only package 
selectable from menuconfig, and when you select it show the other two in 
the menu as well?

> +IBRDTND_CONF_OPTS =   --with-curl  --with-sqlite     --with-tls

More unnecessary spaces.

> +IBRDTND_INSTALL_STAGING_OPTS = 			\
> +	prefix=$(STAGING_DIR)/usr 		\
> +	exec_prefix=$(STAGING_DIR)/usr 		\
> +	PKG_DEVLIB_DIR=$(STAGING_DIR)/usr/lib	\
> +	install
> +
> +IBRDTND_INSTALL_TARGET_OPTS = 			\
> +	prefix=$(TARGET_DIR)/usr 		\
> +	exec_prefix=$(TARGET_DIR)/usr 		\
> +	install

No alignment, IMHO.

Regards,

Vincent.

Dear Tom Sparks,

Thanks for this contribution. Vicente already made a review with some
comments, I'll add some more comments in this review.

On Thu,  6 Aug 2015 22:50:24 +1000, Tom Sparks wrote:
> Signed-off-by: Tom Sparks <tom_a_sparks@yahoo.com.au>
> ---
>  package/ibrcommon/Config.in    | 13 +++++++++++++
>  package/ibrcommon/ibrcommon.mk | 32 ++++++++++++++++++++++++++++++++
>  package/ibrdtn/Config.in       | 13 +++++++++++++
>  package/ibrdtn/ibrdtn.mk       | 32 ++++++++++++++++++++++++++++++++
>  package/ibrdtnd/Config.in      | 23 +++++++++++++++++++++++
>  package/ibrdtnd/ibrdtnd.mk     | 32 ++++++++++++++++++++++++++++++++

As Vincente said, each package should be added in a separate patch.
Each patch should have a title like this:

	<package>: new package

so, your series should have three patches:

	ibrcommon: new package
	ibrdtn: new package
	ibrdtnd: new package

And indeed each patch should add the package in package/Config.in to
make it visible in menuconfig.

> diff --git a/package/ibrcommon/Config.in b/package/ibrcommon/Config.in
> new file mode 100644
> index 0000000..c1d400b
> --- /dev/null
> +++ b/package/ibrcommon/Config.in
> @@ -0,0 +1,13 @@
> +config BR2_PACKAGE_IBRCOMMON
> +	bool "ibrcommon"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_LIBNL
> +	select BR2_PACKAGE_LIBXML2
> +	help
> +	  IBR-DTN is a small dtn application that supports:
> +	  Bundle Protocol RFC 5050
> +	  Bundle Security Protocol RFC 6257
> +
> +	  http://trac.ibr.cs.tu-bs.de/project-cm-2012-ibrdtn

Please read the Buildroot manual about adding a comment mentioning the
toolchain dependencies, especially "17.2.4. Dependencies on target and
toolchain options".

> diff --git a/package/ibrcommon/ibrcommon.mk b/package/ibrcommon/ibrcommon.mk
> new file mode 100644
> index 0000000..3a67307
> --- /dev/null
> +++ b/package/ibrcommon/ibrcommon.mk
> @@ -0,0 +1,32 @@
> +################################################################################
> +#
> +# ibrcommon
> +#
> +################################################################################
> +
> +IBRCOMMON_VERSION = 1.0.1
> +IBRCOMMON_SOURCE = ibrcommon-$(IBRCOMMON_VERSION).tar.gz

Not needed, since it is the default value.

> +IBRCOMMON_SITE = https://www.ibr.cs.tu-bs.de/projects/ibr-dtn/releases/
> +IBRCOMMON_INSTALL_STAGING = YES
> +IBRCOMMON_LICENSE = Apache-2.0
> +IBRCOMMON_LICENSE_FILES = COPYING
> +IBRCOMMON_CONF_OPTS = --with-openssl --with-lowpan   --with-xml 
> +IBRCOMMON_DEPENDENCIES = openssl libnl libxml2
> +
> +# While the configuration system uses autoconf, the Makefiles are
> +# hand-written and do not use automake. Therefore, we have to hack
> +# around their deficiencies by passing installation paths.
> +IBRCOMMON_INSTALL_STAGING_OPTS = 			\
> +	prefix=$(STAGING_DIR)/usr 		\
> +	exec_prefix=$(STAGING_DIR)/usr 		\
> +	PKG_DEVLIB_DIR=$(STAGING_DIR)/usr/lib	\
> +	install
> +
> +IBRCOMMON_INSTALL_TARGET_OPTS = 			\
> +	prefix=$(TARGET_DIR)/usr 		\
> +	exec_prefix=$(TARGET_DIR)/usr 		\
> +	install
> +
> +
> +

Too many unneeded empty lines.

> +$(eval $(autotools-package))
> diff --git a/package/ibrdtn/Config.in b/package/ibrdtn/Config.in
> new file mode 100644
> index 0000000..9da2b54
> --- /dev/null
> +++ b/package/ibrdtn/Config.in
> @@ -0,0 +1,13 @@
> +config BR2_PACKAGE_IBRDTN
> +	bool "ibrdtn"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	select BR2_PACKAGE_IBRCOMMON
> +	help
> +	  IBR-DTN is a small dtn application that supports:
> +	  Bundle Protocol RFC 5050
> +	  Bundle Security Protocol RFC 6257
> +
> +	  http://trac.ibr.cs.tu-bs.de/project-cm-2012-ibrdtn
> +
> +

Too many unneeded new lines. Also, you're missing the Config.in comment
about toolchain dependencies.

> diff --git a/package/ibrdtn/ibrdtn.mk b/package/ibrdtn/ibrdtn.mk
> new file mode 100644
> index 0000000..744bd2f
> --- /dev/null
> +++ b/package/ibrdtn/ibrdtn.mk
> @@ -0,0 +1,32 @@
> +################################################################################
> +#
> +# ibrdtn
> +#
> +################################################################################
> +
> +IBRDTN_VERSION = 1.0.1
> +IBRDTN_SOURCE = ibrdtn-$(IBRDTN_VERSION).tar.gz

Unneeded, this is the default.

Same comments for the rest of the patch. Other than those basic
comments, it seems good.

Did you check if your packages work on noMMU platforms? For example, if
they use fork(), they will not work on noMMU platforms, and so you need
to add a BR2_USE_MMU dependency.

Thanks!

Thomas