Subject: Re: spamd pkgsrc issues
To: Jeremy C. Reed <reed@reedmedia.net>
From: Johnny Lam <jlam@pkgsrc.org>
List: pkgsrc-users
Date: 03/30/2006 10:22:25
Jeremy C. Reed wrote:
> 
> +CPPFLAGS+=		-DSPAMD_HOME=\"${VARBASE}/chroot/spamd\"
>  OWN_DIRS=		${VARBASE}/chroot/spamd
>  RCD_SCRIPTS=            pfspamd
>  
>  CONF_FILES=		${PREFIX}/share/examples/spamd/spamd.conf \
>  			    ${PKG_SYSCONFDIR}/spamd.conf
>  
> +# PATH_SPAMD_CONF             "/usr/pkg/etc/spamd.conf"
> +
> +
>  SUBST_CLASSES+=		fix
>  SUBST_STAGE.fix=	post-patch
>  SUBST_FILES.fix=	spamd/spamd.8

I would rather see this sed goop go away and replaced by a real patch 
that can be sent upstream to the spamd folks.

> @@ -50,4 +55,13 @@
>  	${INSTALL_DATA} ${WRKSRC}/etc/spamd.conf ${PREFIX}/share/examples/spamd
>  
>  .include "../../security/pflkm/buildlink3.mk"
> +
> +# Necessary so USE_BUILTIN.pflkm will be defined
> +.include "../../security/pflkm/builtin.mk"
> +
> +# Using LOCALBASE instead of PREFIX, since not defined yet
> +.if !empty(USE_BUILTIN.pflkm:M[Nn][Oo]) && exists(${LOCALBASE}/sbin/pfctl)
> +CPPFLAGS+=	-DPATH_PFCTL=\"${LOCALBASE}/sbin/pfctl\"
> +.endif

This dance is not necessary.  You should remove everything after you 
include pflkm/buildlink3.mk and just say:

CPPFLAGS+=  -DPATH_PFCTL=\"${BUILDLINK_PREFIX.pflkm}/sbin/pfctl\"

> --- spamd/spamd.c.orig	2006-03-29 23:35:44.000000000 -0800
> +++ spamd/spamd.c	2006-03-29 23:41:51.000000000 -0800
> @@ -1162,8 +1162,8 @@
>  	}
>  
>  jail:
> -	if (chroot("/var/empty") == -1 || chdir("/") == -1) {
> -		syslog(LOG_ERR, "cannot chdir to /var/empty.");
> +	if (chroot(SPAMD_HOME) == -1 || chdir("/") == -1) {
> +		syslog(LOG_ERR, "cannot chdir to " SPAMD_HOME ".");
>  		exit(1);
>  	}

I think we should rename this to PATH_SPAMD_HOME to be consistent with 
how the other macros are named.  We should add this to the top of spamd.c:

#ifndef PATH_SPAMD_HOME
#define PATH_SPAMD_HOME		"/var/empty"
#endif

And then just use PATH_SPAMD_HOME in the .c file.

> --- spamd-setup/spamd-setup.c.orig	2006-03-29 23:55:11.000000000 -0800
> +++ spamd-setup/spamd-setup.c	2006-03-29 23:56:12.000000000 -0800
> @@ -42,8 +42,12 @@
>  #include <zlib.h>
>  
>  #define PATH_FTP		"/usr/bin/ftp"
> +#ifndef PATH_PFCTL
>  #define PATH_PFCTL		"/sbin/pfctl"
> +#endif
> +#ifndef PATH_SPAMD_CONF
>  #define PATH_SPAMD_CONF		"/usr/pkg/etc/spamd.conf"
> +#endif
>  #define SPAMD_ARG_MAX		256 /* max # of args to an exec */

This patch looks like it was generated after the SUBST_SED stuff was 
run.  I doubt this applies cleanly to the extracted sources, but I agree 
with the intent of your changes.

> --- spamd/grey.h.orig	2006-03-29 23:55:23.000000000 -0800
> +++ spamd/grey.h	2006-03-29 23:56:36.000000000 -0800
> @@ -21,7 +21,9 @@
>  #define GREYEXP (60 * 60 * 4) /* remove grey entries after 4 hours */
>  #define WHITEEXP (60 * 60 * 24 * 36) /* remove white entries after 36 days */
>  #define TRAPEXP (60 * 60 * 24) /* hitting a spamtrap blacklists for a day */
> +#ifndef PATH_PFCTL
>  #define PATH_PFCTL "/sbin/pfctl"
> +#endif
>  #define DB_SCAN_INTERVAL 60
>  #define PATH_SPAMD_DB "/var/db/spamd"

I think if we are going down the route you're taking, then you should 
also do an ifndef wrapper around PATH_SPAMD_DB so that we can point it 
to ${VARBASE} in the package Makefile.

	Cheers,

	-- Johnny Lam <jlam@pkgsrc.org>