timbunce / devel-nytprof

Devel::NYTProf is a powerful feature-rich source code profiler for Perl. (Mostly in maintenance mode, so PRs are much more likely to be acted upon than Issues.)
http://blog.timbunce.org/tag/nytprof/
67 stars 51 forks source link

Makefile.PL not locating directories for headers in Appveyor test runs #163

Closed jkeenan closed 3 years ago

jkeenan commented 3 years ago

Adapted from a commit message in the diagnose-zlib-problem-20210424 branch:

commit 8c35ed267054c4ef4bb2910ed1700eee8f23af74
Author:     James E Keenan <jkeenan@cpan.org>
AuthorDate: Sat Apr 24 09:22:42 2021 -0400
Commit:     James E Keenan <jkeenan@cpan.org>
CommitDate: Sat Apr 24 10:44:29 2021 -0400

Diagnose difficulties with 4 tests on Appveyor.

In order to improve test coverage we've been adding test files like t/11-reader.t. Each such test file processes a dummy copy file like nytprof_11-reader.out.txt, and each such dummy copy file uses compression.

It appears that the Strawberry Perl configuration used by Appveyor does not handle compression well. As early as this test run on Mar 31 2024 we were getting failures reported like this:

File uses compression but compression is not supported by this build
of NYTProf at C:\projects\devel-nytprof\blib\lib/Devel/NYTProf/Data.pm line 87.

(That error message is coded in NYTProf.xs.)

As a consequence, for the past month we have been skipping the test files in question when run on Windows (and also, incidentally, VMS). The criterion for skipping the test files has been the operating system -- which is almost certainly too broad. The criterion ought to be more narrower, e.g. whether the library is being compiled with '-DHAS_ZLIB' (which is what the test harness mentions).

However, the diagnostic work in this branch shows that Makefile.PL is coded sub-optimally even before we get to particular test files. Inside that program, @h_dirs is not populated at all on the Strawberry Perl build used by Appveyor. See a recent test run for this branch.

Is this a problem with this specific build of Strawberry Perl? With Strawberry Perl in general? With Devel::NYTProf on Windows in general?

Needs investigation.

diff --git a/Makefile.PL b/Makefile.PL
index 94094ab..0bfe5c3 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -68,14 +68,14 @@ my $cpp = $Config{cpprun} || do {
 print "Looking for header files and functions...\n";
 my $INCLUDE;

-my $h_files;
 my @h_dirs;
 push @h_dirs, split /:/, $ENV{INCLUDE} if $ENV{INCLUDE};
 push @h_dirs, split ' ', $Config{libsdirs};
 push @h_dirs, qw(/include /usr/include /usr/local/include /usr/include/mach);
 @h_dirs = grep { -d $_ } @h_dirs;
+print "No header directories located\n" unless @h_dirs;

-$h_files = find_h_files(@h_dirs);
+my $h_files = (@h_dirs) ? find_h_files(@h_dirs) : {};

 # See lib/ExtUtils/MakeMaker.pm for details of how to influence
 # the contents of the Makefile that is written.
@@ -83,20 +83,28 @@ my %mm_opts;
 my @libs = ();

 my @hdr_match_lib;
-push @hdr_match_lib, ['time.h', qr/(clock_gettime)\s*\(/, '-DHAS_CLOCK_GETTIME', '-lrt']
-    if $opt_gettime;
-push @hdr_match_lib, ['zlib.h', qr/(deflateInit2)(?:_)?\s*\(/, '-DHAS_ZLIB',     '-lz']
-    if $opt_zlib;
-push @hdr_match_lib, ['mach_time.h', qr/(mach_absolute_time)\s*\(/, '-DHAS_MACH_TIME', undef]
-    if $opt_machtime and $^O eq 'darwin';
+my $time_h_test = ['time.h', qr/(clock_gettime)\s*\(/, '-DHAS_CLOCK_GETTIME', '-lrt'];
+my $zlib_h_test = ['zlib.h', qr/(deflateInit2)(?:_)?\s*\(/, '-DHAS_ZLIB',     '-lz'];
+my $mach_time_h_test = ['mach_time.h', qr/(mach_absolute_time)\s*\(/, '-DHAS_MACH_TIME', undef];
+
+push @hdr_match_lib, $time_h_test if $opt_gettime;
+push @hdr_match_lib, $zlib_h_test if $opt_zlib;
+push @hdr_match_lib, $mach_time_h_test if $opt_machtime and $^O eq 'darwin';
+
+print "Based on options settings, we want to investigate these headers for certain symbols:\n";
+print "  $_->[0]\n" for @hdr_match_lib;

 foreach (@hdr_match_lib) {
     my ($header, $regexp, $define, $libs) = @$_;
-    if (my $result = search_h_file($header, $regexp)) {
-        print "Found $result in $header\n";
+    my $result = search_h_file($h_files, $header, $regexp);
+    if (defined $result) {
+        print "  Found $result in $header\n";
         push @libs, $libs if $libs;
         $mm_opts{DEFINE} .= " $define" if $define;
     }
+    else {
+        print "  Did not find what we were looking for in $header; not compiling with '$define'\n";
+    }
 }

 if ($opt_assert or (not defined $opt_assert and $is_developer)) {
@@ -211,28 +219,35 @@ sub find_h_files {
     my %h_files;
     foreach my $dir (@dirs) {
         next unless $dir;
-        opendir(DIR, $dir)
+        opendir(my $DIR, $dir)
             or next;    # silently ignore missing directories

-        while (my $file = readdir(DIR)) {
+        while (my $file = readdir($DIR)) {
             next unless $file =~ /\.h$/;
             $h_files{$file} ||= $dir;    # record first found
         }
+        close $DIR;
     }
-    close DIR;
     return \%h_files;
 }

 sub search_h_file {
-    my ($h_file, $regex) = @_;
-    my $dir = $h_files->{$h_file}
-        or return undef;
-    open H, "$cpp $dir/$h_file |";
-    while (<H>) {
+    my ($h_files, $h_file, $regex) = @_;
+    print "Attempting $h_file ...\n";
+    my $dir = $h_files->{$h_file};
+    unless ($dir) {
+        print "  Could not identify path for $h_file\n";
+        return;
+    }
+    my $file = File::Spec->catfile($dir, $h_file);
+    print "Processing $file ...\n";
+    print "  $file ", (-f $file ? "exists" : "does not exist"), "\n";
+    open my $H, "$cpp $file |";
+    while (<$H>) {
         return $1 if m/$regex/;
     }
-    close H;
-    return undef;
+    close $H;
+    return;
 }
jkeenan commented 3 years ago

This problem may be resolved by p.r. https://github.com/timbunce/devel-nytprof/pull/171.