Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] make_ext.pl: run all Makefile.PLes in 1 process #15054

Closed
p5pRT opened this issue Nov 20, 2015 · 18 comments
Closed

[PATCH] make_ext.pl: run all Makefile.PLes in 1 process #15054

p5pRT opened this issue Nov 20, 2015 · 18 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 20, 2015

Migrated from rt.perl.org#126686 (status was 'open')

Searchable as RT126686$

@p5pRT
Copy link
Author

p5pRT commented Nov 20, 2015

From @bulk88

Created by @bulk88

..\miniperl.exe -I..\lib ..\make_ext.pl "MAKE=dmake" --dir=..\cpan
--dir=..\dist --dir=..\ext --dynamic !Unicode/Normalize

before

Version Number​: Windows NT 5.2 (Build 3790)
Exit Time​: 0​:21 am, Thursday, November 19 2015
Elapsed Time​: 0​:05​:13.765
Process Time​: 0​:00​:00.546
System Calls​: 5903009
Context Switches​: 1995063
Page Faults​: 4406610
Bytes Read​: 921666627
Bytes Written​: 142523970
Bytes Other​: 15734093

Version Number​: Windows NT 5.2 (Build 3790)
Exit Time​: 0​:43 am, Thursday, November 19 2015
Elapsed Time​: 0​:05​:13.062
Process Time​: 0​:00​:00.625
System Calls​: 8432811
Context Switches​: 1964390
Page Faults​: 4419335
Bytes Read​: 1020883832
Bytes Written​: 173697641
Bytes Other​: 15747131

Version Number​: Windows NT 5.2 (Build 3790)
Exit Time​: 0​:53 am, Thursday, November 19 2015
Elapsed Time​: 0​:05​:13.109
Process Time​: 0​:00​:00.515
System Calls​: 5841235
Context Switches​: 1956027
Page Faults​: 4401914
Bytes Read​: 918941172
Bytes Written​: 142660117
Bytes Other​: 15644366

after

Version Number​: Windows NT 5.2 (Build 3790)
Exit Time​: 1​:00 am, Thursday, November 19 2015
Elapsed Time​: 0​:04​:59.843
Process Time​: 0​:00​:06.812
System Calls​: 5727887
Context Switches​: 1921204
Page Faults​: 4301906
Bytes Read​: 883224462
Bytes Written​: 142651901
Bytes Other​: 15350562

Version Number​: Windows NT 5.2 (Build 3790)
Exit Time​: 1​:17 am, Thursday, November 19 2015
Elapsed Time​: 0​:05​:01.046
Process Time​: 0​:00​:06.593
System Calls​: 6557325
Context Switches​: 2065488
Page Faults​: 4430735
Bytes Read​: 883425807
Bytes Written​: 143529569
Bytes Other​: 15375963

Version Number​: Windows NT 5.2 (Build 3790)
Exit Time​: 1​:43 am, Thursday, November 19 2015
Elapsed Time​: 0​:05​:00.750
Process Time​: 0​:00​:06.625
System Calls​: 9391458
Context Switches​: 2107444
Page Faults​: 4335664
Bytes Read​: 998322960
Bytes Written​: 194058504
Bytes Other​: 15480682

notice make_ext.pl's CPU time increased from 0.5 sec to 6.75 sec, since
work was shifted from other processes underneath make_ext.pl to
make_ext.pl, but total wall time decreased by 13 seconds. Many perl
process launches, and parsing/compiling of the whole EUMM software stack
were saved. I decided on using an END block instead of overriding
CORE​::GLOBAL​::exit for conservativeness reasons/maybe perf reasons
(write to closure lexicals, instead of write to sub lexicals AND hash
keys). This commit causes large whitespace changes due to the new
inter-sub scope created. IDK any way around that, other than to use a
object that represents a makefile to remove (the dir, the makefile's
name), which the execution would be bless->do 'Makefile.PL'->disarm
object with meth call (overkill for a class whose only meth is DESTROY)
or directly going inside it->automatic DESTROY (if not explicitly
disarmed earlier, it deletes the makefile).

The before and after %INC module list of make_ext.pl process, does not
change at all with this patch. NOT AT ALL!

..\lib/buildcustomize.pl
Carp.pm
Config.pm
Config_git.pl
Config_heavy.pl
Cwd.pm
DirHandle.pm
DynaLoader.pm
Exporter.pm
Exporter/Heavy.pm
ExtUtils/Liblist.pm
ExtUtils/Liblist/Kid.pm
ExtUtils/MM.pm
ExtUtils/MM_Any.pm
ExtUtils/MM_Unix.pm
ExtUtils/MM_Win32.pm
ExtUtils/MY.pm
ExtUtils/MakeMaker.pm
ExtUtils/MakeMaker/Config.pm
ExtUtils/MakeMaker/Locale.pm
ExtUtils/MakeMaker/version.pm
File/Basename.pm
File/Path.pm
File/Spec.pm
File/Spec/Unix.pm
File/Spec/Win32.pm
FindExt.pm
Symbol.pm
Win32.pm
constant.pm
strict.pm
vars.pm
version.pm
version/regex.pm
warnings.pm
warnings/register.pm

make_ext.pl before this patch, already loaded all of EUMM into the
make_ext.pl for somewhat dubious (IMO) reasons.

How make_ext.pl before, loaded all of EUMM.

  at C​:\p523\src\cpan\ExtUtils-MakeMaker\lib/ExtUtils/MakeMaker.pm line
13, <$mfh> line 71.
  require ExtUtils/MakeMaker.pm called at
C​:\p523\src\cpan\ExtUtils-MakeMaker\lib/ExtUtils/MM_Unix.pm line 14
  ExtUtils​::MM_Unix​::BEGIN() called at
C​:\p523\src\cpan\ExtUtils-MakeMaker\lib/ExtUtils/MakeMaker.pm line 0
  eval {...} called at
C​:\p523\src\cpan\ExtUtils-MakeMaker\lib/ExtUtils/MakeMaker.pm line 0
  require ExtUtils/MM_Unix.pm called at ..\make_ext.pl line 303
  main​::build_extension("dist/PathTools",
"C​:\\p523\\src\\win32\\..\\miniperl.exe", "Cwd", "all",
ARRAY(0x1a238c0)) called at ..\make_ext.pl line 258

make_ext.pl contains

