Bug 178644 - Redesign mail/postgrey RC script, other small tweaks
Summary: Redesign mail/postgrey RC script, other small tweaks
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-14 20:40 UTC by Rin Morningstar
Modified: 2013-05-25 16:00 UTC (History)
0 users

See Also:


Attachments
file.diff (3.64 KB, patch)
2013-05-14 20:40 UTC, Rin Morningstar
no flags Details | Diff
postgrey.diff (3.43 KB, patch)
2013-05-24 10:25 UTC, Guido Falsi
no flags Details | Diff
postgrey-1.34_5_2.diff.txt (4.34 KB, text/plain; charset=windows-1252)
2013-05-24 22:20 UTC, Rin Morningstar
no flags Details
postgrey.diff (5.29 KB, patch)
2013-05-24 23:21 UTC, Guido Falsi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rin Morningstar 2013-05-14 20:40:00 UTC
- Patch postgrey to use PGY_GROUPNAME by default;
- Make the DOCS option transparent (no dialog);
- Remove PGY_USERID and PGY_GROUPID (not used);
- PGY_GROUPNAME no longer follows PGY_USERNAME;
- Bump PORTREVISION.

After feedback from multiple users, I redesigned the RC script:

- Always prepend -d, --pidfile, and --dbdir options, even if you override
  postgrey_flags;
- The dbdir path is set using the postgrey_dbdir variable;
- Trust postgrey to use its defaults (dropped user, group, and
  x-greylist-header from the default postgrey_flags);
- Do not set a listening option (inet/unix) by default--user must set one
  explicitly via postgrey_flags before postgrey will run.

Fix: Update patch attached.

Patch attached with submission follows:
How-To-Repeat: n/a
Comment 1 Guido Falsi freebsd_committer 2013-05-22 17:24:58 UTC
Responsible Changed
From-To: freebsd-ports-bugs->madpilot

I'll take it.
Comment 2 Guido Falsi freebsd_committer 2013-05-24 10:25:07 UTC
Hi,

I have tweaked the rcscript a little more, following suggestions 
reported by rclint (devel/rclint)

I removed redundant quotes and an unneeded one line function.

BTW I am not sure shipping a not working rcscript is really correct, 
especially since a working one has been shipped till now.

This would break POLA (Principle Of Least Astonishment).

If doing this you really need a big warning perhaps using a pkg-message.

But I think displaying a notice using pkg-message telling people to 
check the documentation before trying to use the software and especially 
check the value of postgrey_flags ,which could have a working default, 
would be a much better option also not breaking the port unexpectedly 
for people who have been using it up till now.

Could you write a pkg-message to this effect for the port?

Attaching a patch for the second solution only missing the pkg-message.

Thanks.

-- 
Guido Falsi <madpilot@FreeBSD.org>
Comment 3 Rin Morningstar 2013-05-24 22:20:13 UTC
On 2013-05-24 02:25, Guido Falsi wrote:
> But I think displaying a notice using pkg-message telling people to
> check the documentation before trying to use the software and especially
> check the value of postgrey_flags ,which could have a working default,
> would be a much better option also not breaking the port unexpectedly
> for people who have been using it up till now.

I'd rather not set a --inet or --unix option by default, but I don't 
have a strong conviction about it.  I see POLA arguments in both 
directions.  I'll can leave the default in.

> Could you write a pkg-message to this effect for the port?
>
> Attaching a patch for the second solution only missing the pkg-message.

I incorporated the RC script changes, changed some RC script verbage and 
format, and added a pkg-message.  See attached.  You can drop the last 
line of the commit message.
Comment 4 Guido Falsi freebsd_committer 2013-05-24 23:21:37 UTC
Hi again.

After reading your pkg-message I have thought that adding an entry in 
UPDATING could be appropriate.

I have written such an entry based on the pkg-message you wrote. I have 
reworded your pkg-message to make it more generic, since it will 
accompany your port also for future versions, as long as the rc script 
will not change again. Please revise it and feel free to change anything.

I also noticed a pair of things I missed in your rc script:

why do you use extra_commands for reload? reload is provided by default. 
no need for and extra command for it.

I also have changed it to take advantage of command_args, so it can 
avoid changing postgrey_args on the fly.

Attaching a patch relative to the ports root, so you can also check the 
UPDATING entry.

Thanks!

-- 
Guido Falsi <madpilot@FreeBSD.org>
Comment 5 Rin Morningstar 2013-05-25 00:58:00 UTC
On 2013-05-24 15:21, Guido Falsi wrote:
> Attaching a patch relative to the ports root, so you can also check the
> UPDATING entry.

