On Tue, Dec 22, 2009 at 2:23 PM, Michael G Schwern <schw...@pobox.com> wrote:
> Joshua ben Jore wrote:
>>
>> The just-released EU::MM 6.56 repeats this pattern frequently:
>>
>> open my($fh), '>', ...
>> or croak("Can't open ... for writing: $!");
>> ...
>> print $fh ...; # <<< no error checking
>> close $fh ...; # <<< no error checking
>>
>> Grepping for code:
>>
>> egrep -rnH 'print '\\'$[^ ;]+ ' ExtUtils-MakeMaker-6.56
>>
>> Where is EU::MM's version control? I'd like to submit a patch adding a
>> bunch of:
>>
>> or croak("Can't write '$fn': $!");
>
> Putting in error checking at some key locations is fine, but the prospect of
> adding `or die "Can't print $foo: $!"` to every print and close makes me
> want to claw my eyeballs out. Its unfortunate MakeMaker can't rely on
> autodie and that autodie can't override print.
>
> Since almost nothing checks for print and close failure, and I don't blame
> them its a pain in the ass, out of disk is a veritable minefield of silent
> errors. Scattering error checking inside MakeMaker is sort of like checking
> the ground in front of you and then merrily marching forward. Even full
> error checking inside MakeMaker will still leave you to blow up somewhere
> else, probably in the tests.
>
> Fortunately there is one key location where MakeMaker does almost all its
> writing to disk, ExtUtils::MakeMaker->flush. A simple patch to check that
> would be lovely. If you want to write and insert a safe _print() wrapper
> method and scatter that around that's fine, too.
I already clawed my eyes out years ago. In the middle of this, note
ae5f97d4cf9f314a96d68af2b6fcc6ee1c7cad26 which checks
ExtUtils::MakeMaker->flush. Pushed the below to
http://github.com/jbenjore/extutils-makemaker.
614c15386d79b9d61548007a1b09676017bb5416 Check print()/close() for t/pm_to_blib
bdc102acf0356486a64b9b02f33841f9c4e8cade Check print()/close() for
inc/Test::Builder::_print_to_fh
0304be893b772a904aa141544142026582ba2157 Check print()/close() for
t/fixin.t test_fixin
191edabb511e73cc5e8d3ef41aeb5b924e4c385a Check print()/close() for
EU::Mksymlists::_write_vms
313a354cf9c22c689cb0e163508aeec37eaa3890 Check print()/close() for
EU::Mksymlists::_write_win32
5bf29ac7887d2253c78b29a72f54dadb75658499 Check print()/close() for
EU::Mksymlists::_write_os2
e6b91b1ddc75a08a41dd5e09409377a8b2d29892 Check print()/close() for
EU::Mksymlists::_write_aix
04dd05a36c03b8c697716c7b5d779a71ebcf9814 Check print()/close() for
EU::Mkbootstrap::Mkbootstrap
ae5f97d4cf9f314a96d68af2b6fcc6ee1c7cad26 Check print()/close() for
EU::MakeMaker::flush
f91926398be56d57248293c88c8e28c333189d22 Check print()/close() for
EU::MakeMaker::WriteEmptyMakefile
0a6516dd6d12e39a25f1d9cd3340e3be41ba28e3 Check print()/close() for
EU::Packlist::write
9a56631148de3db096f938c8edaef4c519bf57b0 Check print()/close() for
EU::Mkbootstrap::Mkbootstrap
077292cbe05e8c9ae6b9924c969a5c5b8293bcf5 TODO: Check `` for
EU::MM_VMS::find_perl
67965dbc09a2f867d0b30fb80aa1dccab2af9fed Check print()/close() for
EU::MM_VMS::find_perl
84f52eaf7b113859e836ab141dc925f6d3ed6ec8 Check print()/close() for
EU::MM_Unix::fixin
5b045f97a568b54b7a9b6c264f192eb5eef5b72d Check print()/close() for
EU::MM_OS2::fixin
> MakeMaker's version control is listed in the META.yml and on
> search.cpan.org.
I looked in
http://search.cpan.org/dist/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm#AUTHORS
and found no mention of a repository so I assume it was as-yet
undocumented.
> Does the CPAN testers code have out of disk checks? If not, before checking
> each distribution it could try to write a decently sized file to the build
> directory to check. If it fails, halt the build and inform the admin.
I also wonder if the testers are checking the success of the `tar xzf
Blah-0.01.tar.gz`. Maybe that failed silently.
Josh
