Add variables to rc script so users don't need to cope with postgrey_flags: postgrey_listen - port or unix socket to listen on postgrey_privs - user and group to change to postgrey_dbdir - database directory Remove /var/db/postgrey from pkg-plist as this is a user-specified path Port maintainer (ports.maintainer@evilphi.com) is cc'd. Generated with FreeBSD Port Tools 0.99
Responsible Changed From-To: freebsd-ports-bugs->mm Submitter has GNATS access (via the GNATS Auto Assign Tool)
Maintainer of mail/postgrey, Please note that PR ports/162188 has just been submitted. If it contains a patch for an upgrade, an enhancement or a bug fix you agree on, reply to this email stating that you approve the patch and a committer will take care of it. The full text of the PR can be found at: http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/162188 -- Edwin Groothuis via the GNATS Auto Assign Tool edwin@FreeBSD.org
State Changed From-To: open->feedback Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
I'll address the pkg-plist and postgrey.in changes separately. pkg-plist The current pkg-plist is actually broken in the sense that /var/db/postgrey is specified by PGY_DIR in the Makefile and make use of in-place substitution to drop it into postgrey scripts. The pkg-plist should change to make pkg-plist to have the %%PGY_DIR%% placeholder that gets changed as well. The dbdir is only empty if postgrey was never run, so it should be removed if empty. postgrey.in The problem with enumerating more options is there are far more command-line options to postgrey than is reasonable to enumerate as rc options and only enumerating some would lead to confusion. Instead, just put everything in postgrey_flags. That way it's at least consistent with many other ports and base RC scripts. For example, in /etc/rc.conf.d/postgrey on my mail servers, I have: postgrey_enable="YES" postgrey_dbdir="/var/db/postgrey" postgrey_pidfile="${postgrey_dbdir}/postgrey.pid" postgrey_flags="-d \ --pidfile=${postgrey_pidfile} \ --unix=/var/spool/postfix/private/postgrey \ --user=postgrey \ --group=postgrey \ --dbdir=${postgrey_dbdir} \ --delay=60 \ --retry-window=10 \ --auto-whitelist-clients=10 \ --greylist-action='450' \ --greylist-text='4.7.1 greylisted.' " If we start splitting out options, we run into issues like above where I define two options that use the default value of postgrey_dbdir. I notice there also needs to be some reinplace magic done to this file as well. I'll do up a patch addressing these points, but it will likely be a week before it gets sent back as another follow-up.
Here's a more comprehensive overhaul of the port. Please include the following text in the commit message: Changes: - Added LICENSE support (postgrey is under GPLv2). - Make pkg-install, pkg-plist and the rc script use variables defined in the Makefile. - Start using our own ETCDIR instead of co-opting postfix. - Warn users about relocating the whitelists due to the ETCDIR move. - Set postgrey's default group (--group) to the port's value. - Added rc script postgrey_dbdir and postgrey_options variables to avoid duplication of defaults when adding command-line flags. - Remove redundant flags from the rc script postgrey_flags variable.
Hi, your overhaul contained several errors. - the pkg-plist was bad - %%ETCDIR%% in unexec must be %D/%%ETCDIR - the chown/chmod can be used only in POST-INSTALL phase - what if DBDIR exists but is not a directory? you have not checked this - the creation of default configs should be moved into pkg-plist - you should not use full paths in pkg-install (you have already defined PATH) In addition, as the variable can be specified by the user, PGY_DBDIR does not need to be read from environment IMO. I am attaching a rewrited version. If you are OK with it I can commit it. -- Martin Matuska FreeBSD committer http://blog.vx.sk
mm 2011-11-09 09:15:33 UTC FreeBSD ports repository Modified files: mail/postgrey Makefile pkg-plist mail/postgrey/files pkg-install.in postgrey.in Added files: mail/postgrey/files pkg-message.in Log: - Add LICENSE support (postgrey is under GPLv2). - Make pkg-install, pkg-plist and the rc script use variables defined in the Makefile. - Start using our own ETCDIR instead of co-opting postfix. - Warn users about relocating the whitelists due to the ETCDIR move. - Set postgrey's default group (--group) to the port's value. - Add rc script postgrey_dbdir and postgrey_options variables to avoid duplication of defaults when adding command-line flags. - Remove redundant flags from the rc script postgrey_flags variable. PR: ports/162188 Submitted by: Darren Pilgrim <ports.maintainer@evilphi.com> (maintainer) Reviewed by: myself Revision Changes Path 1.31 +22 -19 ports/mail/postgrey/Makefile 1.4 +17 -31 ports/mail/postgrey/files/pkg-install.in 1.1 +11 -0 ports/mail/postgrey/files/pkg-message.in (new) 1.6 +9 -7 ports/mail/postgrey/files/postgrey.in 1.9 +8 -6 ports/mail/postgrey/pkg-plist _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Hi, New rc script has incorrect default value of postgrey_options line which prevents postgrey to start up unless overwrite the value. Patch is very small and attached: --- mail/postgrey/files/postgrey.in.org 2011-11-10 11:45:35.000000000 +0900 +++ mail/postgrey/files/postgrey.in 2011-11-10 11:45:44.000000000 +0900 @@ -38,7 +38,7 @@ postgrey_dbdir=${postgrey_dbdir:-"%%DBDIR%%"} postgrey_enable=${postgrey_enable:-"NO"} -postgrey_options=${postgrey_options:-"--inet:10023"} +postgrey_options=${postgrey_options:-"--inet=10023"} postgrey_pidfile=${postgrey_pidfile:-"/var/run/postgrey.pid"} postgrey_flags=${postgrey_flags:-"-d --pidfile=${postgrey_pidfile} \ --dbdir=${postgrey_dbdir} ${postgrey_options}"} -- Takao
Dear Martin, I think your original issue for PR ports/162188 is fixed. The (new) problem of the bad rc file, introduced by the last commit to the postgrey port, not contained in your original patch, mentioned by YAMAMOTO Takao, is IMHO unrelated and so I've opened a new PR just for that problem as I was running into that problem after updating. It has been auto-assigned to me by gnats-aa but I don't hold a commit bit. I'm wondering if you are willing to take responsibility of my PR ports/162426 (I've already put that PR into the pool so anybody is free to grab). Please close PR ports/162188. Note to Yamamoto: Please open a new PR if you see new problems introduced instead of hijacking an older PR, thank you. All the best, Volker
There was no response or comment from the maintainer in two months. I am re-sending the patch with fixed inet=10023. If there are no objections in the next 14 days it will be considered approved. -- Martin Matuska FreeBSD committer http://blog.vx.sk
State Changed From-To: feedback->open Maintainer approved
On 2012-02-09 15:29, pgollucci@FreeBSD.org wrote: > Synopsis: [PATCH] mail/postgrey: add rc script options > > State-Changed-From-To: feedback->open > State-Changed-By: pgollucci > State-Changed-When: Thu Feb 9 23:29:17 UTC 2012 > State-Changed-Why: > Maintainer approved Huh? I haven't approved this. Also, I didn't get notified of the Feb 3 follow-up? I've tried emailing Martin revisions to my last patch, but they've gone unanswered. I figured he didn't care enough anymore to provide feedback and since this was at his request, I wasn't going to submit for more commits if the primary requestor couldn't be bothered. I did some digging and it looks like Martin's mail server is badly configured. So much so, in fact, that my content filters had mail.vx.sk on auto-junk status for repeatedly failing basic smell tests. Martin, please fix your mail server's DNS and send email such that you're not failing SPF. I'm happy to change the port for the better, but I can't get relevant feedback and testing if your email is broken. Perhaps freebsd.org has a mail server through which you can relay mm@freebsd.org instead of mail.vx.sk? Do you have a Gmail address by which I may contact you?
State Changed From-To: open->closed Closed on maintainer request. Thanks!