svn commit: r382819 - in head/www: . e2guardian

Alexey Dokuchaev danfe at FreeBSD.org
Tue Mar 31 14:40:34 UTC 2015


On Tue, Mar 31, 2015 at 02:13:40PM +0000, Renato Botelho wrote:
> New Revision: 382819
> URL: https://svnweb.freebsd.org/changeset/ports/382819
> 
> [...]
>   Submitted by:	Marcello Coutinho <marcellocoutinho at gmail.com>
> 
> @@ -0,0 +1,99 @@
> +# Created by: Marcello Coutinho

Would be nice to include creator's email in header as well, at least for
consistency with majority of existing ports.

> +MASTER_SITES=	GH

This line is redundant; in USE_GITHUB case, it will be added by bsd.sites.mk
automatically (see the code around line 533).

> +USE_GITHUB=	yes
> +GH_TAGNAME=	${PORTVERSION:S/^/v/}

Why in-var substitution instead of simple and more readable v${PORTVERSION}?

> +USES=		pkgconfig iconv

Mirordered USES (minor nit).

> +HAS_CONFIGURE=	yes

HAS_CONFIGURE for GNU autoconf-based build system?  Doesn't look right.

> +USE_AUTOTOOLS=	aclocal libtoolize autoheader automake autoconf
> +ACLOCAL_ARGS=	-I m4
> +AUTOMAKE_ARGS=	--add-missing --copy --foreign

Did you consider using USES=autoreconf instead of USE_AUTOTOOLS et al.?

> +OPTIONS_DEFAULT=TRICKLE DOCS 1024

DOCS should not be part of OPTIONS_DEFAULT.

> +APACHE_DESC=	Enable Apache support for access denied page
> +TRICKLE_DESC=	Enable the trickle download manager
> +CLISCAN_DESC=	Enable support for CLI content scanners
> +CLAMD_DESC=	Enable ClamD AV content scanner
> +ICAP_DESC=	Enable ICAP AV content scanner support
> +KAV_DESC=	Enable Kaspersky AV support
> +NTLM_DESC=	Include NTLM authentication plugin
> +DNS_DESC=	Include DNS authetication plugin
> +EMAIL_DESC=	Enable e-mail reporting support

We normally do not prepend the word "Enable" to option descriptions.
See file Mk/bsd.options.desc.mk for examples of well-named options.

> +OPTIONS_RADIO=	DESCRIPTORS
> +OPTIONS_RADIO_DESCRIPTORS=	1024 2048 4096 8192

Description for DESCRIPTORS (DESCRIPTORS_DESC) is missing.

> +pre-configure:

In-place patching is more naturally done in `post-patch' target.

> +	@${REINPLACE_CMD} -e 's|.lresolv||g' \
> +		${WRKSRC}/configure.ac

I'm puzzled over this 's|.lresolv||g' part.  This is the resulting diff I
see:

@@ -788,7 +788,7 @@
        AC_MSG_RESULT(yes)
        dnsauth=true
        DNSAUTHSUPPORT=""
-       CXXFLAGS="${CXXFLAGS} -lresolv"
+       CXXFLAGS="${CXXFLAGS} "
        AC_DEFINE([PRT_DNSAUTH],[],[Define to enable DNS auth plugin])
 fi],
 [

Can you elaborate on why 's|.lresolv||g' was used to achieve this?  I also
do not see the need for global modifier (g).  Notice bogus trailing space
before closing quote.

./danfe


More information about the svn-ports-head mailing list