Hi,
On Wed, 30 Jan 2019 02:13:44 +0100
Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
> On Tue, Jan 29 2019, YASUOKA Masahiko <yasu...@yasuoka.net> wrote:
>> On Tue, 15 Jan 2019 07:19:54 +0100
>> Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
>>> On Fri, Dec 28 2018, YASUOKA Masahiko <yasu...@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).
I actually didn't plan to push it. But thank you for your comments.
> 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".
I see.
> [...]
>
>> 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.
Thanks,
>>> 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.
Ah, I missed that point. Skipping "doc" makes sense.
> And, obviously, we need to remove post-install from our Makefile, since
> that step tries to install the doxygen docs. ;)
You're right, I forgot to delete the block..
>> 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.
Thanks a lot. I checked the tarball.
ok to commit?
