Page MenuHomeFreeBSD

Add missing mail/OpenSMTPD extras
ClosedPublic

Authored by fluffy on Apr 25 2016, 11:34 AM.

Details

Summary
  • Add missing mail/OpenSMTPD extras, ready and experimental

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

fluffy updated this revision to Diff 15570.Apr 25 2016, 11:34 AM
fluffy retitled this revision from to Add missing OpenSMTPS extras.
fluffy updated this object.
fluffy edited the test plan for this revision. (Show Details)
fluffy added reviewers: mat, miwi.
fluffy set the repository for this revision to rP FreeBSD ports repository.
fluffy updated this object.
fluffy retitled this revision from Add missing OpenSMTPS extras to Add missing OpenSMTPD extras.
fluffy added subscribers: mat, miwi, fluffy.
fluffy retitled this revision from Add missing OpenSMTPD extras to Add missing mail/OpenSMTPD extras.Apr 25 2016, 11:36 AM
fluffy updated this object.
miwi added a reviewer: adamw.May 16 2016, 3:45 AM
miwi edited edge metadata.

@adamw can you please have a look as well here.

Thanks

fluffy updated this revision to Diff 16921.May 26 2016, 4:58 PM
fluffy updated this object.
fluffy edited edge metadata.
fluffy changed the visibility from "Custom Policy" to "Public (No Login Required)".

Sync with current ports tree

fluffy added a reviewer: gahr.May 26 2016, 5:01 PM
fluffy added a subscriber: gahr.
adamw edited edge metadata.May 26 2016, 5:13 PM

filter-monkey is a testing module AFAICT and shouldn't be added.

Additionally, AFAIK the lua/perl/python modules are undocumented and not ready for public consumption yet.

In D6084#139028, @adamw wrote:

filter-monkey is a testing module AFAICT and shouldn't be added.

filter-monkey is well-documented and ready to use, provide more power to use than filter-pause

Additionally, AFAIK the lua/perl/python modules are undocumented and not ready for public consumption yet.

They marked as 'experimental' in port option and not turned on by default.

adamw added a comment.May 26 2016, 5:24 PM

I might be mis-understanding filter-monkey. Please correct me:

filter-pause is a spam mitigation technique.

filter-monkey appears to reject random mail with replies like "Not welcome", "Actually, no" and "Try harder!" Is it actually supposed to be used in production environments?

As for the lua/python/perl stuff, I missed that they were marked as experimental. With that, I'm behind you on those modules.

In D6084#139035, @adamw wrote:

I might be mis-understanding filter-monkey. Please correct me:
filter-pause is a spam mitigation technique.
filter-monkey appears to reject random mail with replies like "Not welcome", "Actually, no" and "Try harder!" Is it actually supposed to be used in production environments?

filter-pause uses a fixed delay on greeting stage.

filter-monkey can be configured to random delay (in defined ranges) on connect/helo/mail/rcpt/data/eom stages:
#delay 0:500 on connect
#delay 0:1000 on eom

random reject is not a main function of this filter but useful for stress-testing

adamw added a comment.May 26 2016, 5:40 PM

Ahh okay gotcha. Well, looks like I'll be switching from pause to monkey once you commit this!

Thanks for the explanation, Dima.

