Author: dagolden
Date: Thu Sep  3 04:43:06 2009
New Revision: 13264

Modified:
   Module-Build/trunk/t/lib/DistGen.pm
   Module-Build/trunk/t/lib/MBTest.pm

Log:
Fixed subtle MBTest->tmpdir bug for use with DistGen

Under $ENV{PERL_CORE}, each call to MBTest->tmpdir was using *current*
cwd() as a base directory for temporary directories with File::Temp.
This is wrong because DistGen can potentially call tmpdir multiple
times in a test file, sometimes from within other temporary
directories, causing nesting of temporary directories (with obvious
complications for deletion).

I fixed MBTest->tmpdir to use the *original* cwd from when MBTest was
loaded.  With that, we can go back to relying on File::Temp to do
proper tmpdir cleanup, and the recent hackish cleanup code has been
removed from DistGen.

DistGen has an END block to do a multi-volume-safe chdir back to the
original cwd, which means tests no longer need to worry about changing
back to the original directory themselves or calling $dist->remove to
change directories and cleanup there.  Thus, test code has fewer
"cleanup" type calls, and cleanup just happens as needed at the end.



Modified: Module-Build/trunk/t/lib/DistGen.pm
==============================================================================
--- Module-Build/trunk/t/lib/DistGen.pm (original)
+++ Module-Build/trunk/t/lib/DistGen.pm Thu Sep  3 04:43:06 2009
@@ -72,52 +72,43 @@
 
 ########################################################################
 
-{
-  my @CLEANUP_DIRS;
+END { chdir_all(MBTest->original_cwd); }
 
-  END {
-    chdir_all(MBTest->original_cwd);
-    File::Path::rmtree($_) for @CLEANUP_DIRS;
-  }
-
-  sub new {
-    my $self = bless {}, shift;
-    $self->reset(@_);
-  }
-
-  sub reset {
-    my $self = shift;
-    my %options = @_;
+sub new {
+  my $self = bless {}, shift;
+  $self->reset(@_);
+}
 
-    $options{name} ||= 'Simple';
-    $options{dir}  ||= MBTest->tmpdir( CLEANUP => 0 );
+sub reset {
+  my $self = shift;
+  my %options = @_;
 
-    my %data = (
-      no_manifest   => 0,
-      xs            => 0,
-      %options,
-    );
-    %$self = %data;
+  $options{name} ||= 'Simple';
+  $options{dir} = File::Spec->rel2abs(
+    defined $options{dir} ? $options{dir} : MBTest->tmpdir
+  );
 
-    # So we can clean up later even if the caller chdir()s
-    $self->{dir} = File::Spec->rel2abs($self->{dir});
-    push @CLEANUP_DIRS, $self->{dir};
+  my %data = (
+    no_manifest   => 0,
+    xs            => 0,
+    %options,
+  );
+  %$self = %data;
 
-    tie %{$self->{filedata}}, 'Tie::CPHash';
+  tie %{$self->{filedata}}, 'Tie::CPHash';
 
-    tie %{$self->{pending}{change}}, 'Tie::CPHash';
+  tie %{$self->{pending}{change}}, 'Tie::CPHash';
 
-    # start with a fresh, empty directory
-    if ( -d $self->dirname ) {
-      warn "Warning: Removing existing directory '@{[$self->dirname]}'\n";
-      File::Path::rmtree( $self->dirname );
-    }
-    File::Path::mkpath( $self->dirname );
+  # start with a fresh, empty directory
+  if ( -d $self->dirname ) {
+    warn "Warning: Removing existing directory '@{[$self->dirname]}'\n";
+    File::Path::rmtree( $self->dirname );
+  }
+  File::Path::mkpath( $self->dirname );
 
-    $self->_gen_default_filedata();
+  $self->_gen_default_filedata();
 
-    return $self;
-  }
+  return $self;
 }
 
 sub remove {

Modified: Module-Build/trunk/t/lib/MBTest.pm
==============================================================================
--- Module-Build/trunk/t/lib/MBTest.pm  (original)
+++ Module-Build/trunk/t/lib/MBTest.pm  Thu Sep  3 04:43:06 2009
@@ -103,7 +103,7 @@
 
 # always return to the current directory
 { 
-  my $cwd = Cwd::cwd;
+  my $cwd = File::Spec->rel2abs(Cwd::cwd);
 
   sub original_cwd { return $cwd }
 
@@ -123,10 +123,10 @@
 }
 ########################################################################
 
-# Setup a temp directory 
-sub tmpdir { 
+# Setup a temp directory
+sub tmpdir {
   my ($self, @args) = @_;
-  my $dir = $ENV{PERL_CORE} ? Cwd::cwd : File::Spec->tmpdir;
+  my $dir = $ENV{PERL_CORE} ? MBTest->original_cwd : File::Spec->tmpdir;
   return File::Temp::tempdir('MB-XXXXXXXX', CLEANUP => 1, DIR => $dir, @args);
 }
 

Reply via email to