Bug 254762

Summary: mail/opendkim: PID file
Product: Ports & Packages Reporter: Rob LA LAU <freebsd>
Component: Individual Port(s)Assignee: Chris Rees <crees>
Status: Closed FIXED    
Severity: Affects Some People CC: crees, dewayne, freebsd-ports
Priority: --- Flags: bugzilla: maintainer-feedback? (freebsd-ports)
Version: Latest   
Hardware: Any   
OS: Any   

Description Rob LA LAU 2021-04-04 14:05:21 UTC
The rc script for opendkim sets the variables _piddir and pidfile.
However, the configuration file opendkim.conf also contains a PidFile variable, which is now ignored.

If it is necessary to have the configuration variable overwritten by the rc script (?), a comment should be added to the PidFile variable in the configuration file, to indicate that this var has now become useless.

Also, a comment should be added to the BaseDirectory and Socket variables in the configuration file (and their default values modified), to indicate that a directory /var/run/milteropendkim is created by the rc script; this would allow a sysadmin to keep all runtime data in the same directory.
Comment 1 Chris Rees freebsd_committer freebsd_triage 2021-04-05 12:47:31 UTC
Please take a look at this, which respects PidFile being set if set in the config file:

https://reviews.freebsd.org/D29586

I haven't addressed the second comment part, but I think to be honest that /var/run/$name being created is so standard it doesn't really need special mention.

It will cause conflicts if PidFile is set to the same as another, so perhaps a check if the pidfile exists might be in order, would be easy to add to dkim_get_pidfile.
Comment 2 Rob LA LAU 2021-04-09 11:51:41 UTC
Looks good.

Problem with the /var/run/* directory is that you normally expect settings like those to just work.
However, currently, the opendkim.conf file points at /var/run/opendkim, while the rc script creates the directory /var/run/milteropendkim.

Cheers,
  Rob
Comment 3 Chris Rees freebsd_committer freebsd_triage 2021-04-09 14:21:51 UTC
The modified script creates the parent directory of $pidfile; ${pidfile%/*} removes the shortest suffix (i.e. is equivalent to $(dirname $pidfile)).
Comment 4 Rob LA LAU 2021-04-09 14:38:27 UTC
I'm sorry, you are right.
I had a look at it while I was doing other things.
Yes, this looks good.

Have a nice weekend,
  Rob
Comment 5 Chris Rees freebsd_committer freebsd_triage 2021-04-15 11:29:50 UTC
*** Bug 230685 has been marked as a duplicate of this bug. ***
Comment 6 Rob LA LAU 2021-11-05 22:55:18 UTC
Fixed, so close.
Comment 7 Chris Rees freebsd_committer freebsd_triage 2022-01-13 17:09:22 UTC
Not committed yet!
Comment 8 Chris Rees freebsd_committer freebsd_triage 2022-01-14 18:02:03 UTC
*Now* it's fixed :)  Thanks for reporting!
Comment 9 commit-hook freebsd_committer freebsd_triage 2022-01-14 18:02:10 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=0e46cdf18498118d3309812946e1df0245d62cb0

commit 0e46cdf18498118d3309812946e1df0245d62cb0
Author:     Chris Rees <crees@FreeBSD.org>
AuthorDate: 2022-01-14 17:56:38 +0000
Commit:     Chris Rees <crees@FreeBSD.org>
CommitDate: 2022-01-14 18:01:28 +0000

    mail/opendkim: Respect PidFile setting in opendkim.conf

    While here, pet rclint a bit, though it doesn't like the profile parts
    not being in functions

    PR:             ports/254762
    Reported by:    Rob LA LAU <freebsd@ohreally.nl>
    Reviewed by:    skozlov (a previous iteration)
    Approved by:    maintainer timeout, 9 months
    Differential Revision: https://reviews.freebsd.org/D29586

 mail/opendkim/Makefile                 |  2 +-
 mail/opendkim/files/milter-opendkim.in | 98 ++++++++++++++++------------------
 2 files changed, 48 insertions(+), 52 deletions(-)