Re: [NEW] devel/cjose

On Tue, Jan 29 2019, YASUOKA Masahiko <yasuoka@yasuoka.net> wrote:
> Hi,
>
> On Tue, 15 Jan 2019 07:19:54 +0100
> Jeremie Courreges-Anglas <jca@wxcvbn.org> wrote:
>> On Fri, Dec 28 2018, YASUOKA Masahiko <yasuoka@openbsd.org> wrote:
>>> Hi,
>>>
>>> I'd like to add devel/cjose. It's a C library of JOSE (JavaScript
>>> Object Signing and Encryption). JOSE is used in "OpenID connect"
>>> protocol.
>>>
>>> The original diff is written by my colleague, Naoaki Hoshi.
>>>
>>> ok?
>>
>> Fails to build on gcc archs, so not ok:
>> --8<--
>> cc1: warnings being treated as errors
>> In file included from include/jwk_int.h:8,
>> from jwk.c:8:
>> ../include/cjose/jwk.h:283: warning: type qualifiers ignored on function return type
>> jwk.c:1003: warning: type qualifiers ignored on function return type
>> Error while executing
>> cc -DPACKAGE_NAME="cjose" -DPACKAGE_TARNAME="cjose" -DPACKAGE_VERSION="0.6.1" -DPACKAGE_STRING="cjose
>> 0.6.1" -DPACKAGE_BUGREPORT="" -DPACKAGE_URL="" -DPACKAGE="cjose" -DVERSION="0.6.1" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=".libs/" -DHAVE_LIBCRYPTO=1 -DHAVE_LIBJANSSON=1 -I. -I/include -I/usr/local/include/ -std=gnu99 --pedantic -Wall -Werror -g -O2 -I../include -O2 -pipe -I/usr/local/include/ -MT
>> libcjose_la-jwk.lo -MD -MP -MF .deps/libcjose_la-jwk.Tpo -c
>> jwk.c -fPIC -DPIC -o
>> .libs/libcjose_la-jwk.o
>> -->8--
>>
>> Please drop the -Werror, it's a recipe for problems. The "-g -O2"
>> part should be dropped too, even if -O2 is overriden later ("-O2 -pipe").
>> A cheap way to do this is to patch src/Makefile.in and test/Makefile.in.
>
> Dropped the flags from src/Makefile.am and changed Makefile to do
> autoreconf.

Do you want to patch Makefile.am so that you can push the diff upstream?
If so, dropping "-g -O2" there makes sense because those are the
defaults used by autotools.

But then, you'll still have to convince upstream to also drop -Werror
from the default CFLAGS (because it's harmful on systems/architectures
they may not know about).

Anyway, if you want to patch Makefile.am instead of Makefile.in, you
need to declare a BUILD_DEPENDS on autoconf/automake. Something like

BUILD_DEPENDS = devel/check \
${MODGNU_AUTOCONF_DEPENDS} \
${MODGNU_AUTOMAKE_DEPENDS}

While here, use the new "do-gen" step instead of "pre-configure".

[...]

> Added some use cases.
>
> ***
> cjose is a library implementing Javascript Object Signing and Encryption
> (JOSE). JOSE consists of JSON Web Signature (JWS, RFC 7515), JSON Web
> Encryption (JWE, RFC 7516) and JSON Web Key (JWK, RFC 7517) and is used
> in some protocols (eg. OAUTH, OpenID Connect or XMPP).
> ***

thx, that's probably enough information for any user. I've tweaked the
end of it to reduce the number of parens.

>> Regarding the docs: doxygen updates can be painful, so mechanically
>> adding API docs is not a good approach. I would suggest dropping the
>> doxygen docs, unless of course if you really see value in them.
>
> ok. I dropped "docs" subpackage.

It's not that simple: if doxygen is present at configure time, it will
be used at "make all" time, but dpb can remove doxygen between those two
steps.

--disable-doxygen-docs should be the solution, but it isn't: doxygen is
still used at "make fake" time if doxygen is detected at configure time.
So I propose to just unhook the "doc" directory.

And, obviously, we need to remove post-install from our Makefile, since
that step tries to install the doxygen docs. ;)

> Also, "make test" didn't pass on Octeon. The upstream seems to fix
> this issue on master already. So I took the fix and the diff for
> another memory leak together, put them into patches/.

Hah, indeed. make test also passes on sparc64.

>> Except for these points, LGTM.
>
> Thanks. Let me update the tar.gz.

Here's an updated tarball with all the fixes mentioned above.