Bug #48813 for bioperl: Win32 compatibility PATCH

Tue Aug 18 15:08:49 2009 kmx [...] cpan.org - Ticket created

Subject:

Win32 compatibility PATCH

Hi, please find enclosed simple patch that fixes a failing test t/SeqFeature/SeqFeatCollection.t when installing Bio::Perl on Win32/strawberry. The problem was that File::Temp->new() (called in sub test_output_file) returns an open filehandle to temporary file and it is still open even after return from sub test_output_file. You later in the code unlink this tempfile; however Windows do not allow unlinking an open filehandle - and it was the cause of the problems. So I simply close the temporary filehandle and left the created tempfile on the filesystem (you are handling keeping/deleting these files yourself). -- kmx --- BioPerl-1.6.0/Bio/Root/Test.pm +++ BioPerl-1.6.0patched/Bio/Root/Test.pm @@ -299,10 +299,12 @@ sub test_output_file { die "test_output_file takes no args\n" if @_; - my $tmp = File::Temp->new(); + my $tmp = File::Temp->new(UNLINK => 0); push(@TEMP_FILES, $tmp); + my $fname = $tmp->filename; + close($tmp); - return $tmp->filename; + return $fname; }

Tue Aug 18 16:38:16 2009 cjfields [...] uiuc.edu - Correspondence added

On Tue Aug 18 15:08:49 2009, KMX wrote: Show quoted text

> Hi, > > please find enclosed simple patch that fixes a failing test > t/SeqFeature/SeqFeatCollection.t when installing Bio::Perl on > Win32/strawberry. > > The problem was that File::Temp->new() (called in sub test_output_file) > returns an open filehandle to temporary file and it is still open even > after return from sub test_output_file. You later in the code unlink > this tempfile; however Windows do not allow unlinking an open filehandle > - and it was the cause of the problems. > > So I simply close the temporary filehandle and left the created tempfile > on the filesystem (you are handling keeping/deleting these files yourself). > > -- > kmx > > --- BioPerl-1.6.0/Bio/Root/Test.pm > +++ BioPerl-1.6.0patched/Bio/Root/Test.pm > @@ -299,10 +299,12 @@ > sub test_output_file { > die "test_output_file takes no args\n" if @_; > > - my $tmp = File::Temp->new(); > + my $tmp = File::Temp->new(UNLINK => 0); > push(@TEMP_FILES, $tmp); > + my $fname = $tmp->filename; > + close($tmp); > > - return $tmp->filename; > + return $fname; > }

The problem is we have tons of temp files made using this method, so leaving them present on the user's system is untenable. However, the above works if you close the filehandle but allow unlinking (UNLINK=>1, which is default). Can you test to see if it works? If so we can get rid of @TEMP_FILES (which isn't used AFAICT); if not we can use @TEMP_FILES and unlink everything in an END block in Bio::Root::Test. cjfields

Tue Aug 18 16:38:17 2009 The RT System itself - Status changed from 'new' to 'open'

Wed Aug 19 16:08:57 2009 kmx [...] cpan.org - Correspondence added

Show quoted text

> However, the above works if you close the filehandle but allow > unlinking (UNLINK=>1, which is default). Can you test to see > if it works? If so we can get rid of @TEMP_FILES (which isn't > used AFAICT); if not we can use @TEMP_FILES and unlink everything in > an END block in Bio::Root::Test.

You are absolutely right. The following patch is enough: ***** diff -r -u BioPerl-1.6.0/Bio/Root/Test.pm BioPerl-1.6.0patched/Bio/Root/Test.pm --- BioPerl-1.6.0/Bio/Root/Test.pm +++ BioPerl-1.6.0patched/Bio/Root/Test.pm @@ -301,6 +301,7 @@ my $tmp = File::Temp->new(); push(@TEMP_FILES, $tmp); + close($tmp); #Win32 needs this return $tmp->filename; } **** -- kmx

Wed Aug 19 16:17:58 2009 cjfields [...] uiuc.edu - Correspondence added

Patch committed to bioperl svn in r15990. thanks! -cjfields On Wed Aug 19 16:08:57 2009, KMX wrote: Show quoted text

> > However, the above works if you close the filehandle but allow > > unlinking (UNLINK=>1, which is default). Can you test to see > > if it works? If so we can get rid of @TEMP_FILES (which isn't > > used AFAICT); if not we can use @TEMP_FILES and unlink everything in > > an END block in Bio::Root::Test.

> > You are absolutely right. The following patch is enough: > > ***** > diff -r -u BioPerl-1.6.0/Bio/Root/Test.pm > BioPerl-1.6.0patched/Bio/Root/Test.pm > --- BioPerl-1.6.0/Bio/Root/Test.pm > +++ BioPerl-1.6.0patched/Bio/Root/Test.pm > @@ -301,6 +301,7 @@ > > my $tmp = File::Temp->new(); > push(@TEMP_FILES, $tmp); > + close($tmp); #Win32 needs this > > return $tmp->filename; > } > **** > > -- > kmx >

Wed Aug 19 16:18:00 2009 cjfields [...] uiuc.edu - Status changed from 'open' to 'resolved'