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)

ng0%n0.is@localhost writes:
> [...]
> gnurl (https://gnunet.org/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
COMMENT too.

Other possible suggestions directly inline (based on current
wip/gnurl/Makefile):

 | # $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/buildlink3.mk"
 | .include "../../security/gnutls/libgnutls-config.mk"
 | CONFIGURE_ARGS+= --with-gnutls
 | 
 | .include "../../devel/libidn2/buildlink3.mk"
 | [...] 

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

Other suggestions regarding buildlink3.mk:

 | [...]
 | 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/buildlink3.mk"
 | .include "../../devel/zlib/buildlink3.mk"
 | [...]

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


Thank you!


Home | Main Index | Thread Index | Old Index