Page MenuHomeFreeBSD

New port: mail/slimta: Configurable MTA based on the python-slimta libraries
ClosedPublic

Authored by nc on Sat, Jan 9, 12:03 AM.

Details

Summary

New port: mail/slimta: Configurable MTA based on the python-slimta libraries

Test Plan

Passes poudriere on i386 and amd64.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nc requested review of this revision.Sat, Jan 9, 12:03 AM
This comment was removed by nc.

Add a rc script and a pkg-message.in

Forgot a pkg-plist, since we create a new directory.

Remove files from another port update.

Sorry, had to do it again.

0mp requested changes to this revision.Mon, Jan 11, 6:35 PM
0mp added inline comments.
mail/slimta/Makefile
11 ↗(On Diff #82072)

Is LICENSE_FILE available perhaps?

26 ↗(On Diff #82072)
  1. It would probably be better if you patch occurrences of etc/ with %%PREFIX%% and then used REINPLACE_CMD to replace %%PREFIX%% with ${PREFIX}.
  2. You may want to use REINPLACE_ARGS= -i "". This way it might be easier to grep for it in the future.
30 ↗(On Diff #82072)

Would it also work?

BTW, if upstream offers a sample slimta.yaml file you can install it as a sample.

mail/slimta/files/pkg-message.in
4 ↗(On Diff #82072)

We usually try to avoid you in pkg-messages (although I cannot find any documentation codifying that),

mail/slimta/files/slimta.in
11 ↗(On Diff #82072)

Missing space.

33 ↗(On Diff #82072)

It could be that you'd be fine with:

procname="%%PREFIX%%/bin/slimta"
command="/usr/sbin/daemon"
command_args="-p $pidfile $procname -c $slimta_conf"
mail/slimta/pkg-plist
1 ↗(On Diff #82072)

Could you confirm that this is the what make makeplist generates? Also, if it's only one entry, then maybe PLIST_FILES= is a better mechanism in this case.

This revision now requires changes to proceed.Mon, Jan 11, 6:35 PM

Here's an updated diff.

Some highlights:

  • There is no license file, so LICENSE_FILE isn't used
  • pkg-message.in was removed, since there are sample configuration files
  • pkg-plist is used, since I'm installing the sample config files
  • Some cleanups in the rc script
nc marked 5 inline comments as done.Mon, Jan 11, 7:07 PM
nc added inline comments.
mail/slimta/Makefile
11 ↗(On Diff #82072)

There is no LICENSE_FILE for this port, sorry.

26 ↗(On Diff #82072)

Sure, done.

30 ↗(On Diff #82072)

Sure, done.

nc marked 3 inline comments as done.

Make it possible to stop slimta in the rc.d script.

Use @sample macros for config files

Hmm, it seems like both the slimta_start and the slimta_stop functions are simple enough that they could be removed altogether as their default implementations are probably good enough if not better. Am I missing something subtle?

Otherwise, the patch seems fine. We are almost ready to commit.

mail/slimta/Makefile
35 ↗(On Diff #82126)

${PREFIX}/etc/${PORTNAME} could probably be replaced with ${ETCDIR}

mail/slimta/files/slimta.in
38 ↗(On Diff #82126)

The rc service should support stopping the service if it is not enabled, e.g., when a user issues service slimta onestop.

41 ↗(On Diff #82126)

Would pkill -F ${pidfile} also work?

Here's an updated diff.

Some highlights:

  • I do need a slimta_stop(), but the default slimta_start() works fine
  • Using ${ETCDIR} would mean I would end up with etc/py37-slimta, which obviously can't be done
In D28052#630148, @nc wrote:

Here's an updated diff.

Some highlights:

  • I do need a slimta_stop(), but the default slimta_start() works fine

You might need to define command_interpreter. See rc.subr(8).

Let me know if that works.

  • Using ${ETCDIR} would mean I would end up with etc/py37-slimta, which obviously can't be done

Ach, good catch!

Use command_intepreter in rc script.

This revision is now accepted and ready to land.Sat, Jan 16, 9:19 PM