Bug 7826 - Improve language around whitelist/blacklist and master/slave
Summary: Improve language around whitelist/blacklist and master/slave
Status: NEW
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamassassin (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Target Milestone: 4.0.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
: 7836 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-06-09 19:16 UTC by Kevin A. McGrail
Modified: 2020-12-30 23:58 UTC (History)
7 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
Unbreak allowlist patch None Giovanni Bechis [HasCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin A. McGrail 2020-06-09 19:16:25 UTC
The PMC has voted to support changing whitelist to allowlist and blacklist to blocklist as well as master to [arent and slave will be changed to child.

These changes also include the following consensus:

- will work in concert for 4.0 with deprecation warning so old configuration terms will be allowed as synonyms but will be marked as deprecated and not preferred in the documentation

- Old configuration terms will continue to work for no less than one year and will not be dropped until a version change such as 4.0 to 4.1.

NOTE: the choice of blocklist is done so that acronyms like RBL remain "as-is".
Comment 1 Bill Cole 2020-06-09 23:14:11 UTC
+1
Comment 2 AXB 2020-06-10 06:15:59 UTC
(In reply to Kevin A. McGrail from comment #0)
> and slave will be changed to child.

"killing child" sounds socially acceptable...
Comment 3 macosx10.4.11 2020-06-12 12:03:28 UTC
Is this a joke?
Comment 4 Bill Cole 2020-06-16 23:34:02 UTC
(In reply to macosx10.4.11 from comment #3)
> Is this a joke?

No.

Using colors as a good/bad metaphorical idiom is inaccurate for how SA uses such lists and has the added problem of colliding with the intractable idiomatic use of "Black" and "White" for arbitrary classes of people. Naming collisions are inherently bad, and we can't fix the larger side of this one. Switching to allow/block better reflects what SA does without requiring a subtle understanding of any English metaphorical idiom. 

The use of master/slave in SA is limited to tools in the build directory, which are of interest primarily to active members of the SA development, project sysadmin, and masscheck submitter communities. There *may* be some sites with independent automc and rule QA rigs, but there's no evidence of that. The usage includes scripts for Jenkins, which dropped 'slave' terminology in v2.0 (2016.) I am not entirely conversant with the automc & buildbot architecture where 'slave' is used, but a cursory examination leads me to believe that 'child' or perhaps 'worker' or 'robot' is a more apt metaphor for the tasks where 'slave' is currently used. 

In spamd, 'child' is already used consistently. 

(In reply to AXB  from comment #2)
> "killing child" sounds socially acceptable...

Moot point: there are no references in any SA code or comments to killing slaves. 

We already announce that we are "killing failed child" in lib/Mail/SpamAssassin/SpamdForkScaling.pm which does seem to fall short of the modern norms of good parenting. However, the standard terminology of the Unix/Linux/POSIX process model makes it unwieldy to avoid references to killing children.
Comment 5 Kevin A. McGrail 2020-07-02 22:18:53 UTC
First test is done with allowlist_to replacing whitelist_to
Committed revision 1879456.

NOTE: Not sure how active.list for rules will handle the new rules that are version based for this but the goal is that rules will work on older versions and 4.0.0 has an alias for whitelist_to so both configuration lines are equivalent.

Based on this Proof of Concept, will continue working on more changes.
Comment 6 Henrik Krohns 2020-07-03 09:00:20 UTC
Kevin you gonna break everyone running old trunk versions with the eval function name changes.

We should not check for "version 4.0.0", instead always use has_feature_xxx.
Comment 7 Henrik Krohns 2020-07-03 09:07:46 UTC
Atleast leave the old eval functions as compatibility layer. Just call the new ones from the old.
Comment 8 Kevin A. McGrail 2020-07-03 19:18:45 UTC
Henrik,

I considered that but with 4.0 unreleased, there is an expectation that trunk will be bleeding edge.  I want the future move to remove the backwards compatibility to be simple.

And since plugin WLBLEval will be changing to ALBLEval.pm, there is a level of breakage that is coming from this change with 4.0 that has_xxx and leaving old routines won't help.

I should give a heads-up to the list though for people running trunk that they might want to keep an eye on things on list.

KAM
Comment 9 AXB 2020-07-04 07:06:37 UTC
did this motion get the required PCM votes?
Comment 10 AXB 2020-07-04 07:07:33 UTC
(In reply to AXB from comment #9)
> did this motion get the required PCM votes?

I mean PMC
Comment 11 Henrik Krohns 2020-07-04 07:15:00 UTC
(In reply to AXB from comment #9)
> did this motion get the required PCM votes?

Yeah though atleast few votes were conditional on backwards compatibility..

I haven't seen any discussion agreeing on changing module names. Are we going to replace everything including autoWHITELIST, WHITELISTsubject, etc.. atleast do NOT commit these as random single commits over time. Do _all_ changes in a single commit so there's atleast some consistent revision where compatibility is broken.
Comment 12 Henrik Krohns 2020-07-04 07:16:40 UTC
Also care should be taken if renaming modules, as new "make install" will just leave the old ones lingering. Also nothing will change the old *.pre files to use the new modules! It requires careful planning to not break anything.
Comment 13 AXB 2020-07-04 07:55:29 UTC
" The PMC has voted to support changing whitelist to allowlist and blacklist to blocklist as well as master to [arent and slave will be changed to child."

Whre are all those votes?


Without keeping backward compatibilty for a decade, these changes should be immediately reversed.

Nobody can predict what the outcome will be if changes are done in batches.
Comment 14 AXB 2020-07-04 07:59:48 UTC
-1
Comment 15 Henrik Krohns 2020-07-04 08:00:48 UTC
I'd recommend a separate branch to test things.
Comment 16 AXB 2020-07-04 08:10:09 UTC
(In reply to Henrik Krohns from comment #15)
> I'd recommend a separate branch to test things.

I can relate to that.
Also, plans for such changes should be transparently shared with the SA user's list as these will be the ones affected.
Comment 17 AXB 2020-07-04 08:43:51 UTC
(In reply to Henrik Krohns from comment #6)
> Kevin you gonna break everyone running old trunk versions with the eval
> function name changes.
> 
> We should not check for "version 4.0.0", instead always use has_feature_xxx.

don't masscheckers use trunk?
Comment 18 Henrik Krohns 2020-07-04 08:47:14 UTC
(In reply to AXB from comment #17)
> 
> don't masscheckers use trunk?

masscheckers are supposed to automatically download latest revision matching the rules.. (atleast automasscheck script handles this)
Comment 19 Henrik Krohns 2020-07-05 05:27:02 UTC
Seems atleast last masscheck broke

Jul  5 04:40:24.987 [18525] warn: rules: failed to run __FROM_ADDRLIST_GOV test, skipping:
Jul  5 04:40:24.987 [18525] warn: \t(Can't locate object method "_check_whitelist" via package "Mail: [...]:SpamAssassin::Plugin::WLBLEval" at ../lib/Mail/SpamAssassin/Plugin/WLBLEval.pm line 119.
Jul  5 04:40:24.987 [18525] warn: )

So how bout we rollback the patch and start with a branch?
Comment 20 AXB 2020-07-05 05:31:14 UTC
(In reply to Henrik Krohns from comment #19)
> Seems atleast last masscheck broke

> So how bout we rollback the patch and start with a branch?

good early morning & 

+1
Comment 21 John Hardin 2020-07-05 06:20:16 UTC
(In reply to Henrik Krohns from comment #19)

> So how bout we rollback the patch and start with a branch?

+1

> Jul  5 04:40:24.987 [18525] warn: \t(Can't locate object method
> "_check_whitelist" via package "Mail: [...]:SpamAssassin::Plugin::WLBLEval"
> at ../lib/Mail/SpamAssassin/Plugin/WLBLEval.pm line 119.

Also, re terminology: we can retain "W" (as in "WLBLEval") if we adopt "W" = "Welcome": WelcomeList/BlockList.

That might help avoid some of the renaming.

Also, the internal method names should be retained for backwards compatibility. They are not generally visible to non-developers and may be in use by third-party plugins.
Comment 22 Kevin A. McGrail 2020-07-07 21:40:36 UTC
Hi All,

Since trunk is an unreleased branch, I am -1 to to switch to another branch.  Users will be carefully alerted via an UPGRADE file and release announcements for 4.0.

We did one patch to the system to prove it worked before we went through all the various functions one by one.  I wanted the patch to be bite size and reviewable.

To fix masscheck, a temporary stub for backwards compatibility until all the functions are done should work like this:

sub _check_whitelist {
  return _check_allowlist(@_);
}

I like the idea of welcome and block to replace white and black. I +1 that and can revert this change, switch from allowlist to welcomelist and add the stub if others concur.
Comment 23 Bill Cole 2020-07-07 22:04:49 UTC
(In reply to Kevin A. McGrail from comment #22)
 
> I like the idea of welcome and block to replace white and black. I +1 that
> and can revert this change, switch from allowlist to welcomelist and add the
> stub if others concur.

+1 for the acronym-preserving nomenclature.
 
+1 for moving forward on trunk with stubs as needed. The only way to get any exercise of new code is by putting it on trunk, as virtually no one will run the head of an explicit test branch.
Comment 24 AXB 2020-07-08 05:31:12 UTC
(In reply to Bill Cole from comment #23)
> (In reply to Kevin A. McGrail from comment #22)
>  
> > I like the idea of welcome and block to replace white and black. I +1 that
> > and can revert this change, switch from allowlist to welcomelist and add the
> > stub if others concur.
> 
> +1 for the acronym-preserving nomenclature.
>  
> +1 for moving forward on trunk with stubs as needed. The only way to get any
> exercise of new code is by putting it on trunk, as virtually no one will run
> the head of an explicit test branch.

As I don't understand the need for all these changes I will abstain from any 
further votes.
Comment 25 Henrik Krohns 2020-07-08 06:35:43 UTC
(In reply to Bill Cole from comment #23)
>  
> +1 for moving forward on trunk with stubs as needed. The only way to get any
> exercise of new code is by putting it on trunk, as virtually no one will run
> the head of an explicit test branch.

Branch will allow testing out the changes safely. Alternatively, don't commit unfinished and untested tryouts then. Trunk as a mass check branch, is not a test bed for these kinds of invasive changes.

If Kevin is doing this alone and disappearing for days every time masschecks and things break, it's not looking good for the project.
Comment 26 Henrik Krohns 2020-07-08 07:26:08 UTC
(In reply to Kevin A. McGrail from comment #22)
> 
> We did one patch to the system to prove it worked before we went through all
> the various functions one by one.  I wanted the patch to be bite size and
> reviewable.

Who is "we"? It was obvious within seconds from looking at the commit that it's going to break things. If you don't have time to properly review things yourself, commit a branch or post a patch, so others can review it without breaking things.

> To fix masscheck, a temporary stub for backwards compatibility until all the
> functions are done should work like this:
> 
> sub _check_whitelist {
>   return _check_allowlist(@_);
> }

The backwards compatibility should be for check_to_in_whitelist eval function, not _check_whitelist. And why should it be temporary?? Just leave it there! People can have local rules too which sa-update has no control over.
Comment 27 Giovanni Bechis 2020-07-08 14:01:48 UTC
Created attachment 5709 [details]
Unbreak allowlist

Quick fix to unbreak masscheck, more fixes needed to have good-looking code.
I think backwards compatibility should be a must due to custom rules that could depend on changed eval().
Comment 28 Loren Wilton 2020-07-08 16:36:21 UTC
Personally I consider this whole concept to be extremely ill-advised and strongly suggest that it be dropped. 

Has anyone ever considered bringing this change up on the SA users list and discussing it there? It seems that users should have some say in a matter that will affect most all of them, such as forcing virtually everyone with local rules to rewrite their rules. I can find no reference to it in the SA users list.

It's all well and good for some shadowy very small group of people to make overall policy decisions that will affect many without soliciting input from the people affected. But it isn't always the wisest choice.
Comment 29 Henrik Krohns 2020-07-09 07:20:47 UTC
*** Bug 7836 has been marked as a duplicate of this bug. ***
Comment 30 Henrik Krohns 2020-07-09 07:46:11 UTC
Sorry guys, I did not want to partake in this bug, but it seems the ball has been dropped completely.

Being PMC member and probably the most active developer, I use my right to emergency rollback the painful original commit.

Transmitting file data .......done
Committing transaction...
Committed revision 1879684.

I want to see SA work and prosper, but this is just embarassing. A week since faulty commit and all that happens is just some mumbling about it.

Now that the slate is clean, PLEASE create a branch for it. Count the votes or vote again if you want. But do not proceed commiting such individual one by one changes which make no sense.
Comment 31 jidanni 2020-07-09 12:06:28 UTC
Thanks for making Spamassassin work again.
(Whatever you guys do, remember to do a
https://en.wikipedia.org/wiki/Smoke_testing_(software)
when ever you alter the main tree.

Spamassassin is very frail: there is no simple "revert" SVN button I can
push here downstream once I swallow any changes.
My Spamassassin is "out of business" until someone upstream fixes it
and I can download a new version.)
Comment 32 Kevin A. McGrail 2020-07-10 03:15:31 UTC
Thank you for diving in.  I appreciate the attention this matter is getting because I think it is very important.  

Three notes:

Please remember with votes being made on this thread, you should be waiting 72 hours for people to weigh in. 

Please leave this ticket for me to work on.  I'm working on it and rolling things back is breaking a lot of work we are doing on two systems in the background.  

Trunk is unreleased and no rules are pending release so masscheck being broken is a null impact.

-KAM
Comment 33 Loren Wilton 2020-07-10 06:08:47 UTC
Kevin, did you notice bug 7836, linked as a duplicate of this?

          Reporter: jidanni@jidanni.org
  Target Milestone: Undefined

Updating spamassassin makes it unusable.

sa-update shows several

rules: failed to run __FROM_ADDRLIST_BANKS test, skipping:
        (Can't locate object method "_check_whitelist" via package
"Mail::SpamAssassin::Plugin::WLBLEval" at
/home/jidanni/.spamassassin-tree/share/perl/5.30.3/Mail/SpamAssassin/Plugin/WLBLEval.pm
line 119.
Comment 34 Henrik Krohns 2020-07-10 06:12:30 UTC
(In reply to Kevin A. McGrail from comment #32)
> 
> Please remember with votes being made on this thread, you should be waiting
> 72 hours for people to weigh in. 

People have weighted in.

> Please leave this ticket for me to work on.  I'm working on it and rolling
> things back is breaking a lot of work we are doing on two systems in the
> background.  

Again, who is "we"? Why would _trunk_ have any effect in your "two systems"? Use a branch as suggested, so no other change in trunk will bother your testings etc. And people will actually have something to weight in, before things break!!

> Trunk is unreleased and no rules are pending release so masscheck being
> broken is a null impact.

Uh huh. No.

From the way this bug started, I have little faith in the way you are going on about it. Is there even a plan?? Where is it? What things are you going to rename? What things are you going to break? "I'm going to rename WLBL to ALBL... uhh no it's not necessary to rename it..", how about actually posting the complete plan and waiting for people to weight in on all the renames? Or do as suggested and go on implementing the things in trunk so we have review and test the changes easily, instead of yet again more questions in mailing lists on why there are errors and why rule updates are not working etc.
Comment 35 Henrik Krohns 2020-07-10 06:14:21 UTC
(In reply to Henrik Krohns from comment #34)
> Or do as suggested and go on implementing the things in trunk

... meaning branch of course :-)
Comment 36 Kevin A. McGrail 2020-07-10 06:21:38 UTC
I just committed a second version of the proof of concept for whitelist_to:

svn commit -m 'changing whitelist_to to welcomelist_to including backward compatibility stubs and add a feature check instead of using a version test for rules.'
Sending        MANIFEST
Sending        lib/Mail/SpamAssassin/Conf.pm
Sending        lib/Mail/SpamAssassin/Plugin/WLBLEval.pm
Sending        rules/30_text_de.cf
Sending        rules/30_text_fr.cf
Sending        rules/30_text_pl.cf
Sending        rules/30_text_pt_br.cf
Sending        rules/50_scores.cf
Sending        rules/60_whitelist.cf
Sending        rules/active.list
Sending        rules/v343.pre
Sending        t/whitelist_to.t
Transmitting file data ............
Committed revision 1879735.

As noted in the commit log, this uses welcomelist, has stubs for compatibility and uses if can(Mail::SpamAssassin::Conf::feature_blocklist_welcomelist) for the rule encapsulation.

Using WelcomeList and BlockList will allow all acronyms like WLBLEval and RBL to remain the same.

As with before, if this proves to work, we will start on whitelist_from and move on to more routines until finished.
Comment 37 AXB 2020-07-10 06:35:38 UTC
(In reply to Kevin A. McGrail from comment #36)
> I just committed a second version of the proof of concept for whitelist_to:
> 
> svn commit -m 'changing whitelist_to to welcomelist_to including backward
> compatibility stubs and add a feature check instead of using a version test
> for rules.'
> Sending        MANIFEST
> Sending        lib/Mail/SpamAssassin/Conf.pm
> Sending        lib/Mail/SpamAssassin/Plugin/WLBLEval.pm
> Sending        rules/30_text_de.cf
> Sending        rules/30_text_fr.cf
> Sending        rules/30_text_pl.cf
> Sending        rules/30_text_pt_br.cf
> Sending        rules/50_scores.cf
> Sending        rules/60_whitelist.cf
> Sending        rules/active.list
> Sending        rules/v343.pre
> Sending        t/whitelist_to.t
> Transmitting file data ............
> Committed revision 1879735.
> 
> As noted in the commit log, this uses welcomelist, has stubs for
> compatibility and uses if
> can(Mail::SpamAssassin::Conf::feature_blocklist_welcomelist) for the rule
> encapsulation.
> 
> Using WelcomeList and BlockList will allow all acronyms like WLBLEval and
> RBL to remain the same.
> 
> As with before, if this proves to work, we will start on whitelist_from and
> move on to more routines until finished.

why is a POC put in trunk? - PLEASE, PLEASE put the playpen in a branch
Comment 38 Kevin A. McGrail 2020-07-10 07:12:39 UTC
Axb, please see my response on list re: branches but for those only reading the bug, we have no mechanism for testing ruleqa and masscheck on anything but trunk.

We will do our best to avoid problems but if you are running trunk, you are running bleeding edge development code and you should expect issues.  

I was talked out of creating a 4.0 branch however and I think now that should be reconsidered.  I am worried that this bug aside, we have a lot of code planned for 4.0 so I don't want to see people so upset.
Comment 39 AXB 2020-07-10 07:21:12 UTC
(In reply to Kevin A. McGrail from comment #38)

> I was talked out of creating a 4.0 branch however and I think now that
> should be reconsidered.  I am worried that this bug aside, we have a lot of
> code planned for 4.0 so I don't want to see people so upset.

Then start a branch for 5.0 or name it Milk&Coffee - just don't play around with trunk which is what third party developers, integrators, etc work with.

You can't expect the rest of the world to keep track & review what "you and your staff"  does in small batches.

It's not your project...  it's not mine... 

We owe respect to EVERYBODY and playing around with the code isn't cutting it.
Comment 40 Kevin A. McGrail 2020-07-10 07:37:15 UTC
This is a bug tracker not a mailing list.  Please move this onlist where there is already discussion about creating a 4.0 branch.
Comment 41 AXB 2020-07-10 07:42:59 UTC
(In reply to Kevin A. McGrail from comment #40)
> This is a bug tracker not a mailing list.  Please move this onlist where
> there is already discussion about creating a 4.0 branch.

I don't see a problem in having a disussion regarding such drastic changes being documented in the bug tracker. This is not our daily quota of blah.
Comment 42 Kevin A. McGrail 2020-07-10 10:08:12 UTC
Found two new files that I added to MANIFEST but not to svn add.  Committed.

svn commit -m 'Artifacts in MANIFEST but not in SVN for BZ 7826'
Adding         rules/v400.pre
Adding         t/welcomelist_to.t
Transmitting file data ..
Committed revision 1879755.

Plan to wait the weekend out on this task to make sure things look good before continuing with whitelist_from.
Comment 43 jidanni 2020-07-10 18:23:23 UTC
> we have no mechanism for testing ruleqa and masscheck on anything but trunk.
All I know is then make a copy of trunk and test on that.

And be sure to always make sure to run an email through a basic spam check to make sure nothing is broken, and if it is back out (revert changes) within five minutes. Thanks.
Comment 44 Kevin A. McGrail 2020-07-10 20:38:18 UTC
(In reply to jidanni from comment #43)
> > we have no mechanism for testing ruleqa and masscheck on anything but trunk.
> All I know is then make a copy of trunk and test on that.

Afraid it's not that simple.  See https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7837
Comment 45 Kevin A. McGrail 2020-07-12 21:41:06 UTC
Also see bug 7838 for a description on one of the new rules causing a warning.
Comment 46 John Hardin 2020-07-12 23:40:20 UTC
(In reply to Kevin A. McGrail from comment #0)
> - Old configuration terms will continue to work for no less than one year
> and will not be dropped until a version change such as 4.0 to 4.1.

We should also emit lint warnings for use of the old option names.
Comment 47 Kevin A. McGrail 2020-07-13 01:27:36 UTC
(In reply to John Hardin from comment #46)
> (In reply to Kevin A. McGrail from comment #0)
> > - Old configuration terms will continue to work for no less than one year
> > and will not be dropped until a version change such as 4.0 to 4.1.
> 
> We should also emit lint warnings for use of the old option names.

Good idea.  We are using an existing configuration option "aliases" not really any new configuration parsing.  Would need something like a aliases_warn option, I think.
Comment 48 John Hardin 2020-07-14 16:50:26 UTC
(In reply to Kevin A. McGrail from comment #0)

> - Old configuration terms will continue to work for no less than one year
> and will not be dropped until a version change such as 4.0 to 4.1.

I also want to make it clear I *strongly* suggest that the old terms continue to be supported as aliases *indefinitely*, absent a strong technical reason to end such support. And I feel "to keep the code clean" isn't sufficiently strong in this case.

Do we need a PMC vote on this detail?
Comment 49 Kevin A. McGrail 2020-07-14 17:18:40 UTC
(In reply to John Hardin from comment #48)
> Do we need a PMC vote on this detail?

Votes on a bug imply a blocker to this bug which I'm posting here to say I'm assuming that is NOT your intent.  Please post discussion/vote/results threads on the pmc list.  

IMO, yes, we will need a vote on ending the backwards compatibility but it will be way down the line when we start a 4.1 branch.  Voting on it now will just be lost to the sands of time and should be left for the future PMC at that time to decide.
Comment 50 John Hardin 2020-07-14 17:50:43 UTC
(In reply to Kevin A. McGrail from comment #49)
> (In reply to John Hardin from comment #48)
> > Do we need a PMC vote on this detail?
> 
> Votes on a bug imply a blocker to this bug which I'm posting here to say I'm
> assuming that is NOT your intent.  Please post discussion/vote/results
> threads on the pmc list.  
> 
> IMO, yes, we will need a vote on ending the backwards compatibility but it
> will be way down the line when we start a 4.1 branch.  Voting on it now will
> just be lost to the sands of time and should be left for the future PMC at
> that time to decide.

OK, that works.(In reply to Kevin A. McGrail from comment #49)
> (In reply to John Hardin from comment #48)
> > Do we need a PMC vote on this detail?
> 
> Votes on a bug imply a blocker to this bug which I'm posting here to say I'm
> assuming that is NOT your intent.  Please post discussion/vote/results
> threads on the pmc list.  
> 
> IMO, yes, we will need a vote on ending the backwards compatibility but it
> will be way down the line when we start a 4.1 branch.  Voting on it now will
> just be lost to the sands of time and should be left for the future PMC at
> that time to decide.

Not a blocker.

I was more thinking voting now to commit to and publicly declare making the support for the historical terminology permanent. This is primarily to quell objections on the list to forcing the change on users, regardless of how far down the road we kick that can.
Comment 51 Kevin A. McGrail 2020-07-24 18:11:43 UTC
Just an update that the rules for whitelist_to are working work for 3.3.1 to 3.4.X .  For 4.0, we'll keep updating the has feature function so those rules don't go active until 4.0 is released.  Working on whitelist_from.
Comment 52 Kevin A. McGrail 2020-08-01 04:41:13 UTC
Next commit lays more groundwork and changes whitelist_from to welcomelist_from.

svn commit -m 'changing whitelist_from to welcomelist_from including backward compatibility stubs and using feature check instead of using a version test for rules.  Also added a new plugin for allowing the RaciallyCharged language and added it disabled to v400.pre.  Logic and scoring added for the new plugin as well to the rules for welcomelist_from and welcomelist_to.'
Sending        MANIFEST
Sending        ldap/README
Sending        lib/Mail/SpamAssassin/Conf/Parser.pm
Sending        lib/Mail/SpamAssassin/Conf.pm
Sending        lib/Mail/SpamAssassin/PerMsgStatus.pm
Sending        lib/Mail/SpamAssassin/Plugin/DKIM.pm
Adding         lib/Mail/SpamAssassin/Plugin/RaciallyCharged.pm
Sending        lib/Mail/SpamAssassin/Plugin/SPF.pm
Sending        lib/Mail/SpamAssassin/Plugin/WLBLEval.pm
Sending        rules/50_scores.cf
Sending        rules/60_whitelist.cf
Sending        rules/user_prefs.template
Sending        rules/v400.pre
Sending        sql/README
Sending        t/data/01_test_rules.cf
Sending        t/data/01_test_rules.pre
Sending        t/debug.t
Adding         t/spamd_welcomelist_leak.t
Adding         t/welcomelist_from.t
Sending        t/welcomelist_to.t
Sending        t/whitelist_to.t
Transmitting file data .....................
Committed revision 1880494.
Comment 53 Loren Wilton 2020-08-01 04:57:13 UTC
I have to say that I strongly object to naming a standard plugin (or any standard visible interface) "RaciallyCharged". Clearly that is what the author perceives a lot of terms to be. But a whole lot of other people in a large part of the world don't see the terms as having anything to do with race, and will see this as deliberate political labeling of terms that, while perhaps not neutral, were not "racially" charged until deemed so by others. Will the next plugin be "MorallyCharged" when someone goes after some other term?

I'd MUCH prefer a more neutral and, dare I say it, "inclusive" term like "DepreciatedTerms". This can be used as a waste basket for all terms that people decide to not like, whether they consider them racial, moral, religious, traditional, or any other negative connotation.
Comment 54 Giovanni Bechis 2020-08-01 08:37:52 UTC
I would prefer a Deprecate.pm plugin, it could be useful in the future too if we would like to deprecate other keywords or plugins.
Comment 55 Kevin A. McGrail 2020-08-01 13:27:25 UTC
(In reply to Giovanni Bechis from comment #54)
> I would prefer a Deprecate.pm plugin, it could be useful in the future too
> if we would like to deprecate other keywords or plugins.

Have a look at the 60_whitelist.cf logic and the v400.pre.  This isn't a plugin to disable features (i.e. deprecate), it's to enable them.

By having the plugin, the old options work and this can extend to tests like spamd_whitelist_leak.t for 4.0 so we can ensure that the backwards compatibility with racially charged options works.

If you need a deprecate plugin, it would be trivial to add another plugin for that.  The current version on revision 1880494 is very simple.

I might be making it more complex though to put the extra linting concepts we want to add.  No digging has been done into that.  So this revision is noted on purpose because it's a very very simple plugin to add.
Comment 56 Kevin A. McGrail 2020-08-01 13:34:54 UTC
(In reply to Loren Wilton from comment #53)
> I have to say ...

This is a bug forum not a discussion forum.  Discussions belong on-list.
Comment 57 John Hardin 2020-08-01 17:22:28 UTC
(In reply to Kevin A. McGrail from comment #55)

> This isn't a plugin to disable features (i.e. deprecate), it's to enable them.

So name it "EnableDeprecatedTerminology.pm"

I second the objection to "RaciallyCharged.pm". Doing that is essentially scolding anyone who enables backwards compatibility features.
Comment 58 Kevin A. McGrail 2020-08-01 19:23:16 UTC
(In reply to John Hardin from comment #57)
> (In reply to Kevin A. McGrail from comment #55)
> 
> > This isn't a plugin to disable features (i.e. deprecate), it's to enable them.
> 
> So name it "EnableDeprecatedTerminology.pm"
> 
> I second the objection to "RaciallyCharged.pm". Doing that is essentially
> scolding anyone who enables backwards compatibility features.

The RaciallyCharged plugin doesn't enabled deprecated terminology, it enables the racially charged language. It's used in the rules specifically for this issue and the tests as well for backwards compatible.  It won't be suitable as a generic plugin.  The plugin describes exactly what the plugin does and I stand by the name.  People should stop using the language.
Comment 59 Kevin A. McGrail 2020-09-17 21:47:45 UTC
svn commit -m 'whitelist_from_rcvd to welcomelist_from_rcvd, unwhitelist_from_rcvd to unwelcomelist_from_rcvd, def_whitelist_from_rcvd to def_welcomelist_from_rcvd for bug 7826'
Sending        lib/Mail/SpamAssassin/Conf.pm
Sending        lib/Mail/SpamAssassin/Plugin/DKIM.pm
Sending        lib/Mail/SpamAssassin/Plugin/WLBLEval.pm
Sending        rules/60_whitelist.cf
Sending        t/spamd_welcomelist_leak.t
Sending        t/welcomelist_from.t
Transmitting file data ......
Committed revision 1881803.
Comment 60 Kevin A. McGrail 2020-12-29 21:07:21 UTC
svn commit -m 'whitelist_auth to welcomelist_auth, 
unwhitelist_auth to unwelcomelist_auth,
def_whitelist_auth to def_welcomelist_auth, 
check_forged_in_whitelist to check_forged_in_welcomelist, 
check_from_in_default_whitelist to check_from_in_default_welcomelist, 
check_uri_host_in_whitelist to check_uri_host_in_welcomelist, 
check_from_in_blacklist to check_from_in_blocklist,
check_to_in_blacklist to check_from_in_blocklist, 
check_to_in_blacklist to check_to_in_blocklist,
and whitelist_bounce_relays to welcomelist_bounce_relays for bug 7826'

Sending        MANIFEST
Sending        lib/Mail/SpamAssassin/Conf.pm
Sending        lib/Mail/SpamAssassin/Plugin/DKIM.pm
Sending        lib/Mail/SpamAssassin/Plugin/FreeMail.pm
Sending        lib/Mail/SpamAssassin/Plugin/SPF.pm
Sending        lib/Mail/SpamAssassin/Plugin/VBounce.pm
Sending        lib/Mail/SpamAssassin/Plugin/WLBLEval.pm
Sending        rules/20_vbounce.cf
Sending        rules/50_scores.cf
Sending        rules/60_whitelist.cf
Sending        rules/60_whitelist_auth.cf
Adding         t/blocklist_autolearn.t
Sending        t/data/01_test_rules.cf
Adding         t/freemail_welcome_block.t
Adding         t/spf_welcome_block.t
Transmitting file data ...............
Committed revision 1884922.
Comment 61 Kevin A. McGrail 2020-12-30 22:13:00 UTC
svn commit -m 'Found missing rule eval registration in VBounce.pm for check_whitelist_bounce_relays and changed 60_whitelist.cf hierarchy to handle ifplugin/if/else/endif problems'
Sending        lib/Mail/SpamAssassin/Plugin/VBounce.pm
Sending        rules/60_whitelist.cf
Transmitting file data ..done
Committing transaction...
Committed revision 1884961.
Comment 62 Kevin A. McGrail 2020-12-30 23:58:35 UTC
svn commit -m 'added logic for vbounce rules for 3.4 vs 4.0 with the new code changes and fixed two meta rule misspellings as well as cleaned up a lot of whitespace for bug 7826'
Sending        rules/20_vbounce.cf
Sending        rules/60_whitelist.cf
Transmitting file data ..done
Committing transaction...
Committed revision 1884976.