----------------------------------------------------------
  if (-f $makefile) {
  $makefile_no_minus_f = 0;
  open my $mfh, $makefile or die "Cannot open $makefile​: $!";
  while (<$mfh>) {
  # Plagiarised from CPAN​::Distribution
  last if /MakeMaker post_initialize section/;
  next unless /^#\s+VERSION_FROM\s+=>\s+(.+)/;
  my $vmod = eval $1;
  my $oldv;
  while (<$mfh>) {
  next unless /^XS_VERSION = (\S+)/;
  $oldv = $1;
  last;
  }
  last unless defined $oldv;
  require ExtUtils​::MM_Unix;<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  defined (my $newv = parse_version MM $vmod) or last;
--------------------------------------------------------

ExtUtils​::MM_Unix contains

----------------------------------------------------
use ExtUtils​::MakeMaker qw($Verbose neatvalue);
----------------------------------------------------

Now all of EUMM was sucked in. "require ExtUtils​::MM_Unix;" was added
in
http​://perl5.git.perl.org/perl.git/commit/baff067e717fe1c071dea706f7425994cc1d4ce9
"[perl #113940] Make make_ext delete Makefiles when version changes"
from Sun Jul 29 01​:48​:27 2012
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=113940 , I think that that
commit is perf robbing and shouldn't have ever been committed, but oh
well. I guess reverting "[perl #113940] Make make_ext delete Makefiles
when version changes" is out of the question.

With this change of running the Makefile.PL inside make_ext.pl, 1
load/compile of the EUMM stack is prevented. Even on 1 module per
make_ext.pl usage of make_ext.pl for the unix makefile, some resources
will still be saved since the number of EUMM loads was cut in half
instead of cut by 4/$number_of_core_modules_buildable_on_Win32 as on Win32.

Perl Info

Flags:
      category=core
      severity=low

Site configuration information for perl 5.23.5:

Configured by Owner at Sun Oct 25 19:14:27 2015.

Summary of my perl5 (revision 5 version 23 subversion 5) configuration:
    Derived from: 644207b7a8ff7a2b1661c05a7f9ac2df9a5dad91
    Platform:
      osname=MSWin32, osvers=6.1, archname=MSWin32-x86-multi-thread
      uname=''
      config_args='undef'
      hint=recommended, useposix=true, d_sigaction=undef
      useithreads=define, usemultiplicity=define
      use64bitint=undef, use64bitall=undef, uselongdouble=undef
      usemymalloc=n, bincompat5005=undef
    Compiler:
      cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -GL 
-DWIN32 -D_CONSOLE -DNO_STRICT -D_CRT_SECURE_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_DEPRECATE  -DPERL_TEXTMODE_SCRIPTS 
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS',
      optimize='-O1 -MD -Zi -DNDEBUG -GL',
      cppflags='-DWIN32'
      ccversion='18.00.31101', gccversion='', gccosandvers=''
      intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234, 
doublekind=3
      d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8, 
longdblkind=0
      ivtype='long', ivsize=4, nvtype='double', nvsize=8, 
Off_t='__int64', lseeksize=8
      alignbytes=8, prototype=define
    Linker and Libraries:
      ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf 
-ltcg         -libpath:"c:\p523\lib\CORE"         -machine:x86 
"/manifestdependency:type='Win32' 
name='Microsoft.Windows.Common-Controls' version='6.0.0.0' 
processorArchitecture='*' publicKeyToken='6595b64144ccf1df' 
language='*'" -subsystem:console,"5.01"'
      libpth=\lib
      libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib 
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib 
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib 
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
      perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib 
winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib 
oleaut32.lib netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib 
version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
      libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl523.lib
      gnulibc_version=''
    Dynamic Linking:
      dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
      cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug 
-opt:ref,icf -ltcg         -libpath:"c:\p523\lib\CORE"  -machine:x86 
"/manifestdependency:type='Win32' 
name='Microsoft.Windows.Common-Controls' version='6.0.0.0' 
processorArchitecture='*' publicKeyToken='6595b64144ccf1df' 
language='*'" -subsystem:console,"5.01"'

Locally applied patches:
      uncommitted-changes


@INC for perl 5.23.5:
      C:/p523/site/lib
      C:/p523/lib
      .


Environment for perl 5.23.5:
      HOME (unset)
      LANG (unset)
      LANGUAGE (unset)
      LD_LIBRARY_PATH (unset)
      LOGDIR (unset)
      PATH=C:\p523\site\bin;C:\p523\bin;C:\Program Files\ActiveState 
Komodo Edit 
9\;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program 
Files\TortoiseGit\bin;C:\Program Files\Microsoft Windows Performance 
Toolkit\;C:\strawberry\c\bin;C:\strawberry\perl\site\bin;C:\strawberry\perl\bin;C:\Program 
Files\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program 
Files\Microsoft SQL Server\110\Tools\Binn\;C:\Program Files\Microsoft 
SDKs\TypeScript\1.0\;C:\Program Files\TortoiseHg\;
      PERL_BADLANG (unset)
      SHELL (unset)



@p5pRT
Copy link
Author

p5pRT commented Nov 20, 2015

From @bulk88

0001-make_ext.pl-run-all-Makefile.PLes-in-1-process.patch
From 03b715f0d82386dbed79e0c571fa544e0893b675 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Thu, 19 Nov 2015 22:27:25 -0500
Subject: [PATCH] make_ext.pl: run all Makefile.PLes in 1 process

This saves 13 wall seconds for "..\miniperl.exe -I..\lib ..\make_ext.pl
"MAKE=dmake" --dir=..\cpan --dir=..\dist --dir=..\ext --dynamic
!Unicode/Normalize" to complete on Win32. This patch catches all exit()
calls from Makefile.PLs, including exit(0) but no core modules do exit(0)
as normal flow in their Makefile.PLs. It also catches exceptions too. See
Perl RT ticket for details.
---
 make_ext.pl | 476 +++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 247 insertions(+), 229 deletions(-)

diff --git a/make_ext.pl b/make_ext.pl
index 223f67e..7ac17bb 100644
--- a/make_ext.pl
+++ b/make_ext.pl
@@ -258,143 +258,147 @@ foreach my $spec (@extspec)  {
 		    [@pass_through, @{$extra_passthrough{$spec} || []}]);
 }
 
-sub build_extension {
-    my ($ext_dir, $perl, $mname, $target, $pass_through) = @_;
+{
+    my($del_makefile_on_exit, $ext_dir, $makefile);
 
-    unless (chdir "$ext_dir") {
-	warn "Cannot cd to $ext_dir: $!";
-	return;
-    }
+    sub build_extension { #build_extensions is not recursion safe
+	$ext_dir = shift;
+	my ($perl, $mname, $target, $pass_through) = @_;
+
+	unless (chdir "$ext_dir") {
+	    warn "Cannot cd to $ext_dir: $!";
+	    return;
+	}
 
-    my $up = $ext_dir;
-    $up =~ s![^/]+!..!g;
+	my $up = $ext_dir;
+	$up =~ s![^/]+!..!g;
 
-    $perl ||= "$up/miniperl";
-    my $return_dir = $up;
-    my $lib_dir = "$up/lib";
+	$perl ||= "$up/miniperl";
+	my $return_dir = $up;
+	my $lib_dir = "$up/lib";
 
-    my ($makefile, $makefile_no_minus_f);
-    if (IS_VMS) {
-	$makefile = 'descrip.mms';
-	if ($target =~ /clean$/
-	    && !-f $makefile
-	    && -f "${makefile}_old") {
-	    $makefile = "${makefile}_old";
+	my ($makefile_no_minus_f);
+	if (IS_VMS) {
+	    $makefile = 'descrip.mms';
+	    if ($target =~ /clean$/
+		&& !-f $makefile
+		&& -f "${makefile}_old") {
+		$makefile = "${makefile}_old";
+	    }
+	} else {
+	    $makefile = 'Makefile';
 	}
-    } else {
-	$makefile = 'Makefile';
-    }
-    
-    if (-f $makefile) {
-	$makefile_no_minus_f = 0;
-	open my $mfh, $makefile or die "Cannot open $makefile: $!";
-	while (<$mfh>) {
-	    # Plagiarised from CPAN::Distribution
-	    last if /MakeMaker post_initialize section/;
-	    next unless /^#\s+VERSION_FROM\s+=>\s+(.+)/;
-	    my $vmod = eval $1;
-	    my $oldv;
+
+	if (-f $makefile) {
+	    $makefile_no_minus_f = 0;
+	    open my $mfh, $makefile or die "Cannot open $makefile: $!";
 	    while (<$mfh>) {
-		next unless /^XS_VERSION = (\S+)/;
-		$oldv = $1;
-		last;
+		# Plagiarised from CPAN::Distribution
+		last if /MakeMaker post_initialize section/;
+		next unless /^#\s+VERSION_FROM\s+=>\s+(.+)/;
+		my $vmod = eval $1;
+		my $oldv;
+		while (<$mfh>) {
+		    next unless /^XS_VERSION = (\S+)/;
+		    $oldv = $1;
+		    last;
+		}
+		last unless defined $oldv;
+		require ExtUtils::MM_Unix;
+		defined (my $newv = parse_version MM $vmod) or last;
+		if ($newv ne $oldv) {
+		    close $mfh or die "close $makefile: $!";
+		    _unlink($makefile);
+		    {
+			no warnings 'deprecated';
+			goto NO_MAKEFILE;
+		    }
+		}
 	    }
-	    last unless defined $oldv;
-	    require ExtUtils::MM_Unix;
-	    defined (my $newv = parse_version MM $vmod) or last;
-	    if ($newv ne $oldv) {
-		close $mfh or die "close $makefile: $!";
-		_unlink($makefile);
-		{
-		    no warnings 'deprecated';
-		    goto NO_MAKEFILE;
+
+	    if (IS_CROSS) {
+		# If we're cross-compiling, it's possible that the host's
+		# Makefiles are around.
+		seek($mfh, 0, 0) or die "Cannot seek $makefile: $!";
+
+		my $cross_makefile;
+		while (<$mfh>) {
+		    # XXX This might not be throughout enough.
+		    # For example, it's possible to cause a false-positive
+		    # if cross compiling on and for the Raspberry Pi,
+		    # which is insane but plausible.
+		    # False positives are really not troublesome, though;
+		    # all they mean is that the module gets rebuilt.
+		    if (/^CC = \Q$Config{cc}\E/) {
+			$cross_makefile = 1;
+			last;
+		    }
+		}
+
+		if (!$cross_makefile) {
+		    print "Deleting non-Cross makefile\n";
+		    close $mfh or die "close $makefile: $!";
+		    _unlink($makefile);
 		}
 	    }
+	} else {
+	    $makefile_no_minus_f = 1;
 	}
 
-        if (IS_CROSS) {
-            # If we're cross-compiling, it's possible that the host's
-            # Makefiles are around.
-            seek($mfh, 0, 0) or die "Cannot seek $makefile: $!";
-            
-            my $cross_makefile;
-            while (<$mfh>) {
-                # XXX This might not be throughout enough.
-                # For example, it's possible to cause a false-positive
-                # if cross compiling on and for the Raspberry Pi,
-                # which is insane but plausible.
-                # False positives are really not troublesome, though;
-                # all they mean is that the module gets rebuilt.
-                if (/^CC = \Q$Config{cc}\E/) {
-                    $cross_makefile = 1;
-                    last;
-                }
-            }
-            
-            if (!$cross_makefile) {
-                print "Deleting non-Cross makefile\n";
-                close $mfh or die "close $makefile: $!";
-                _unlink($makefile);
-            }
-        }
-    } else {
-	$makefile_no_minus_f = 1;
-    }
+	if ($makefile_no_minus_f || !-f $makefile) {
+	    NO_MAKEFILE:
+	    if (!-f 'Makefile.PL') {
+		unless (just_pm_to_blib($target, $ext_dir, $mname, $return_dir)) {
+		    # No problems returned, so it has faked everything for us. :-)
+		    chdir $return_dir || die "Cannot cd to $return_dir: $!";
+		    return;
+		}
 
-    if ($makefile_no_minus_f || !-f $makefile) {
-	NO_MAKEFILE:
-	if (!-f 'Makefile.PL') {
-            unless (just_pm_to_blib($target, $ext_dir, $mname, $return_dir)) {
-                # No problems returned, so it has faked everything for us. :-)
-                chdir $return_dir || die "Cannot cd to $return_dir: $!";
-                return;
-            }
+		print "\nCreating Makefile.PL in $ext_dir for $mname\n" if $verbose;
+		my ($fromname, $key, $value);
+		if ($mname eq 'podlators') {
+		    # We need to special case this somewhere, and this is fewer
+		    # lines of code than a core-only Makefile.PL, and no more
+		    # complex
+		    $fromname = 'VERSION';
+		    $key = 'DISTNAME';
+		    $value = 'podlators';
+		    $mname = 'Pod';
+		} else {
+		    $key = 'ABSTRACT_FROM';
+		    # We need to cope well with various possible layouts
+		    my @dirs = split /::/, $mname;
+		    my $leaf = pop @dirs;
+		    my $leafname = "$leaf.pm";
+		    my $pathname = join '/', @dirs, $leafname;
+		    my @locations = ($leafname, $pathname, "lib/$pathname");
+		    foreach (@locations) {
+			if (-f $_) {
+			    $fromname = $_;
+			    last;
+			}
+		    }
 
-	    print "\nCreating Makefile.PL in $ext_dir for $mname\n" if $verbose;
-	    my ($fromname, $key, $value);
-	    if ($mname eq 'podlators') {
-		# We need to special case this somewhere, and this is fewer
-		# lines of code than a core-only Makefile.PL, and no more
-		# complex
-		$fromname = 'VERSION';
-		$key = 'DISTNAME';
-		$value = 'podlators';
-		$mname = 'Pod';
-	    } else {
-		$key = 'ABSTRACT_FROM';
-		# We need to cope well with various possible layouts
-		my @dirs = split /::/, $mname;
-		my $leaf = pop @dirs;
-		my $leafname = "$leaf.pm";
-		my $pathname = join '/', @dirs, $leafname;
-		my @locations = ($leafname, $pathname, "lib/$pathname");
-		foreach (@locations) {
-		    if (-f $_) {
-			$fromname = $_;
-			last;
+		    unless ($fromname) {
+			die "For $mname tried @locations in $ext_dir but can't find source";
 		    }
+		    ($value = $fromname) =~ s/\.pm\z/.pod/;
+		    $value = $fromname unless -e $value;
 		}
 
-		unless ($fromname) {
-		    die "For $mname tried @locations in $ext_dir but can't find source";
+		if ($mname eq 'Pod::Checker') {
+		    # the abstract in the .pm file is unparseable by MM,
+		    # so special-case it. We can't use the package's own
+		    # Makefile.PL, as it doesn't handle the executable scripts
+		    # right.
+		    $key = 'ABSTRACT';
+		    # this is copied from the CPAN Makefile.PL v 1.171
+		    $value = 'Pod::Checker verifies POD documentation contents for compliance with the POD format specifications';
 		}
-		($value = $fromname) =~ s/\.pm\z/.pod/;
-		$value = $fromname unless -e $value;
-	    }
 
-            if ($mname eq 'Pod::Checker') {
-                # the abstract in the .pm file is unparseable by MM,
-                # so special-case it. We can't use the package's own
-                # Makefile.PL, as it doesn't handle the executable scripts
-                # right.
-                $key = 'ABSTRACT';
-                # this is copied from the CPAN Makefile.PL v 1.171
-                $value = 'Pod::Checker verifies POD documentation contents for compliance with the POD format specifications';
-            }
-
-	    open my $fh, '>', 'Makefile.PL'
-		or die "Can't open Makefile.PL for writing: $!";
-	    printf $fh <<'EOM', $0, $mname, $fromname, $key, $value;
+		open my $fh, '>', 'Makefile.PL'
+		    or die "Can't open Makefile.PL for writing: $!";
+		printf $fh <<'EOM', $0, $mname, $fromname, $key, $value;
 #-*- buffer-read-only: t -*-
 
 # This Makefile.PL was written by %s.
@@ -447,99 +451,104 @@ WriteMakefile(
 
 # ex: set ro:
 EOM
-	    close $fh or die "Can't close Makefile.PL: $!";
-	    # As described in commit 23525070d6c0e51f:
-	    # Push the atime and mtime of generated Makefile.PLs back 4
-	    # seconds. In certain circumstances ( on virtual machines ) the
-	    # generated Makefile.PL can produce a Makefile that is older than
-	    # the Makefile.PL. Altering the atime and mtime backwards by 4
-	    # seconds seems to resolve the issue.
-	    eval {
-        my $ftime = (stat('Makefile.PL'))[9] - 4;
-        utime $ftime, $ftime, 'Makefile.PL';
-	    };
-        } elsif ($mname =~ /\A(?:Carp
-                            |ExtUtils::CBuilder
-                            |Safe
-                            |Search::Dict)\z/x) {
-            # An explicit list of dual-life extensions that have a Makefile.PL
-            # for CPAN, but we have verified can also be built using the fakery.
-            my ($problem) = just_pm_to_blib($target, $ext_dir, $mname, $return_dir);
-            # We really need to sanity test that we can fake it.
-            # Otherwise "skips" will go undetected, and the build slow down for
-            # everyone, defeating the purpose.
-            if (defined $problem) {
-                if (-d "$return_dir/.git") {
-                    # Get the list of files that git isn't ignoring:
-                    my @files = `git ls-files --cached --others --exclude-standard 2>/dev/null`;
-                    # on error (eg no git) we get nothing, but that's not a
-                    # problem. The goal is to see if git thinks that the problem
-                    # file is interesting, by getting a positive match with
-                    # something git told us about, and if so bail out:
-                    foreach (@files) {
-                        chomp;
-                        # We really need to sanity test that we can fake it.
-                        # The intent is that this should only fail because
-                        # you've just added a file to the dual-life dist that
-                        # we can't handle. In which case you should either
-                        # 1) remove the dist from the regex a few lines above.
-                        # or
-                        # 2) add the file to regex of "safe" filenames earlier
-                        #    in this function, that starts with ChangeLog
-                        die "FATAL - $0 has $mname in the list of simple extensions, but it now contains file '$problem' which we can't handle"
-                            if $problem eq $_;
-                    }
-                    # There's an unexpected file, but it seems to be something
-                    # that git will ignore. So fall through to the regular
-                    # Makefile.PL handling code below, on the assumption that
-                    # we won't get here for a clean build.
-                }
-                warn "WARNING - $0 is building $mname using EU::MM, as it found file '$problem'";
-            } else {
-                # It faked everything for us.
-                chdir $return_dir || die "Cannot cd to $return_dir: $!";
-                return;
-            }
-	}
-
-        # We are going to have to use Makefile.PL:
-	print "\nRunning Makefile.PL in $ext_dir\n" if $verbose;
+		close $fh or die "Can't close Makefile.PL: $!";
+		# As described in commit 23525070d6c0e51f:
+		# Push the atime and mtime of generated Makefile.PLs back 4
+		# seconds. In certain circumstances ( on virtual machines ) the
+		# generated Makefile.PL can produce a Makefile that is older than
+		# the Makefile.PL. Altering the atime and mtime backwards by 4
+		# seconds seems to resolve the issue.
+		eval {
+	    my $ftime = (stat('Makefile.PL'))[9] - 4;
+	    utime $ftime, $ftime, 'Makefile.PL';
+		};
+	    } elsif ($mname =~ /\A(?:Carp
+				|ExtUtils::CBuilder
+				|Safe
+				|Search::Dict)\z/x) {
+		# An explicit list of dual-life extensions that have a Makefile.PL
+		# for CPAN, but we have verified can also be built using the fakery.
+		my ($problem) = just_pm_to_blib($target, $ext_dir, $mname, $return_dir);
+		# We really need to sanity test that we can fake it.
+		# Otherwise "skips" will go undetected, and the build slow down for
+		# everyone, defeating the purpose.
+		if (defined $problem) {
+		    if (-d "$return_dir/.git") {
+			# Get the list of files that git isn't ignoring:
+			my @files = `git ls-files --cached --others --exclude-standard 2>/dev/null`;
+			# on error (eg no git) we get nothing, but that's not a
+			# problem. The goal is to see if git thinks that the problem
+			# file is interesting, by getting a positive match with
+			# something git told us about, and if so bail out:
+			foreach (@files) {
+			    chomp;
+			    # We really need to sanity test that we can fake it.
+			    # The intent is that this should only fail because
+			    # you've just added a file to the dual-life dist that
+			    # we can't handle. In which case you should either
+			    # 1) remove the dist from the regex a few lines above.
+			    # or
+			    # 2) add the file to regex of "safe" filenames earlier
+			    #    in this function, that starts with ChangeLog
+			    die "FATAL - $0 has $mname in the list of simple extensions, but it now contains file '$problem' which we can't handle"
+				if $problem eq $_;
+			}
+			# There's an unexpected file, but it seems to be something
+			# that git will ignore. So fall through to the regular
+			# Makefile.PL handling code below, on the assumption that
+			# we won't get here for a clean build.
+		    }
+		    warn "WARNING - $0 is building $mname using EU::MM, as it found file '$problem'";
+		} else {
+		    # It faked everything for us.
+		    chdir $return_dir || die "Cannot cd to $return_dir: $!";
+		    return;
+		}
+	    }
 
-	my @args = ("-I$lib_dir", 'Makefile.PL');
-	if (IS_VMS) {
-	    my $libd = VMS::Filespec::vmspath($lib_dir);
-	    push @args, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
-	} else {
-	    push @args, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
-		'INSTALLMAN3DIR=none';
-	}
-	push @args, @$pass_through;
-	_quote_args(\@args) if IS_VMS;
-	print join(' ', $perl, @args), "\n" if $verbose;
-	my $code = do {
-	   local $ENV{PERL_MM_USE_DEFAULT} = 1;
-	    system $perl, @args;
-	};
-	if($code != 0){
-	    #make sure next build attempt/run of make_ext.pl doesn't succeed
-	    _unlink($makefile);
-	    die "Unsuccessful Makefile.PL($ext_dir): code=$code";
-	}
+	    # We are going to have to use Makefile.PL:
+	    print "\nRunning Makefile.PL in $ext_dir\n" if $verbose;
+
+	    my $err;
+	    {
+		local @ARGV = (); #fake a system($^X) for perf
+		if (IS_VMS) {
+		    my $libd = VMS::Filespec::vmspath($lib_dir);
+		    push @ARGV, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
+		} else {
+		    push @ARGV, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
+			'INSTALLMAN3DIR=none';
+		}
+		push @ARGV, @$pass_through;
+		print join(' ', @ARGV), "\n" if $verbose;
+		local $ENV{PERL_MM_USE_DEFAULT} = 1;
+		$del_makefile_on_exit = 1;
+		#last statement not guarenteed to be true, see XSLoader Makefile.PL
+		do '.\Makefile.PL';
+		$del_makefile_on_exit = 0;
+	    }
+	    if($@) {
+		my $err = $@;
+		#make sure next build attempt/run of make_ext.pl doesn't succeed
+		#even though Makefile.PL failed
+		_unlink($makefile);
+		die "Unsuccessful Makefile.PL($ext_dir): error=$err";
+	    }
 
-	# Right. The reason for this little hack is that we're sitting inside
-	# a program run by ./miniperl, but there are tasks we need to perform
-	# when the 'realclean', 'distclean' or 'veryclean' targets are run.
-	# Unfortunately, they can be run *after* 'clean', which deletes
-	# ./miniperl
-	# So we do our best to leave a set of instructions identical to what
-	# we would do if we are run directly as 'realclean' etc
-	# Whilst we're perfect, unfortunately the targets we call are not, as
-	# some of them rely on a $(PERL) for their own distclean targets.
-	# But this always used to be a problem with the old /bin/sh version of
-	# this.
-	if (IS_UNIX) {
-	    foreach my $clean_target ('realclean', 'veryclean') {
-                fallback_cleanup($return_dir, $clean_target, <<"EOS");
+	    # Right. The reason for this little hack is that we're sitting inside
+	    # a program run by ./miniperl, but there are tasks we need to perform
+	    # when the 'realclean', 'distclean' or 'veryclean' targets are run.
+	    # Unfortunately, they can be run *after* 'clean', which deletes
+	    # ./miniperl
+	    # So we do our best to leave a set of instructions identical to what
+	    # we would do if we are run directly as 'realclean' etc
+	    # Whilst we're perfect, unfortunately the targets we call are not, as
+	    # some of them rely on a $(PERL) for their own distclean targets.
+	    # But this always used to be a problem with the old /bin/sh version of
+	    # this.
+	    if (IS_UNIX) {
+		foreach my $clean_target ('realclean', 'veryclean') {
+		    fallback_cleanup($return_dir, $clean_target, <<"EOS");
 cd $ext_dir
 if test ! -f Makefile -a -f Makefile.old; then
     echo "Note: Using Makefile.old"
@@ -552,32 +561,41 @@ else
 fi
 cd $return_dir
 EOS
+		}
 	    }
 	}
-    }
 
-    if (not -f $makefile) {
-	print "Warning: No Makefile!\n";
-    }
+	if (not -f $makefile) {
+	    print "Warning: No Makefile!\n";
+	}
 
-    if (IS_VMS) {
-	_quote_args($pass_through);
-	@$pass_through = (
-			  "/DESCRIPTION=$makefile",
-			  '/MACRO=(' . join(',',@$pass_through) . ')'
-			 );
-    }
+	if (IS_VMS) {
+	    _quote_args($pass_through);
+	    @$pass_through = (
+			      "/DESCRIPTION=$makefile",
+			      '/MACRO=(' . join(',',@$pass_through) . ')'
+			     );
+	}
+
+	my @targ = ($target, @$pass_through);
+	print "Making $target in $ext_dir\n@make @targ\n" if $verbose;
+	local $ENV{PERL_INSTALL_QUIET} = 1;
+	my $code = system(@make, @targ);
+	if($code >> 8 != 0){ # probably cleaned itself, try again once more time
+	    $code = system(@make, @targ);
+	}
+	die "Unsuccessful make($ext_dir): code=$code" if $code != 0;
 
-    my @targ = ($target, @$pass_through);
-    print "Making $target in $ext_dir\n@make @targ\n" if $verbose;
-    local $ENV{PERL_INSTALL_QUIET} = 1;
-    my $code = system(@make, @targ);
-    if($code >> 8 != 0){ # probably cleaned itself, try again once more time
-        $code = system(@make, @targ);
+	chdir $return_dir || die "Cannot cd to $return_dir: $!";
     }
-    die "Unsuccessful make($ext_dir): code=$code" if $code != 0;
 
-    chdir $return_dir || die "Cannot cd to $return_dir: $!";
+    END {
+	if($del_makefile_on_exit) { #exit() called, probably non-zero, delete
+	    #makefile so next run of make_ext.pl doesn't succeed
+	    _unlink($makefile);
+	    die "Unsuccessful Makefile.PL($ext_dir): error=exit($?) ";
+	}
+    }
 }
 
 sub _quote_args {
-- 
1.8.0.msysgit.0

@p5pRT
Copy link
Author

p5pRT commented Nov 23, 2015

From @jkeenan

On Thu Nov 19 21​:24​:00 2015, bulk88 wrote​:

This is a bug report for perl from bulk88@​hotmail.com,
generated with the help of perlbug 1.40 running under perl 5.23.5.

-----------------------------------------------------------------
[Please describe your issue here]

..\miniperl.exe -I..\lib ..\make_ext.pl "MAKE=dmake" --dir=..\cpan
--dir=..\dist --dir=..\ext --dynamic !Unicode/Normalize

The patch, which touches only one file, is over 500 lines along -- but my quick scan of it suggests that many of the changes are whitespace only. That makes it more difficult to review.

Would it be possible to re-draw the patch against blead without making any whitespace changes?

(I'm not opposed to tidying the code, but because the file being revised is not well known, I'd like to get discussion of the changes going before worrying about tidying.)

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Nov 23, 2015

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Nov 28, 2015

From @bulk88

On Mon Nov 23 15​:46​:49 2015, jkeenan wrote​:

Would it be possible to re-draw the patch against blead without making
any whitespace changes?

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Nov 28, 2015

From @bulk88

0001-make_ext.pl-run-all-Makefile.PLes-in-1-process.patch
From 69dd64f723b005a1f67e9bf8b83bab953e14b9f4 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Fri, 27 Nov 2015 17:39:36 -0500
Subject: [PATCH] make_ext.pl: run all Makefile.PLes in 1 process

This saves 13 wall seconds for "..\miniperl.exe -I..\lib ..\make_ext.pl
"MAKE=dmake" --dir=..\cpan --dir=..\dist --dir=..\ext --dynamic
!Unicode/Normalize" to complete on Win32. This patch catches all exit()
calls from Makefile.PLs, including exit(0) but no core modules do exit(0)
as normal flow in their Makefile.PLs. It also catches exceptions too. See
Perl RT ticket for details.
---
 make_ext.pl | 56 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/make_ext.pl b/make_ext.pl
index 223f67e..b01e59a 100644
--- a/make_ext.pl
+++ b/make_ext.pl
@@ -258,8 +258,12 @@ foreach my $spec (@extspec)  {
 		    [@pass_through, @{$extra_passthrough{$spec} || []}]);
 }
 
-sub build_extension {
-    my ($ext_dir, $perl, $mname, $target, $pass_through) = @_;
+{
+    my($del_makefile_on_exit, $ext_dir, $makefile);
+
+sub build_extension { #build_extensions is not recursion safe
+    $ext_dir = shift;
+    my ($perl, $mname, $target, $pass_through) = @_;
 
     unless (chdir "$ext_dir") {
 	warn "Cannot cd to $ext_dir: $!";
@@ -273,7 +277,7 @@ sub build_extension {
     my $return_dir = $up;
     my $lib_dir = "$up/lib";
 
-    my ($makefile, $makefile_no_minus_f);
+    my ($makefile_no_minus_f);
     if (IS_VMS) {
 	$makefile = 'descrip.mms';
 	if ($target =~ /clean$/
@@ -505,25 +509,30 @@ EOM
         # We are going to have to use Makefile.PL:
 	print "\nRunning Makefile.PL in $ext_dir\n" if $verbose;
 
-	my @args = ("-I$lib_dir", 'Makefile.PL');
-	if (IS_VMS) {
-	    my $libd = VMS::Filespec::vmspath($lib_dir);
-	    push @args, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
-	} else {
-	    push @args, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
-		'INSTALLMAN3DIR=none';
+	my $err;
+	{
+	    local @ARGV = (); #fake a system($^X) for perf
+	    if (IS_VMS) {
+		my $libd = VMS::Filespec::vmspath($lib_dir);
+		push @ARGV, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
+	    } else {
+		push @ARGV, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
+		    'INSTALLMAN3DIR=none';
+	    }
+	    push @ARGV, @$pass_through;
+	    print join(' ', @ARGV), "\n" if $verbose;
+	    local $ENV{PERL_MM_USE_DEFAULT} = 1;
+	    $del_makefile_on_exit = 1;
+	    #last statement not guarenteed to be true, see XSLoader Makefile.PL
+	    do '.\Makefile.PL';
+	    $del_makefile_on_exit = 0;
 	}
-	push @args, @$pass_through;
-	_quote_args(\@args) if IS_VMS;
-	print join(' ', $perl, @args), "\n" if $verbose;
-	my $code = do {
-	   local $ENV{PERL_MM_USE_DEFAULT} = 1;
-	    system $perl, @args;
-	};
-	if($code != 0){
+	if($@) {
+	    my $err = $@;
 	    #make sure next build attempt/run of make_ext.pl doesn't succeed
+	    #even though Makefile.PL failed
 	    _unlink($makefile);
-	    die "Unsuccessful Makefile.PL($ext_dir): code=$code";
+	    die "Unsuccessful Makefile.PL($ext_dir): error=$err";
 	}
 
 	# Right. The reason for this little hack is that we're sitting inside
@@ -580,6 +589,15 @@ EOS
     chdir $return_dir || die "Cannot cd to $return_dir: $!";
 }
 
+    END {
+	if($del_makefile_on_exit) { #exit() called, probably non-zero, delete
+	    #makefile so next run of make_ext.pl doesn't succeed
+	    _unlink($makefile);
+	    die "Unsuccessful Makefile.PL($ext_dir): error=exit($?) ";
+	}
+    }
+}
+
 sub _quote_args {
     my $args = shift; # must be array reference
 
-- 
1.9.5.msysgit.1

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2015

From @jkeenan

On Fri Nov 27 18​:23​:59 2015, bulk88 wrote​:

On Mon Nov 23 15​:46​:49 2015, jkeenan wrote​:

Would it be possible to re-draw the patch against blead without making
any whitespace changes?

Thanks for the revised patch. Unfortunately, when I applied the patch to a branch forked from blead at commit d61acc4 and then called, as I usually do, 'sh ./Configure -des -Dusedevel', 'make' failed. Transcript of 'make' attached.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2015

From @jkeenan

126686-make.typescript

@p5pRT
Copy link
Author

p5pRT commented Nov 30, 2015

From @bulk88

On Sun Nov 29 05​:11​:27 2015, jkeenan wrote​:

On Fri Nov 27 18​:23​:59 2015, bulk88 wrote​:

On Mon Nov 23 15​:46​:49 2015, jkeenan wrote​:

Would it be possible to re-draw the patch against blead without
making
any whitespace changes?

Thanks for the revised patch. Unfortunately, when I applied the patch
to a branch forked from blead at commit
d61acc4 and then called, as I usually
do, 'sh ./Configure -des -Dusedevel', 'make' failed. Transcript of
'make' attached.

Revised patch attached.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Nov 30, 2015

From @bulk88

0001-make_ext.pl-run-all-Makefile.PLes-in-1-process.patch
From 45141c2bfafd2e18dfb00d6feb88d3f6debf8995 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Mon, 30 Nov 2015 00:50:45 -0500
Subject: [PATCH] make_ext.pl: run all Makefile.PLes in 1 process

This saves 13 wall seconds for "..\miniperl.exe -I..\lib ..\make_ext.pl
"MAKE=dmake" --dir=..\cpan --dir=..\dist --dir=..\ext --dynamic
!Unicode/Normalize" to complete on Win32. This patch catches all exit()
calls from Makefile.PLs, including exit(0) but no core modules do exit(0)
as normal flow in their Makefile.PLs. It also catches exceptions too. See
Perl RT ticket for details.
---
 make_ext.pl | 56 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/make_ext.pl b/make_ext.pl
index 223f67e..575239d 100644
--- a/make_ext.pl
+++ b/make_ext.pl
@@ -258,8 +258,12 @@ foreach my $spec (@extspec)  {
 		    [@pass_through, @{$extra_passthrough{$spec} || []}]);
 }
 
-sub build_extension {
-    my ($ext_dir, $perl, $mname, $target, $pass_through) = @_;
+{
+    my($del_makefile_on_exit, $ext_dir, $makefile);
+
+sub build_extension { #build_extensions is not recursion safe
+    $ext_dir = shift;
+    my ($perl, $mname, $target, $pass_through) = @_;
 
     unless (chdir "$ext_dir") {
 	warn "Cannot cd to $ext_dir: $!";
@@ -273,7 +277,7 @@ sub build_extension {
     my $return_dir = $up;
     my $lib_dir = "$up/lib";
 
-    my ($makefile, $makefile_no_minus_f);
+    my ($makefile_no_minus_f);
     if (IS_VMS) {
 	$makefile = 'descrip.mms';
 	if ($target =~ /clean$/
@@ -505,25 +509,30 @@ EOM
         # We are going to have to use Makefile.PL:
 	print "\nRunning Makefile.PL in $ext_dir\n" if $verbose;
 
-	my @args = ("-I$lib_dir", 'Makefile.PL');
-	if (IS_VMS) {
-	    my $libd = VMS::Filespec::vmspath($lib_dir);
-	    push @args, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
-	} else {
-	    push @args, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
-		'INSTALLMAN3DIR=none';
+	my $err;
+	{
+	    local @ARGV = (); #fake a system($^X) for perf
+	    if (IS_VMS) {
+		my $libd = VMS::Filespec::vmspath($lib_dir);
+		push @ARGV, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
+	    } else {
+		push @ARGV, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
+		    'INSTALLMAN3DIR=none';
+	    }
+	    push @ARGV, @$pass_through;
+	    print join(' ', @ARGV), "\n" if $verbose;
+	    local $ENV{PERL_MM_USE_DEFAULT} = 1;
+	    $del_makefile_on_exit = 1;
+	    #last statement not guarenteed to be true, see XSLoader Makefile.PL
+	    do './Makefile.PL';
+	    $del_makefile_on_exit = 0;
 	}
-	push @args, @$pass_through;
-	_quote_args(\@args) if IS_VMS;
-	print join(' ', $perl, @args), "\n" if $verbose;
-	my $code = do {
-	   local $ENV{PERL_MM_USE_DEFAULT} = 1;
-	    system $perl, @args;
-	};
-	if($code != 0){
+	if($@) {
+	    my $err = $@;
 	    #make sure next build attempt/run of make_ext.pl doesn't succeed
+	    #even though Makefile.PL failed
 	    _unlink($makefile);
-	    die "Unsuccessful Makefile.PL($ext_dir): code=$code";
+	    die "Unsuccessful Makefile.PL($ext_dir): error=$err";
 	}
 
 	# Right. The reason for this little hack is that we're sitting inside
@@ -580,6 +589,15 @@ EOS
     chdir $return_dir || die "Cannot cd to $return_dir: $!";
 }
 
+    END {
+	if($del_makefile_on_exit) { #exit() called, probably non-zero, delete
+	    #makefile so next run of make_ext.pl doesn't succeed
+	    _unlink($makefile);
+	    die "Unsuccessful Makefile.PL($ext_dir): error=exit($?) ";
+	}
+    }
+}
+
 sub _quote_args {
     my $args = shift; # must be array reference
 
-- 
1.9.5.msysgit.1

@p5pRT
Copy link
Author

p5pRT commented Dec 1, 2015

From @jkeenan

On Mon Nov 30 01​:14​:58 2015, bulk88 wrote​:

On Sun Nov 29 05​:11​:27 2015, jkeenan wrote​:

On Fri Nov 27 18​:23​:59 2015, bulk88 wrote​:

On Mon Nov 23 15​:46​:49 2015, jkeenan wrote​:

Would it be possible to re-draw the patch against blead without
making
any whitespace changes?

Thanks for the revised patch. Unfortunately, when I applied the patch
to a branch forked from blead at commit
d61acc4 and then called, as I usually
do, 'sh ./Configure -des -Dusedevel', 'make' failed. Transcript of
'make' attached.

Revised patch attached.

This patch also experienced 'make' failures. On linux-x86_64 I forked a branch from blead at 4c29884 and applied https://rt-archive.perl.org/perl5/Ticket/Attachment/1377765/739931/0001-make_ext.pl-run-all-Makefile.PLes-in-1-process.patch to the branch. I then ran​:

sh ./Configure -des -Dusedevel && make

Full transcript, gzipped, attached. Excerpt from its tail​:

#####
make[1]​: Leaving directory `/home/jkeenan/gitwork/perl/dist/PathTools'
./miniperl -Ilib make_ext.pl lib/auto/Data/Dumper/Dumper.so MAKE="make" LIBPERL_A=libperl.a LINKTYPE=dynamic
Generating a Unix-style Makefile
Writing Makefile for Data​::Dumper
make[1]​: Entering directory `/home/jkeenan/gitwork/perl/dist/Data-Dumper'
Running Mkbootstrap for Data​::Dumper ()
chmod 644 "Dumper.bs"
"../../miniperl" "-I../../lib" "-I../../lib" "../../lib/ExtUtils/xsubpp" -typemap "../../lib/ExtUtils/typemap" Dumper.xs > Dumper.xsc && mv Dumper.xsc Dumper.c
cc -c -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite-strings -O2 -DVERSION=\"2.158\" -DXS_VERSION=\"2.158\" -fPIC "-I../.." Dumper.c
rm -f ../../lib/auto/Data/Dumper/Dumper.so
cc -shared -O2 -L/usr/local/lib -fstack-protector Dumper.o -o ../../lib/auto/Data/Dumper/Dumper.so \
  \
 
chmod 755 ../../lib/auto/Data/Dumper/Dumper.so
/home/jkeenan/gitwork/perl/dist/Data-Dumper/../../miniperl "-I../../lib" "-I../../lib" -MExtUtils​::Command​::MM -e 'cp_nonempty' -- Dumper.bs ../../lib/auto/Data/Dumper/Dumper.bs 644
make[1]​: Leaving directory `/home/jkeenan/gitwork/perl/dist/Data-Dumper'
./miniperl -Ilib make_ext.pl lib/auto/DB_File/DB_File.so MAKE="make" LIBPERL_A=libperl.a LINKTYPE=dynamic
Parsing config.in...
Looks Good.
Generating a Unix-style Makefile
Writing Makefile for DB_File
Unsuccessful Makefile.PL(cpan/DB_File)​: error=exit(0) at make_ext.pl line 596.
END failed--call queue aborted at make_ext.pl line 527.
make​: *** [lib/auto/DB_File/DB_File.so] Error 2
#####

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 1, 2015

@p5pRT
Copy link
Author

p5pRT commented Dec 1, 2015

From @bulk88

On Mon Nov 30 16​:59​:41 2015, jkeenan wrote​:

This patch also experienced 'make' failures. On linux-x86_64 I forked
a branch from blead at 4c29884 and
applied https://rt-archive.perl.org/perl5/Ticket/Attachment/1377765/739931/0001-
make_ext.pl-run-all-Makefile.PLes-in-1-process.patch to the branch. I
then ran​:

sh ./Configure -des -Dusedevel && make

Full transcript, gzipped, attached. Excerpt from its tail​:

#####
make[1]​: Leaving directory `/home/jkeenan/gitwork/perl/dist/PathTools'
./miniperl -Ilib make_ext.pl lib/auto/Data/Dumper/Dumper.so
MAKE="make" LIBPERL_A=libperl.a LINKTYPE=dynamic
Generating a Unix-style Makefile
Writing Makefile for Data​::Dumper
make[1]​: Entering directory `/home/jkeenan/gitwork/perl/dist/Data-
Dumper'
Running Mkbootstrap for Data​::Dumper ()
chmod 644 "Dumper.bs"
"../../miniperl" "-I../../lib" "-I../../lib"
"../../lib/ExtUtils/xsubpp" -typemap "../../lib/ExtUtils/typemap"
Dumper.xs > Dumper.xsc && mv Dumper.xsc Dumper.c
cc -c -fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall
-Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite-
strings -O2 -DVERSION=\"2.158\" -DXS_VERSION=\"2.158\" -fPIC "-
I../.." Dumper.c
rm -f ../../lib/auto/Data/Dumper/Dumper.so
cc -shared -O2 -L/usr/local/lib -fstack-protector Dumper.o -o
../../lib/auto/Data/Dumper/Dumper.so \
\

chmod 755 ../../lib/auto/Data/Dumper/Dumper.so
/home/jkeenan/gitwork/perl/dist/Data-Dumper/../../miniperl "-
I../../lib" "-I../../lib" -MExtUtils​::Command​::MM -e 'cp_nonempty' --
Dumper.bs ../../lib/auto/Data/Dumper/Dumper.bs 644
make[1]​: Leaving directory `/home/jkeenan/gitwork/perl/dist/Data-
Dumper'
./miniperl -Ilib make_ext.pl lib/auto/DB_File/DB_File.so MAKE="make"
LIBPERL_A=libperl.a LINKTYPE=dynamic
Parsing config.in...
Looks Good.
Generating a Unix-style Makefile
Writing Makefile for DB_File
Unsuccessful Makefile.PL(cpan/DB_File)​: error=exit(0) at make_ext.pl
line 596.
END failed--call queue aborted at make_ext.pl line 527.
make​: *** [lib/auto/DB_File/DB_File.so] Error 2
#####

Revised again. The last patch I smoked on a Unix OS but it apparently didn't have libdb so DB_File was never attempted to be built on it per the logic in FindExt.pm.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Dec 1, 2015

From @bulk88

0001-make_ext.pl-run-all-Makefile.PLes-in-1-process.patch
From f0267747fcc38be985926dc44bfa97fe94c1806d Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Tue, 1 Dec 2015 16:44:12 -0500
Subject: [PATCH] make_ext.pl: run all Makefile.PLes in 1 process

This saves 13 wall seconds for "..\miniperl.exe -I..\lib ..\make_ext.pl
"MAKE=dmake" --dir=..\cpan --dir=..\dist --dir=..\ext --dynamic
!Unicode/Normalize" to complete on Win32. This patch catches all exit()
calls from Makefile.PLs, including exit(0). While exit(0) is techincally
legal for Makefile.PLs to do, it is rare, nearly all Makefile.PLs fall off
the end when they are done. So exit(0) for core purposes is really an error
since the build will prematurly stop. Fix the 1 case where exit(0) was
called.  This patch also catches exceptions from the Makefile.PL.
See Perl RT ticket for details.
---
 cpan/DB_File/Makefile.PL |  3 ---
 make_ext.pl              | 56 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/cpan/DB_File/Makefile.PL b/cpan/DB_File/Makefile.PL
index f5ef7c8..f203deb 100644
--- a/cpan/DB_File/Makefile.PL
+++ b/cpan/DB_File/Makefile.PL
@@ -147,9 +147,6 @@ else {
       or die "Can't copy fallback.xs to constants.xs: $!";
 }
 
-exit;
-
-
 sub MY::libscan
 {
     my $self = shift ;
diff --git a/make_ext.pl b/make_ext.pl
index 223f67e..575239d 100644
--- a/make_ext.pl
+++ b/make_ext.pl
@@ -258,8 +258,12 @@ foreach my $spec (@extspec)  {
 		    [@pass_through, @{$extra_passthrough{$spec} || []}]);
 }
 
-sub build_extension {
-    my ($ext_dir, $perl, $mname, $target, $pass_through) = @_;
+{
+    my($del_makefile_on_exit, $ext_dir, $makefile);
+
+sub build_extension { #build_extensions is not recursion safe
+    $ext_dir = shift;
+    my ($perl, $mname, $target, $pass_through) = @_;
 
     unless (chdir "$ext_dir") {
 	warn "Cannot cd to $ext_dir: $!";
@@ -273,7 +277,7 @@ sub build_extension {
     my $return_dir = $up;
     my $lib_dir = "$up/lib";
 
-    my ($makefile, $makefile_no_minus_f);
+    my ($makefile_no_minus_f);
     if (IS_VMS) {
 	$makefile = 'descrip.mms';
 	if ($target =~ /clean$/
@@ -505,25 +509,30 @@ EOM
         # We are going to have to use Makefile.PL:
 	print "\nRunning Makefile.PL in $ext_dir\n" if $verbose;
 
-	my @args = ("-I$lib_dir", 'Makefile.PL');
-	if (IS_VMS) {
-	    my $libd = VMS::Filespec::vmspath($lib_dir);
-	    push @args, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
-	} else {
-	    push @args, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
-		'INSTALLMAN3DIR=none';
+	my $err;
+	{
+	    local @ARGV = (); #fake a system($^X) for perf
+	    if (IS_VMS) {
+		my $libd = VMS::Filespec::vmspath($lib_dir);
+		push @ARGV, "INST_LIB=$libd", "INST_ARCHLIB=$libd";
+	    } else {
+		push @ARGV, 'INSTALLDIRS=perl', 'INSTALLMAN1DIR=none',
+		    'INSTALLMAN3DIR=none';
+	    }
+	    push @ARGV, @$pass_through;
+	    print join(' ', @ARGV), "\n" if $verbose;
+	    local $ENV{PERL_MM_USE_DEFAULT} = 1;
+	    $del_makefile_on_exit = 1;
+	    #last statement not guarenteed to be true, see XSLoader Makefile.PL
+	    do './Makefile.PL';
+	    $del_makefile_on_exit = 0;
 	}
-	push @args, @$pass_through;
-	_quote_args(\@args) if IS_VMS;
-	print join(' ', $perl, @args), "\n" if $verbose;
-	my $code = do {
-	   local $ENV{PERL_MM_USE_DEFAULT} = 1;
-	    system $perl, @args;
-	};
-	if($code != 0){
+	if($@) {
+	    my $err = $@;
 	    #make sure next build attempt/run of make_ext.pl doesn't succeed
+	    #even though Makefile.PL failed
 	    _unlink($makefile);
-	    die "Unsuccessful Makefile.PL($ext_dir): code=$code";
+	    die "Unsuccessful Makefile.PL($ext_dir): error=$err";
 	}
 
 	# Right. The reason for this little hack is that we're sitting inside
@@ -580,6 +589,15 @@ EOS
     chdir $return_dir || die "Cannot cd to $return_dir: $!";
 }
 
+    END {
+	if($del_makefile_on_exit) { #exit() called, probably non-zero, delete
+	    #makefile so next run of make_ext.pl doesn't succeed
+	    _unlink($makefile);
+	    die "Unsuccessful Makefile.PL($ext_dir): error=exit($?) ";
+	}
+    }
+}
+
 sub _quote_args {
     my $args = shift; # must be array reference
 
-- 
1.9.5.msysgit.1

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2015

From @jkeenan

On Tue Dec 01 14​:21​:26 2015, bulk88 wrote​:

On Mon Nov 30 16​:59​:41 2015, jkeenan wrote​:

This patch also experienced 'make' failures. On linux-x86_64 I
forked
a branch from blead at 4c29884 and
applied https://rt-archive.perl.org/perl5/Ticket/Attachment/1377765/739931/0001-
make_ext.pl-run-all-Makefile.PLes-in-1-process.patch to the branch.
I
then ran​:

sh ./Configure -des -Dusedevel && make

Full transcript, gzipped, attached. Excerpt from its tail​:

#####
make[1]​: Leaving directory
`/home/jkeenan/gitwork/perl/dist/PathTools'
./miniperl -Ilib make_ext.pl lib/auto/Data/Dumper/Dumper.so
MAKE="make" LIBPERL_A=libperl.a LINKTYPE=dynamic
Generating a Unix-style Makefile
Writing Makefile for Data​::Dumper
make[1]​: Entering directory `/home/jkeenan/gitwork/perl/dist/Data-
Dumper'
Running Mkbootstrap for Data​::Dumper ()
chmod 644 "Dumper.bs"
"../../miniperl" "-I../../lib" "-I../../lib"
"../../lib/ExtUtils/xsubpp" -typemap "../../lib/ExtUtils/typemap"
Dumper.xs > Dumper.xsc && mv Dumper.xsc Dumper.c
cc -c -fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall
-Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite-
strings -O2 -DVERSION=\"2.158\" -DXS_VERSION=\"2.158\" -fPIC "-
I../.." Dumper.c
rm -f ../../lib/auto/Data/Dumper/Dumper.so
cc -shared -O2 -L/usr/local/lib -fstack-protector Dumper.o -o
../../lib/auto/Data/Dumper/Dumper.so \
\

chmod 755 ../../lib/auto/Data/Dumper/Dumper.so
/home/jkeenan/gitwork/perl/dist/Data-Dumper/../../miniperl "-
I../../lib" "-I../../lib" -MExtUtils​::Command​::MM -e 'cp_nonempty' --
Dumper.bs ../../lib/auto/Data/Dumper/Dumper.bs 644
make[1]​: Leaving directory `/home/jkeenan/gitwork/perl/dist/Data-
Dumper'
./miniperl -Ilib make_ext.pl lib/auto/DB_File/DB_File.so MAKE="make"
LIBPERL_A=libperl.a LINKTYPE=dynamic
Parsing config.in...
Looks Good.
Generating a Unix-style Makefile
Writing Makefile for DB_File
Unsuccessful Makefile.PL(cpan/DB_File)​: error=exit(0) at make_ext.pl
line 596.
END failed--call queue aborted at make_ext.pl line 527.
make​: *** [lib/auto/DB_File/DB_File.so] Error 2
#####

Revised again. The last patch I smoked on a Unix OS but it apparently
didn't have libdb so DB_File was never attempted to be built on it per
the logic in FindExt.pm.

This version of the patch built successfully. It is now smoking in this branch​:

smoke-me/jkeenan/bulk88/126686-make-ext-3

I'll leave to others the questions​: (a) whether it speeds up 'make' on both Windows and other OSes; and (b) whether it should be applied.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jan 4, 2016

From @bulk88

bump

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented May 16, 2016

From @tonycoz

On Tue Dec 01 14​:21​:26 2015, bulk88 wrote​:

!Unicode/Normalize" to complete on Win32. This patch catches all exit() calls
from Makefile.PLs, including exit(0). While exit(0) is techincally legal for
Makefile.PLs to do, it is rare, nearly all Makefile.PLs fall off the end when
they are done. So exit(0) for core purposes is really an error since the
build will prematurly stop. Fix the 1 case where exit(0) was called. This
patch also catches exceptions from the Makefile.PL. See Perl RT ticket for
details.

That's pretty questionable.

DB_File is CPAN upstream, I'm not sure it's worth breaking its Makefile.PL for a small build-time optimization.

There is a build time advantage on Linux - full make -j6 build time (not include Configure) on a 4 core CPU​:

before​:

real 1m16.346s
user 3m54.684s
sys 0m11.048s

real 1m13.703s
user 3m54.704s
sys 0m10.884s

real 1m10.914s
user 3m56.012s
sys 0m10.872s

after​:

real 1m10.237s
user 3m56.036s
sys 0m10.676s

real 1m8.678s
user 3m54.780s
sys 0m11.132s

real 1m10.573s
user 3m55.748s
sys 0m10.868s

Tony

@toddr
Copy link
Member

toddr commented Feb 13, 2020

@bulk88 Given this is not a bug but a minor build optimization, I'm going to suggest that further revisions of this idea be moved to a pull request.

@toddr toddr closed this as completed Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants