5.36 | File::Find::Rule issues

[shawnlaffan]

This might be an issue with the 5.36 builds, hence I am reporting here first. If it can be replicated using a build of 5.37.x then I'll report it upstream to File::Find::Rule (although maybe it's a File::Find issue underneath).

The code below passes on 5.32, but fails on 5.36. The array populated from paths with backslashes is empty.

My suspicion is that the patching we are doing to File::Find is incomplete, or the patch itself is incomplete. Can someone who has compiled a recent 5.37 on windows please try this code and report? @sisyphus perhaps?

use strict;
use warnings;
use 5.014;  #  regexp /r flag
use Env qw /@PATH/;
use File::Find::Rule;
use Test::More;

#  Need to remove system root or we get different inconsistencies,
#  which are presumably a different issue.
my $system_root = $ENV{SystemRoot} || $ENV{WINDIR};
my @path = grep {$_ !~ /\Q$system_root\E/} @PATH;

my @path_fw_slash = map {$_ =~ s{\\}{/}gr} @path;

my @dll_files1 = sort map {$_ =~ s{\\}{/}gr} 
    File::Find::Rule->file()
                        ->name( "*.$Config::Config{so}" )
                        ->maxdepth(1)
                        ->in( @path );

my @dll_files2 = sort map {$_ =~ s{\\}{/}gr} 
    File::Find::Rule->file()
                        ->name( "*.$Config::Config{so}" )
                        ->maxdepth(1)
                        ->in( @path_fw_slash );

is (scalar @dll_files1, scalar @dll_files2);
is_deeply (\@dll_files1, \@dll_files2);

done_testing();

5.32:

ok 1
ok 2
1..2

5.36:

not ok 1
#   Failed test at xx.pl line 25.
#     Structures begin differing at:
#          $got->[0] = Does not exist
#     $expected->[0] = 'C:/Program Files (x86)/Common Files/Intel/Shared Libraries/redist/intel64/compiler/cilkrts20.dll'
1..1
# Looks like you failed 1 test of 1.

[sisyphus]

With x64 build of portable SP-5.32.0 I'm getting:

D:\pscrpt>perl try.pl
Use of uninitialized value $_ in pattern match (m//) at try.pl line 11.
Use of uninitialized value $_ in substitution (s///) at try.pl line 13.
invalid top directory at D:/sp/_64/sp-5.32.0/perl/lib/File/Find.pm line 130.

With x64 build of perl-5.37.9 (current devel release - though 5.37.10 will be out in a day or two), I'm getting the nearly identical:

D:\pscrpt>perl try.pl
Use of uninitialized value $_ in pattern match (m//) at try.pl line 11.
Use of uninitialized value $_ in substitution (s///) at try.pl line 13.
invalid top directory at D:/perl-5.37.9-1220/lib/File/Find.pm line 143.

With x64 perl-5.36.0, I get:

D:\pscrpt>perl try.pl
not ok 1
#   Failed test at try.pl line 27.
#          got: '3409'
#     expected: '3412'
not ok 2
#   Failed test at try.pl line 28.
#     Structures begin differing at:
#          $got->[74] = 'C:/Windows/system32/0ae3b998-9a38-4b72-a4c4-06849441518d_Servicing-Stack.dll'
#     $expected->[74] = 'C:/Windows/System32/WindowsPowerShell/v1.0/PSEvents.dll'
1..2
# Looks like you failed 2 tests of 2.

Not sure where that leaves you.
I'm not at all familiar with this stuff.

Line 11 is:

my @path = grep {$_ !~ /\Q$system_root\E/} @PATH;

and line 13 is:

my @path_fw_slash = map {$_ =~ s{\\}{/}gr} @path;

Cheers,
Rob

[shawnlaffan]

Another data point.

Installing File::Find::Rule::Perl with 5.36 gets these failures. They look similar to those in #84

"C:\strawberry\perl\bin\perl.exe" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib\lib', 'blib\arch')" t/*.t
t/01_compile.t ... ok
t/02_main.t ...... ok
t/03_no_index.t .. 1/22
#   Failed test 'Found the expected files'
#   at t/03_no_index.t line 125.
#     Structures begin differing at:
#          $got->[0] = 't/dist/META.yml'
#     $expected->[0] = 'META.yml'

#   Failed test 'Found the expected files'
#   at t/03_no_index.t line 156.
#     Structures begin differing at:
#          $got->[0] = 'C:/Users/z9803549/.cpanm/work/1679220091.27400/File-Find-Rule-Perl-1.16/t/dist/META.yml'
#     $expected->[0] = 'META.yml'
# Looks like you failed 2 tests of 22.
t/03_no_index.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/22 subtests

Test Summary Report
-------------------
t/03_no_index.t (Wstat: 512 (exited 2) Tests: 22 Failed: 2)
  Failed tests:  13, 19
  Non-zero exit status: 2
Files=3, Tests=31,  1 wallclock secs ( 0.01 usr +  0.02 sys =  0.03 CPU)
Result: FAIL
Failed 1/3 test programs. 2/31 subtests failed.
gmake: *** [makefile:858: test_dynamic] Error 2
FAIL

[shawnlaffan]

Looking at File::Find::Rule it is using File::Spec, but if File::Find is returning paths with forward slashes on Windows now then File::Spec operations won't work. (Unless it also has been updated to handle forward slashes).

See sub starting at https://metacpan.org/release/RCLAMP/File-Find-Rule-0.34/source/lib/File/Find/Rule.pm#L547

Edit: Although this is not borne out by checking.

perl -MFile::Spec -E"say join ' ', File::Spec->splitdir('c:/a/b/c')"
c: a b c

[shawnlaffan]

Thanks @sisyphus . It seems $ENV{SystemRoot} and $ENV{WINDIR} are both undefined on your system. That just means the secondary issue is triggered.

The main issue is the paths. On 5.36 dev I am getting an empty array.

I suspect there might be a missing setting somewhere in the 5.36 config.

[sisyphus]

@sisyphus . It seems $ENV{SystemRoot} and $ENV{WINDIR} are both undefined on your system.

Nah ... they're both set to C:\WINDOWS on all three builds.
If, on perl-5.36.0, I do a case-insensitive match (my @path = grep {$_ !~ /\Q$system_root\E/i} @PATH; then the tests pass.
But, with the case-insensitive match, 5.32.0 and 5.37.9 still fail in the same way that they did before..

Cheers,
Rob

[sisyphus]

If I replace the original:

my @path = grep {$_ !~ /\Q$system_root\E/} @PATH;

with

my @path;
for(@PATH) {
  next unless $_;
  push @path, $_
    unless $_ =~ /\Q$system_root\E/i;
}

then 5.32.0 and 5.36.0 pass both tests (and no warnings).
But 5.37.9 fails with:

D:\pscrpt>perl try.pl
not ok 1
#   Failed test at try.pl line 41.
#          got: '0'
#     expected: '240'
not ok 2
#   Failed test at try.pl line 42.
#     Structures begin differing at:
#          $got->[0] = Does not exist
#     $expected->[0] = 'C:/winlibs-gcc-1220/mingw64/bin/libLLVMAggressiveInstCombine.dll'
1..2
# Looks like you failed 2 tests of 2.

Cheers,
Rob

[shawnlaffan]

Thanks again Rob.

Which 5.36 dev version are you using? If it's the initial release then that did not have the File::Find patch applied. The current one is https://github.com/StrawberryPerl/Perl-Dist-Strawberry/releases/tag/dev_20230318

I should have been clearer about that but if you're using the initial release then that helps narrow things down.

[shawnlaffan]

This could be an issue in File::Find where calling code is not expecting paths with forward slashes.

Excel::Writer::XLSX 1.10 is showing failures for perl 5.37.8. These are fixed in 1.11.

It's just speculation, but perhaps File::Find on Windows should return paths with whatever slashes were passed in (if this is detectable) and backslashes by default.

[sisyphus]

I was actually using my own build of 5.36.0

Running your original script on both 20230316 and 20230318, I get:

D:\pscrpt>perl try.pl
Use of uninitialized value $_ in pattern match (m//) at try.pl line 11.
Use of uninitialized value $_ in substitution (s///) at try.pl line 13.
invalid top directory at c:/strawberry/perl/lib/File/Find.pm line 140.

Cheers,
Rob

[shawnlaffan]

Thanks once again Rob.

The invalid top directory suggests an undef starting directory is sneaking in somehow on your system. It suspect it's a different issue (but could of course be wrong).

The fact that your 5.37.9 build has the same error indicates to me that the changes in Perl PR20008 need further tweaking as it seems File::Find is returning different data that the calling libraries are not expecting. Alternately the expectations of the calling code need to be updated, as was done in jmcnamara/excel-writer-xlsx#284.

[sisyphus]

I'll probably bow out of this one, I think. It all just confuses me.
It's stupid that Windows paths are mostly (but not always) allowed to contain either forward-slashes or back-slashes,

I still don't understand why your grep search is case-sensitive.
"C:\WINDOWS" does not match "C:\Windows" unless case is ignored - and for me, @path is simply a copy of @PATH.
Also. my @PATH contains stuff like ...;%SystemRoot%\system32;%SystemRoot%;%SystemRoot%\System32\Wbem;..which, of course, gets grepped into @path.
I wonder what's the point of removing C:\Windows if %SystemRoot% is allowed to remain, given that they're both the same.
That's apparently also the source of the "invalid top directory" error ?

If I replace

my @path = grep {$_ !~ /\Q$system_root\E/} @PATH;

with

my @path;
for(@PATH) {
  next unless $_;
  next if $_ =~ /%/;
  push @path, $_
    unless $_ =~ /\Q$system_root\E/i;
}

then on sp536_20230318, I get:

D:\pscrpt>perl try.pl
not ok 1
#   Failed test at try.pl line 35.
#          got: '0'
#     expected: '87'
not ok 2
#   Failed test at try.pl line 36.
#     Structures begin differing at:
#          $got->[0] = Does not exist
#     $expected->[0] = 'c:/strawberry/c/bin/libXpm__.dll'
1..2
# Looks like you failed 2 tests of 2.

Cheers,
Rob

[shawnlaffan]

Removal of C:\Windows was to simplify things in the next part, but obviously this has not survived contact with reality :)

I had not considered paths with windows variables in them (without them being expanded to their actual paths). If such directories are passed through to File::Find without being then it won't know what to do with them.

In any case, your results seem to confirm the main issue, that File::Find is behaving differently on windows for 5.37.9 and the "sp5.36dev" build. Edit - and that this behavioural change is not expected by several of the libraries on CPAN.

@genio - any thoughts?

[shawnlaffan]

The issue (symptom) in File::Find::Rule is where it is using regexes to check file paths in its File::Find wanted sub, but the paths in File::Find use forward slashes on Windows since the application of Perl/perl5#20008.

Updating the File::Find code to use forward slashes for $topdir around L601 avoids the issue. https://metacpan.org/release/RCLAMP/File-Find-Rule-0.34/source/lib/File/Find/Rule.pm#L601

for my $path (@_) {
        # $topdir is used for relative and maxdepth
        $topdir = $path;
        $topdir =~ s{\\}{/}g;  ###  THIS LINE HAS BEEN ADDED
        # slice off the trailing slash if there is one (the
        # maxdepth/mindepth code is fussy)
        $topdir =~ s{/?$}{}
          unless $topdir eq '/';
        $self->_call_find( { %{ $self->{extras} }, wanted => $sub }, $path );
    }

I'll report this to File::Find::Rule, but the general question is whether this should be handled in File::Find, as there are no doubt other packages out there that will hit the same problems.

@xenu - FYI

[shawnlaffan]

Reported to FIle::Find::Rule at https://rt.cpan.org/Ticket/Display.html?id=147252

[shawnlaffan]

the general question is whether this should be handled in File::Find, as there are no doubt other packages out there that will hit the same problems.

Although this issue here was in the sub passed to File::Find, so there is nothing File::Find can do about that.

[shawnlaffan]

PR submitted to File::Find::Rule. If need be then we can patch the module for 5.36.

richardc/perl-file-find-rule#3

[genio]

I'd say let's patch it for now so we can get over this particular hump. Sorry, I've been a bit MIA. I'll try to get back on this

[shawnlaffan]

I've had no response from the maintainer so patching looks the way to go, using a dev version ID (0.41_01).

The simplest approach is to put the tarball in the share dir on this repo. We could also add it to strawberryperl.com next to some of the other patched modules. Alternately I can list it as a release on my fork.

[shawnlaffan]

Patched distro is at https://github.com/StrawberryPerl/Perl-Dist-Strawberry/releases/download/dev_20230318/File-Find-Rule-0.34_01.tar.gz

It will be visible in the build script when next I push changes.