Re: [ptxdist] [PATCH] new package jansson

[Alexander Aring <alex.aring@gmail.com>]

Hi,

On Fri, Apr 04, 2014 at 01:13:31PM +0000, Gieseler, Christian wrote:
> Hi i have added jansson, a C library for encoding, decoding and manipulating of JSON data for our BSP.
> 
cool.

> I would like to share the new package, but I don`t know to do that exactly. Also the Section hast to be updated. Can someone guide me how to create a proper patch to make it acceptable for ptxdist. Which commands apart from diff do I have to use?

cool. Checkout current git version of ptxdist [0]. 

git clone git://git.pengutronix.de/git/ptxdist.git

or do you have a http proxy, like for me in my university check it out
via http:

git clone http://git.pengutronix.de/git/ptxdist.git

Then add your changes with git there and bake some patches and send it via
git send-email to this list. A nice tutorial can you see at [1].

I will comment now some of your changes, don't feel scared.

[0] http://git.pengutronix.de/?p=ptxdist.git;a=summary
[1] https://home.regit.org/technical-articles/git-for-the-newbie/

> 
> diff -upNr a/rules/ b/rules/ >patch results in the patch-file below: 
> 
> diff -upNr a/rules/jansson.in b/rules/jansson.in
> --- a/rules/jansson.in	1970-01-01 01:00:00.000000000 +0100
> +++ b/rules/jansson.in	2014-04-04 13:42:11.761735453 +0200
> @@ -0,0 +1,7 @@
> +## SECTION=project_specific
> +
You need to have a proper SECTION here. For me JSON is something like
XML and they reinvent the wheel. I looked in a mainline rules like cat
rules/libxml2.in (this is a well known xml C lib). It has the section:

## SECTION=system_libraries

You should also use that, because when somebody wants XML and see your
JSON rule, maybe he will use JSON then. :-)

> +config JANSSON
> +	tristate
> +	prompt "jansson"
> +	help
> +	  C-Library for encoding, decoding and manipulating JSON data

Seems to be okay, maybe a dot at end of the sentence.

> diff -upNr a/rules/jansson.make b/rules/jansson.make
> --- a/rules/jansson.make	1970-01-01 01:00:00.000000000 +0100
> +++ b/rules/jansson.make	2014-04-04 14:57:20.390288447 +0200
> @@ -0,0 +1,105 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2014 by Christian Gieseler <cg@eks-engel.de>
> +#
> +# See CREDITS for details about who has contributed to this project.
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +#
> +# We provide this package
> +#
> +PACKAGES-$(PTXCONF_JANSSON) += jansson
> +
> +#
> +# Paths and names
> +#
> +JANSSON_VERSION	:= 2.6
> +JANSSON_MD5	:= c70a52488db623a26f7213c7c6b7c878
> +JANSSON		:= jansson-$(JANSSON_VERSION)
> +JANSSON_SUFFIX	:= tar.bz2
> +JANSSON_URL	:= http://www.digip.org/jansson/releases/$(JANSSON).$(JANSSON_SUFFIX)
> +JANSSON_SOURCE	:= $(SRCDIR)/$(JANSSON).$(JANSSON_SUFFIX)
> +JANSSON_DIR	:= $(BUILDDIR)/lib$(JANSSON)
> +JANSSON_LICENSE	:= MIT
> +

Didn't check that but be sure if you have a tabwidth of 8 chars.

> +# ----------------------------------------------------------------------------
> +# Get
> +# ----------------------------------------------------------------------------
> +
> +#$(JANSSON_SOURCE):
> +#	@$(call targetinfo)
> +#	@$(call get, JANSSON)
> +
Remove this commented GET section.

> +# ----------------------------------------------------------------------------
> +# Prepare
> +# ----------------------------------------------------------------------------
> +
> +JANSSON_PATH	:= PATH=$(CROSS_PATH)
> +JANSSON_ENV	:= $(CROSS_ENV)
> +
You don't need that. You can drop this. But not remove the Prepare
headline, we usually leave it there because it could be change in the
future (I think that's the reason). But the get rule will be never
changed in the most case.
> +#
> +# autoconf
> +#
> +#JANSSON_AUTOCONF 	:= \
> +#			--host=arm-linux 
> +
Drop the JANSSON_AUTOCONF, think but leave the autoconf comment. I think
you tried here to set some configure options. But you can't hardcode
this here. Maybe somebody used a powerpc arch to build his software.
JANSSON_CONF_OPT     := $(CROSS_AUTOCONF_USR) should to the job.

> +JANSSON_CONF_TOOL	:= autoconf
> +JANSSON_CONF_OPT	:= $(CROSS_AUTOCONF_USR)
> +

You can set the configure options here with:

JANSSON_CONF_OPT     := \
	$(CROSS_AUTOCONF_USR) \
	--enable-foo \
	--disable-bar

I looked in the jansson dir and type ./configure --help, you can see all
possible options there.

I saw something like "--disable-windows-cryptoapi - Don't use CryptGenRandom to seed the hash function"
You should add this here, because we never compile windows here.

> +#$(STATEDIR)/jansson.prepare:
> +#	@$(call targetinfo)
> +#	@$(call clean, $(JANSSON_DIR)/config.cache)
> +#	cd $(JANSSON_DIR) && \
> +#		$(JANSSON_PATH) $(JANSSON_ENV) \
> +#		./configure $(JANSSON_CONF_OPT)
> +#	@$(call touch)
> +
Drop this commented stage here.

> +# ----------------------------------------------------------------------------
> +# Compile
> +# ----------------------------------------------------------------------------
> +
> +#$(STATEDIR)/jansson.compile:
> +#	@$(call targetinfo)
> +#	@$(call world/compile, JANSSON)
> +#	@$(call touch)
> +
Drop this commented stage with section comments here.
> +# ----------------------------------------------------------------------------
> +# Install
> +# ----------------------------------------------------------------------------
> +
> +#$(STATEDIR)/jansson.install:
> +#	@$(call targetinfo)
> +#	@$(call world/install, JANSSON)
> +#	@$(call touch)
> +
Drop this commented stage with section comments here.
> +# ----------------------------------------------------------------------------
> +# Target-Install
> +# ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/jansson.targetinstall:
> +	@$(call targetinfo)
> +
> +	@$(call install_init, jansson)
> +	@$(call install_fixup, jansson,PRIORITY,optional)
> +	@$(call install_fixup, jansson,SECTION,base)
> +	@$(call install_fixup, jansson,AUTHOR,"Christian Gieseler <cg@eks-engel.de>")
> +	@$(call install_fixup, jansson,DESCRIPTION,missing)
> +
> +	@$(call install_copy, jansson, 0, 0, 0755, $(JANSSON_DIR)/src/.libs/libjansson.so.4, /lib/libjansson.so.4)
yea that's maybe working but you did set a wrong permission 755 and
installed in /lib instead /usr/lib/ and normally you installed it direkt
from $(JANSSON_DIR), there is some PKGDIR which should installed from...

But the big very big big think is that /lib/libjansson.so.4 should be a
symlink :D. You have normally some libjansson.so and libjansson.so.4 are
symlink to this to do some versioning.

But the good news, you don't need install_copy here. PTXdist has his own
install_lib here. Use:

@$(call install_lib, jansson, 0, 0, 0644, libjansson)

that should do the job!

> +
> +	@$(call install_finish, jansson)
> +
> +	@$(call touch)
> +
> +# ----------------------------------------------------------------------------
> +# Clean
> +# ----------------------------------------------------------------------------
> +
> +#$(STATEDIR)/jansson.clean:
> +#	@$(call targetinfo)
> +#	@$(call clean_pkg, JANSSON)
Drop this commented stage with section comments here.
> +
> +# vim: syntax=make
> 

- Alex

-- 
ptxdist mailing list
ptxdist@pengutronix.de