adamw added inline comments.May 26 2016, 5:45 PM
opensmtpd-extras-filter-regex/Makefile
12 ↗(On Diff #16921)

When the plist contains keywords, I'd really prefer to keep the plist as a separate file.

27.05.16 3:45, adamw (Adam Weinberger) пишет:

When the plist contains keywords, I'd really prefer to keep the plist as
a separate file.

Okay, will keep it.

adamw added inline comments.May 26 2016, 6:11 PM
opensmtpd-extras/Makefile
6 ↗(On Diff #16921)

Just one more thing, Dima. If you're manually setting PORTREVISION in the slave ports, can you add a reminder note here saying be sure to check for and remove PORTREVISION in slave ports?

It's the kind of thing I forget all the time.

27.05.16 4:11, adamw (Adam Weinberger) пишет:

Just one more thing, Dima. If you're manually setting PORTREVISION in
the slave ports, can you add a reminder note here saying be sure to
check for and remove PORTREVISION in slave ports?
It's the kind of thing I forget all the time.

As you can see, I prefer to always declare a PORTREVISION in the slave
ports, even with '0' value, to be sure that slave not automagically
bumped when some not-related (to slave) stuff come to master port (and
bump it's revision).

However, you're right, master port need a reminder about slaves'
PORTREVISION.

gahr requested changes to this revision.EditedMay 26 2016, 6:52 PM
gahr edited edge metadata.

I have a few points:

  1. Changes to existing ports have nothing to do with the topic of this review. Everything related to lines reordering in Makefiles and double-indentation after VARIABLE= should go in a new differential review.
  2. Preparing slave ports to handle PORTREVISION is good, but see point #1 -> new differential review.
  3. The change to CONFIGURE_ARGS in slave ports from append to set defies the idea that build-system specific flags could be set at master port level. I think it is wrong to change that.
  4. Moving USES=pkgconf from master to slave is, uhm.. surgical. IMHO it is common enough to keep it in master.
  5. I do *not* want to maintain these new slave ports, as I know next to nothing about them :)

Having said that, I'm more and more in favour of moving away from master/slave ports for opensmtpd-extras. That would mean killing mail/opensmtpd-extras and provide a Makefile.extras in mail/opensmtpd that extra ports can include. Perhaps we should do that before this whole thing gets too messy (see OPTIONS...)

This revision now requires changes to proceed.May 26 2016, 6:52 PM
adamw added a comment.May 26 2016, 7:04 PM
In D6084#139113, @gahr wrote:
  1. The change to CONFIGURE_ARGS in slave ports from append to set defies the idea that build-system specific flags could be set at master port level. I think it is wrong to change that.

I didn't catch those. I agree with gahr 100% here. Those were += for a reason. And with leaving the += in place, the indentation changes should be removed as well.

I'm not personally opposed to fluffy adding PORTREVISION=0 and correcting the spelling of "OpenSMTPD" as part of this commit.

Having said that, I'm more and more in favour of moving away from master/slave ports for opensmtpd-extras. That would mean killing mail/opensmtpd-extras and provide a Makefile.extras in mail/opensmtpd that extra ports can include. Perhaps we should do that before this whole thing gets too messy (see OPTIONS...)

I am 100% in favour of that.

gahr added a comment.May 26 2016, 7:11 PM
In D6084#139135, @adamw wrote:

I'm not personally opposed to fluffy adding PORTREVISION=0 and correcting the spelling of "OpenSMTPD" as part of this commit.

Ack.

  1. Changes to existing ports have nothing to do with the topic of this review. Everything related to lines reordering in Makefiles and double-indentation after VARIABLE= should go in a new differential review.

Will be splitted for two new patches here at Phabricator in next hours.
And restored double-identation to reduce diffs

  1. Preparing slave ports to handle PORTREVISION is good, but see point #1 -> new differential review.
  1. The change to CONFIGURE_ARGS in slave ports from append to set defies the idea that build-system specific flags could be set at master port level. I think it is wrong to change that.

Yep, my fault. += used again (and require master CONFIGURE_ARGS to fix
in-compiled default paths)

  1. Moving USES=pkgconf from master to slave is, uhm.. surgical. IMHO it is common enough to keep it in master.

Think, no. Only 6 of 20 subports require pkgconfig, so why to use
overhead' way ?

  1. I do *not* want to maintain these new slave ports, as I know next to nothing about them :)

I'll take new ports to myself. And may take all opensmtpd family ports
to one hands to keep continuity of updates, if anyone wish...

27.05.16 5:04, adamw (Adam Weinberger) пишет:

I didn't catch those. I agree with gahr 100% here. Those were += for a
reason. And with leaving the += in place, the indentation changes should
be removed as well.

BTW, we need to decide — use (or not) double identation in all subports
or leave a style mixtype as it is exist now?

I'm not personally opposed to fluffy adding PORTREVISION=0 and
correcting the spelling of "OpenSMTPD" as part of this commit.

fluffy updated this revision to Diff 16993.May 27 2016, 3:12 PM
fluffy updated this object.
fluffy edited edge metadata.
fluffy edited edge metadata.
adamw added inline comments.May 27 2016, 3:20 PM
opensmtpd-extras/Makefile
6 ↗(On Diff #16993)

Please do add a comment about checking/clearing PORTREVISION in the dependent ports.

17 ↗(On Diff #16993)

I thought some of the pre-existing filters needed pkgconfig? Are you sure it doesn't break anything by removing it here?

28.05.16 1:20, adamw (Adam Weinberger) пишет:

Please do add a comment about checking/clearing PORTREVISION in the
dependent ports.

Yep, will do

I thought some of the pre-existing filters needed pkgconfig? Are you
sure it doesn't break anything by removing it here?

https://reviews.freebsd.org/D6607 takes care of this. pkgconfig will be
hit only by 6 subports of 20.

fluffy updated this revision to Diff 17009.May 27 2016, 6:50 PM
fluffy updated this object.
fluffy edited edge metadata.
adamw added inline comments.Jun 1 2016, 3:38 PM
opensmtpd-extras/Makefile
11 ↗(On Diff #17009)

Can we sort this please?

18 ↗(On Diff #17009)

For consistency, OPTIONS_DEFAULT should contain ${OPTIONS_GROUP_TOOLS}, not TOOL_STATS.

30 ↗(On Diff #17009)

Can you change the text here for me? I figure if I make a separate commit it'll just make your life even harder trying to keep the patch up to date~

FILTER_DNSBL_DESC=      Check incoming senders against DNSBLs
33 ↗(On Diff #17009)

And again here...

FILTER_PAUSE_DESC=    Pause before SMTP greeting to reduce spam
100 ↗(On Diff #17009)

There's no need for b.p.o.mk here anymore; it can be removed.

adamw added a comment.Jun 1 2016, 3:43 PM

Ugh, I have no idea why it applied the comments to the wrong line numbers.

"Sort this please" is for opensmtpd-extras/Makefile:44 (OPTIONS_GROUP=)

fluffy added a comment.Jun 1 2016, 4:27 PM

02.06.16 1:43, adamw (Adam Weinberger) пишет:

Ugh, I have no idea why it applied the comments to the wrong line numbers.
"Sort this please" is for opensmtpd-extras/Makefile:44 (OPTIONS_GROUP=)

Current order was used to correct visual placing stable and experimental
groups in selection window

  • {F525315, layout=link}
adamw added a comment.Jun 1 2016, 4:31 PM

Current order was used to correct visual placing stable and experimental
groups in selection window

Then I believe it'll need NO_OPTIONS_SORT=yes.

fluffy updated this revision to Diff 17189.Jun 1 2016, 4:53 PM

style, typos and missed ldap line :)

adamw accepted this revision.Jun 1 2016, 5:27 PM
adamw edited edge metadata.

I haven't build tested any of it, but it looks great. You've got my vote.

gahr added a comment.Jun 1 2016, 7:20 PM

Sorry folks, haven't got the chance to look at this yet. Will review tomorrow.

fluffy added a comment.Jun 5 2016, 6:13 PM
In D6084#140921, @gahr wrote:

Sorry folks, haven't got the chance to look at this yet. Will review tomorrow.

Will disturb you with question: Any objections? :)

gahr accepted this revision.Jun 6 2016, 7:45 AM
gahr edited edge metadata.
In D6084#140921, @gahr wrote:

Sorry folks, haven't got the chance to look at this yet. Will review tomorrow.

Will disturb you with question: Any objections? :)

Apologies for the delay. No objections, go ahead!

This revision is now accepted and ready to land.Jun 6 2016, 7:45 AM
This revision was automatically updated to reflect the committed changes.