You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by he...@apache.org on 2022/04/29 16:26:57 UTC

svn commit: r1900390 - /spamassassin/trunk/lib/Mail/SpamAssassin.pm

Author: hege
Date: Fri Apr 29 16:26:57 2022
New Revision: 1900390

URL: http://svn.apache.org/viewvc?rev=1900390&view=rev
Log:
Purge write testfiles only sometimes, remember to use catdir

Modified:
    spamassassin/trunk/lib/Mail/SpamAssassin.pm

Modified: spamassassin/trunk/lib/Mail/SpamAssassin.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin.pm?rev=1900390&r1=1900389&r2=1900390&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin.pm Fri Apr 29 16:26:57 2022
@@ -1997,9 +1997,10 @@ sub set_global_state_dir {
 sub test_global_state_dir {
     my ($self, $dir) = @_;
     eval { mkpath($dir, 0, 0700); }; # just a single stat if exists already
-    # Purge stale test files
-    if (opendir(WT_DIR, $dir)) {
-      foreach (grep {/^\.sawritetest/ && (-M "$dir/$_"||0) > 0.0001} readdir(WT_DIR)) {
+    # Purge stale test files (enough to do only some times randomly)
+    if (rand() < 0.2 && opendir(WT_DIR, $dir)) {
+      foreach (grep {index($_, '.sawritetest') == 0 &&
+                 (-M File::Spec->catdir($dir, $_)||0) > 0.0001} readdir(WT_DIR)) {
         unlink(Mail::SpamAssassin::Util::untaint_file_path(File::Spec->catdir($dir, $_)));
       }
       closedir WT_DIR;



Re: File::Spec?

Posted by Riccardo Alfieri <ri...@spamteq.com>.
On 29/04/22 20:22, Henrik K wrote:

>   If someone actually still uses SA on Windows,
> hopefully they will test and report if there are things that don't work.
> :-)

Just FYI, MDaemon (https://www.altn.com/) uses SpamAssassin, version 
3.4.4 in their latest version. I don't know their market penetration but 
it's a product that has been using SA for decades. It may be not nice to 
break Windows compatibility in the next version :)

-- 
Best regards,
Riccardo Alfieri

Spamhaus Technology
https://www.spamhaustech.com/


Re: File::Spec?

Posted by Henrik K <he...@hege.li>.
On Fri, Apr 29, 2022 at 11:02:20AM -0700, Loren Wilton wrote:
> Windows NT (that  is, any kind of current Windows) file functions will
> natively accept either \ or / equivalently in file paths. There is an option
> to disable the acceptance of /, but almost nobody knows it exists, and I
> can't imagine anyone setting it on a rational system.

Cool, learned something new..

This was just some random pondering anyway, certainly I won't waste time on
changing lots of stuff that have no effect on functionality..  let's focus
on getting 4.0.0 out.  If someone actually still uses SA on Windows,
hopefully they will test and report if there are things that don't work. 
:-)


Re: File::Spec?

Posted by Loren Wilton <lw...@earthlink.net>.
Windows NT (that  is, any kind of current Windows) file functions will 
natively accept either \ or / equivalently in file paths. There is an option 
to disable the acceptance of /, but almost nobody knows it exists, and I 
can't imagine anyone setting it on a rational system.

The main difference I can think of is "<drive>:", which so far as I know 
doesn't exist on Linux derived systems. I'm not sure what the native (to 
perl) file calls with do with "C:\foo\bar" if it shows up. If that works, I 
suspect almost everything should be fine.

        Loren


Re: File::Spec?

Posted by Henrik K <he...@hege.li>.
I actually had Strawberry Perl on my Windows..

Though printing with catdir results in native backslashes, all the file
functions happily accept paths even in Unix style anyway.

C:\>perl -e "use File::Spec; print File::Spec->catdir('temp','test.txt')"
temp\test.txt

C:\>perl -e "use File::Spec; print File::Spec->catdir('C:','temp','test.txt')"
C:\temp\test.txt

C:\>perl -e "open(FOO, '/temp/test.txt') or die; print <FOO>"
foobar

C:\>perl -e "open(FOO, '\temp\test.txt') or die; print <FOO>"
foobar

C:\>perl -e "open(FOO, 'C:\temp\test.txt') or die; print <FOO>"
foobar

So kinda seems pointless trying to cater for the native way..


On Fri, Apr 29, 2022 at 07:46:50PM +0300, Henrik K wrote:
> 
> Ok it seems I'm going down the path of just doing stuff for the sake of
> "that's the way it's always been done"..
> 
> Does someone actually know why File::Spec->catdir (and catfile, which seems
> to work completely identically anyway) is used all over SpamAssassin?  Was
> it for some legacy Windows compatibility?  Does any Perl on Windows actually
> use native C:\Foo\Bar style paths that would require this method?  Does SA
> 4.0 even have any promise to work on Windows, and at what capacity? 
> Strawberry Perl?  Activestate Perl?  Cygwin?  WSL?  Why not just simplify
> code and ditch all the complicated to look and read File::Spec stuff..  you
> can also find many many examples where it is not used..  either it should be
> used 100% everywhere, or then there is no actual portability.
> 
> 
> On Fri, Apr 29, 2022 at 04:26:57PM -0000, hege@apache.org wrote:
> > Author: hege
> > Date: Fri Apr 29 16:26:57 2022
> > New Revision: 1900390
> > 
> > URL: http://svn.apache.org/viewvc?rev=1900390&view=rev
> > Log:
> > Purge write testfiles only sometimes, remember to use catdir
> > 
> > Modified:
> >     spamassassin/trunk/lib/Mail/SpamAssassin.pm
> > 
> > Modified: spamassassin/trunk/lib/Mail/SpamAssassin.pm
> > URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin.pm?rev=1900390&r1=1900389&r2=1900390&view=diff
> > ==============================================================================
> > --- spamassassin/trunk/lib/Mail/SpamAssassin.pm (original)
> > +++ spamassassin/trunk/lib/Mail/SpamAssassin.pm Fri Apr 29 16:26:57 2022
> > @@ -1997,9 +1997,10 @@ sub set_global_state_dir {
> >  sub test_global_state_dir {
> >      my ($self, $dir) = @_;
> >      eval { mkpath($dir, 0, 0700); }; # just a single stat if exists already
> > -    # Purge stale test files
> > -    if (opendir(WT_DIR, $dir)) {
> > -      foreach (grep {/^\.sawritetest/ && (-M "$dir/$_"||0) > 0.0001} readdir(WT_DIR)) {
> > +    # Purge stale test files (enough to do only some times randomly)
> > +    if (rand() < 0.2 && opendir(WT_DIR, $dir)) {
> > +      foreach (grep {index($_, '.sawritetest') == 0 &&
> > +                 (-M File::Spec->catdir($dir, $_)||0) > 0.0001} readdir(WT_DIR)) {
> >          unlink(Mail::SpamAssassin::Util::untaint_file_path(File::Spec->catdir($dir, $_)));
> >        }
> >        closedir WT_DIR;
> > 

File::Spec?

Posted by Henrik K <he...@hege.li>.
Ok it seems I'm going down the path of just doing stuff for the sake of
"that's the way it's always been done"..

Does someone actually know why File::Spec->catdir (and catfile, which seems
to work completely identically anyway) is used all over SpamAssassin?  Was
it for some legacy Windows compatibility?  Does any Perl on Windows actually
use native C:\Foo\Bar style paths that would require this method?  Does SA
4.0 even have any promise to work on Windows, and at what capacity? 
Strawberry Perl?  Activestate Perl?  Cygwin?  WSL?  Why not just simplify
code and ditch all the complicated to look and read File::Spec stuff..  you
can also find many many examples where it is not used..  either it should be
used 100% everywhere, or then there is no actual portability.


On Fri, Apr 29, 2022 at 04:26:57PM -0000, hege@apache.org wrote:
> Author: hege
> Date: Fri Apr 29 16:26:57 2022
> New Revision: 1900390
> 
> URL: http://svn.apache.org/viewvc?rev=1900390&view=rev
> Log:
> Purge write testfiles only sometimes, remember to use catdir
> 
> Modified:
>     spamassassin/trunk/lib/Mail/SpamAssassin.pm
> 
> Modified: spamassassin/trunk/lib/Mail/SpamAssassin.pm
> URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin.pm?rev=1900390&r1=1900389&r2=1900390&view=diff
> ==============================================================================
> --- spamassassin/trunk/lib/Mail/SpamAssassin.pm (original)
> +++ spamassassin/trunk/lib/Mail/SpamAssassin.pm Fri Apr 29 16:26:57 2022
> @@ -1997,9 +1997,10 @@ sub set_global_state_dir {
>  sub test_global_state_dir {
>      my ($self, $dir) = @_;
>      eval { mkpath($dir, 0, 0700); }; # just a single stat if exists already
> -    # Purge stale test files
> -    if (opendir(WT_DIR, $dir)) {
> -      foreach (grep {/^\.sawritetest/ && (-M "$dir/$_"||0) > 0.0001} readdir(WT_DIR)) {
> +    # Purge stale test files (enough to do only some times randomly)
> +    if (rand() < 0.2 && opendir(WT_DIR, $dir)) {
> +      foreach (grep {index($_, '.sawritetest') == 0 &&
> +                 (-M File::Spec->catdir($dir, $_)||0) > 0.0001} readdir(WT_DIR)) {
>          unlink(Mail::SpamAssassin::Util::untaint_file_path(File::Spec->catdir($dir, $_)));
>        }
>        closedir WT_DIR;
>