- Add missing mail/OpenSMTPD extras, ready and experimental
Details
Details
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
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.
Comment Actions
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.
Comment Actions
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.
Comment Actions
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
Comment Actions
Ahh okay gotcha. Well, looks like I'll be switching from pause to monkey once you commit this!
Thanks for the explanation, Dima.
| 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. |
Comment Actions
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.
| 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. |
Comment Actions
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.
Comment Actions
I have a few points:
- 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.
- Preparing slave ports to handle PORTREVISION is good, but see point #1 -> new differential review.
- 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.
- Moving USES=pkgconf from master to slave is, uhm.. surgical. IMHO it is common enough to keep it in master.
- 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...)
Comment Actions
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.
Comment Actions
- 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
- Preparing slave ports to handle PORTREVISION is good, but see point #1 -> new differential review.
- 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)
- 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 ?
- 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...
Comment Actions
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.
Comment Actions
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.
Comment Actions
Update diff after https://reviews.freebsd.org/D6607 was committed (https://svnweb.freebsd.org/changeset/ports/415954)
| 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. |
Comment Actions
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=)
Comment Actions
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}