tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: wip/gnurl: Request for review

Hello N.,
Nice work, some possible comments/suggestions inline!
(I will try to test it soon, I have only reviewed it without build
testing it) writes:
> [...]
> gnurl ( is a microfork of curl I'm
> maintaining for gnunet as a gnunet developer.
> [...]

I think it is probably worth to add that informmation in DESCR and

Other possible suggestions directly inline (based on current

 | # $NetBSD$
 | [...]
 | CONFIGURE_ARGS+=	--with-ssl=${BUILDLINK_PREFIX.openssl}
 | CONFIGURE_ARGS+=	--with-ca-path=${SSLCERTS}

Are them copypastos from www/curl/Makefile? Isn't gnutls used
instead? (If that's the case I think it can be removed)

 | [...] 
 | # We want GnuTLS with Dane.
 | .include "../../security/gnutls/"
 | .include "../../security/gnutls/"
 | CONFIGURE_ARGS+= --with-gnutls
 | .include "../../devel/libidn2/"
 | [...] 

Minor nitpick: it is probably better to add all bl3 inclusion at
the end for consistency.

Other suggestions regarding

 | [...]
 | BUILDLINK_API_DEPENDS.gnurl+=	gnurl>=7.12.3
 | BUILDLINK_ABI_DEPENDS.gnurl+=	gnurl>=7.61.0nb2
 | BUILDLINK_PKGSRCDIR.gnurl?=	../../wip/gnurl
 | [...]

This is mostly a nitpick but I think that BUILDLINK_ABI_DEPENDS.gnurl
line can be removed and and BUILDLINK_API_DEPENDS.gnurl can be
bumped to `gnurl>=7.63.0' (7.63.0 - apart if locally packaged - is
the first gnurl version appeared in pkgsrc.).

 | [...]
 | .include "../../devel/gettext-lib/"
 | .include "../../devel/zlib/"
 | [...]

Please also include gnutls and libidn2 bl3 (they are included
unconditionally in the Makefile).

Thank you!

Home | Main Index | Thread Index | Old Index