There's a typo in the UPDATING message, third para, first sentence. 
s/optionall/optional/

Otherwise, this all looks great!  Patch approved.
Comment 6 Guido Falsi freebsd_committer 2013-05-25 15:55:27 UTC
State Changed
From-To: open->closed

Committed. Thanks!
Comment 7 dfilter service freebsd_committer 2013-05-25 15:55:33 UTC
Author: madpilot
Date: Sat May 25 14:55:19 2013
New Revision: 319058
URL: http://svnweb.freebsd.org/changeset/ports/319058

Log:
  - Patch postgrey to use PGY_GROUPNAME by default
  - Don't show OPTIONS dialog for just DOCS option
  - Tidy up port Makefile
  - Rework rc script
  - Add UPDATING message and pkg-message to inform users about the
    rc script changes
  
  PR:		ports/178644
  Submitted by:	Darren Pilgrim <ports.maintainer@evilphi.com> (maintainer)

Added:
  head/mail/postgrey/pkg-message   (contents, props changed)
Modified:
  head/UPDATING
  head/mail/postgrey/Makefile
  head/mail/postgrey/files/pkg-install.in   (contents, props changed)
  head/mail/postgrey/files/postgrey.in   (contents, props changed)

Modified: head/UPDATING
==============================================================================
--- head/UPDATING	Sat May 25 14:41:37 2013	(r319057)
+++ head/UPDATING	Sat May 25 14:55:19 2013	(r319058)
@@ -6,6 +6,17 @@ You should get into the habit of checkin
 you update your ports collection, before attempting any port upgrades.
 
 20130525:
+  AFFECTS: users of mail/postgrey
+  AUTHOR: Darren Pilgrim <ports.maintainer@evilphi.com>
+
+  The RC script for postgrey has been modified. If you use the
+  default value for postgrey_flags this does not affect you.
+
+  If you have postgrey listening on a unix socket or set any optional
+  values, please read the comments in the RC scripts and check your
+  settings in rc.conf prior to restarting postgrey.
+
+20130525:
   AFFECTS: users of x11/xorg and all xorg ports
   AUTHOR zeising@FreeBSD.org
 

Modified: head/mail/postgrey/Makefile
==============================================================================
--- head/mail/postgrey/Makefile	Sat May 25 14:41:37 2013	(r319057)
+++ head/mail/postgrey/Makefile	Sat May 25 14:55:19 2013	(r319058)
@@ -3,7 +3,7 @@
 
 PORTNAME=	postgrey
 PORTVERSION=	1.34
-PORTREVISION=	4
+PORTREVISION=	5
 CATEGORIES=	mail
 MASTER_SITES=	http://postgrey.schweikert.ch/pub/ \
 		http://postgrey.schweikert.ch/pub/old/
@@ -23,14 +23,13 @@ NO_BUILD=	yes
 POD2MAN?=	pod2man
 PORTDOCS=	README Changes README.exim
 SUB_FILES=	pkg-install
-SUB_LIST=	USER=${PGY_USERNAME} UID=${PGY_USERID} GROUP=${PGY_GROUPNAME} \
-		GID=${PGY_GROUPID} ETCFILES="${ETCFILES}" \
+SUB_LIST=	USER=${PGY_USERNAME} \
+		GROUP=${PGY_GROUPNAME} \
+		ETCFILES="${ETCFILES}" \
 		POSTGREYDIR=${PGY_DIR}
 ETCFILES=	whitelist_clients whitelist_recipients
 PGY_USERNAME?=	postgrey
-PGY_USERID?=	225
-PGY_GROUPNAME?=	${PGY_USERNAME}
-PGY_GROUPID?=	${PGY_USERID}
+PGY_GROUPNAME?=	postgrey
 PGY_DIR?=	/var/db/postgrey
 
 USERS=		${PGY_USERNAME}
@@ -40,12 +39,10 @@ MAN1=		${PORTNAME}.1 policy-test.1 postg
 USES=		shebangfix
 SHEBANG_FILES=	${WRKSRC}/postgrey
 
-OPTIONS_DEFINE=	DOCS
-OPTIONS_DEFAULT=DOCS
-
 .include <bsd.port.options.mk>
 
 post-patch:
+	@${REINPLACE_CMD} -e 's#nogroup#${PGY_GROUPNAME}#' ${WRKSRC}/postgrey
 	@${REINPLACE_CMD} -e 's#/etc/main.cf#/etc/postfix/main.cf#' ${WRKSRC}/postgrey
 	@${REINPLACE_CMD} -e 's#/etc/postfix#${PREFIX}&#' ${WRKSRC}/postgrey ${WRKSRC}/postgrey_whitelist_*
 	@${REINPLACE_CMD} -e 's#/var/spool/postfix/postgrey#${PGY_DIR}#' ${WRKSRC}/postgrey ${WRKSRC}/contrib/postgreyreport

