Bug 162188

Comment 1 Edwin Groothuis 2011-10-31 09:30:39 UTC

Responsible Changed
From-To: freebsd-ports-bugs->mm

Submitter has GNATS access (via the GNATS Auto Assign Tool)

Comment 2 Edwin Groothuis 2011-10-31 09:30:44 UTC

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

Comment 3 Edwin Groothuis 2011-10-31 09:30:47 UTC

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.

Comment 6 Martin Matuska 2011-11-07 07:43:17 UTC

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

Comment 7 dfilter service 2011-11-09 09:15:42 UTC

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

Comment 9 Volker Werth 2011-11-10 08:22:34 UTC

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

Comment 10 Martin Matuska 2012-02-02 23:23:43 UTC

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

Comment 11 Philip M. Gollucci 2012-02-09 23:29:17 UTC

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?

Comment 13 Martin Matuska 2012-02-11 00:13:56 UTC

State Changed
From-To: open->closed

Closed on maintainer request. Thanks!