Modified: head/mail/postgrey/files/pkg-install.in
==============================================================================
--- head/mail/postgrey/files/pkg-install.in	Sat May 25 14:41:37 2013	(r319057)
+++ head/mail/postgrey/files/pkg-install.in	Sat May 25 14:55:19 2013	(r319058)
@@ -1,21 +1,16 @@
 #! /bin/sh
 #
-# $FreeBSD: /tmp/pcvs/ports/mail/postgrey/files/pkg-install.in,v 1.6 2012-04-18 07:54:30 crees Exp $
+# $FreeBSD$
 
 PATH=/bin:/usr/bin:/usr/sbin
 
 case $2 in
 
 PRE-INSTALL)
-  echo "---> Starting install script:"
-
-  if [ -z "%%POSTGREYDIR%%" -o \
-       -z "%%USER%%" -o -z "%%GROUP%%" -o \
-       -z "%%UID%%" -o -z "%%GID%%" ]; then
+  if [ -z "%%POSTGREYDIR%%" -o -z "%%USER%%" -o -z "%%GROUP%%" ]; then
     echo "ERROR: A required pragma was empty"
     exit 1
   fi
-
   ;;
 
 POST-INSTALL)

Modified: head/mail/postgrey/files/postgrey.in
==============================================================================
--- head/mail/postgrey/files/postgrey.in	Sat May 25 14:41:37 2013	(r319057)
+++ head/mail/postgrey/files/postgrey.in	Sat May 25 14:55:19 2013	(r319058)
@@ -7,41 +7,37 @@
 # BEFORE: mail
 # KEYWORD: shutdown
 
-#
 # Add the following lines to /etc/rc.conf to enable postgrey:
 #
-# postgrey_enable="YES"
+# postgrey_enable (bool)        Set to 'YES' to enable
+#                               Default: NO
+# postgrey_dbdir (path)         Location of postgrey database files.
+#                               Default: /var/db/postgrey
+# postgrey_flags (extra args)   Additional command-line parameters.
+#                               Default: --inet=10023
 #
-# See perldoc postgrey for flags
+# Note:
 #
+# postgrey_flags must include a --inet or --unix option or postgrey will
+# not run.  Change the --dbdir option with postgrey_dbdir.  Please see
+# the postgrey(1) man page or perldoc postgrey for more information.
 
 . /etc/rc.subr
 
 name=postgrey
-rcvar=postgrey_enable
-
-command=%%PREFIX%%/sbin/postgrey
-required_dirs=/var/db/postgrey
-extra_commands=reload
-
-stop_postcmd=stop_postcmd
 
-stop_postcmd()
-{
-  rm -f $pidfile
-}
+load_rc_config $name
 
-# set defaults
+: ${postgrey_enable:=NO}
+: ${postgrey_dbdir:=/var/db/postgrey}
+: ${postgrey_flags:=--inet=10023}
 
-load_rc_config $name
+command=%%PREFIX%%/sbin/postgrey
+pidfile=/var/run/postgrey.pid
+required_dirs=${postgrey_dbdir}
 
-postgrey_enable=${postgrey_enable:-"NO"}
-postgrey_greylist_header=${postgrey_greylist_header:-"X-Greylist: delayed %t seconds by postgrey-%v at %h\; %d"}
-postgrey_pidfile=${postgrey_pidfile:-"/var/run/postgrey.pid"}
-postgrey_flags=${postgrey_flags:-"--pidfile=${postgrey_pidfile} \
-	--inet=10023 -d --user=%%USER%% --group=%%GROUP%% --dbdir=/var/db/postgrey \
-	--x-greylist-header=${postgrey_greylist_header}"}
+command_args="-d --pidfile=${pidfile} --dbdir=${postgrey_dbdir}"
 
-pidfile="${postgrey_pidfile}"
+stop_postcmd="rm -f ${pidfile}"
 
 run_rc_command "$1"

Added: head/mail/postgrey/pkg-message
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/mail/postgrey/pkg-message	Sat May 25 14:55:19 2013	(r319058)
@@ -0,0 +1,7 @@
+
+ATTENTION
+
+The startup script for postgrey will make it listen on TCP port
+10023 by default. If you want to use a different setting, please
+read the comments in the RC script and set the appropriate settings
+via rc.conf before starting postgrey.
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"