-- David
On Mon, Dec 21, 2009 at 4:02 PM, Joshua ben Jore <twi...@gmail.com> wrote:
> The CPAN smokers regularly spit back UNKNOWN results where the text of
> the error is:
>
> Output from '/usr/bin/make':
>
> make: *** No targets specified and no makefile found. Stop.
>
> One example is http://www.nntp.perl.org/group/perl.cpan.testers/2009/07/msg4616138.html
> but there are many, many others. Does anyone know what's going on
> here?
>
> Josh
>
Ran out of diskspace I would guess.
--
Chris Williams
aka BinGOs
PGP ID 0x4658671F
http://www.gumbynet.org.uk
==========================
Can the smoker detect that, and bail appropriately?
(I suspect that df isn't portable, but likely trying to create a 2 meg file
in the working directory with a known pattern for contents,, read it back
correctly, and then delete it, should be)
Nicholas Clark
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': $!");
Josh
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
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.
MakeMaker's version control is listed in the META.yml and on search.cpan.org.
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.
--
Stabbing you in the face for your own good.
The problem with this approach, and I'm sorry you did all that work, is that
it complicates the code and it'll won't be maintained without effort. The
next patch will forget to include the cumbersome "or die ..." logic. When you
have to scatter duplicated code all over to warn about an unrecoverable
failure in the surrounding environment... well hell, when you have to scatter
duplicated code all over that's bad, full stop.
I'm very sorry but I have to reject most of that work. I feel like a dick for
doing it.
A little experimentation with a small disk image shows that close() will error
if there's no disk left. No need to check every print. And a close() wrapper
is trivial. It does mean there needs to be a close() for every open() which
is annoying but less so. Though I may have found a non-invasive way for
autodie to take care of that, it doesn't help us.
https://rt.cpan.org/Ticket/Display.html?id=52985
I'm not going to ask you to write up that patch, I'll do it myself unless
somebody gets to it first. I probably won't have time to touch it until
tomorrow if you're feeling frisky.
>> 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.
Just letting you know that search.cpan.org tracks that stuff now for future
use. I'll add a link to the POD. Thanks for mentioning it.
--
170. Not allowed to "defect" to OPFOR during training missions.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
> 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.
Would *not* fixing bugs in EU::MM hasten its demise? I can get behind that
strategy.
-- c
You have to check every print. Most prints will just extend the buffer
but some prints will flush the buffer and return a real
success/failure. You can't predict in advance unless you can predict
that you won't have printed a buffer's full.
The repeated "or croak" or "or die" is unfortunate and could easily be
refactored. I find it easy to resist that urge withing EU::MM because
it is so important and avoiding breaking it is so incredibly
important.
It would be reasonable to take the diff I've generated to generate a
new diff reading something like:
$fh->print( ... );
You've got full freedom to assert some policies like "we'll complain
bitterly if we run out of disk."
Josh
Incidentally, I wrote a fix to automatically transform print and close
into their "or die" alternatives 5+ years ago at
http://perlmonks.org/?node_id=367478. This is out of line for EU::MM
but autodie could become smart enough to use this idea.
Josh
But when print() or flush() fails, doesn't that put the filehandle into a
state where close() will always fail?
From the perldocs for close():
Closes the file or pipe associated with the file handle, flushes the IO
buffers, and closes the system file descriptor. Returns true if those
operations have succeeded and if no error was reported by any PerlIO
layer.
If flushing the IO buffers fails once because the disk is full, I don't expect
that it would succeed in the future, right?
I've been using the technique of skipping checks on print() but checking
close() for a while now. Maybe I've been lucky, but it seems to have been
pretty successful at detecting disk-full conditions. The only downside is
that you don't know as soon as it happens -- you only know when you close() --
but depending on how your program is structured, that's usually not a problem.
Marvin Humphrey
I'd never thought to try this. What you suggested appears to work well
enough for ordinary code like EU::MM.
It fails interestingly. The first print that flushes the buffer
returns ENOSPC. Subsequent prints that don't flush the buffer return
EBADF. If the buffer flushes start succeeding the print calls still
keep erroneously returning EBADF. The close seems to return ENOSPC or
EBADF just like the print regardless of whether it is succeeding. The
EBADF results were all wrong. They ought to be have been ENOSPC or
success.
Josh
That's my thinking exactly. If there is a set of conditions where the buffers
line up with the stars, that's ok. The next close will fail. Really all we
care here is that "you're out of disk" shows up somewhere in the report. The
cost of covering that edge case is not worth the benefit.
--
94. Crucifixes do not ward off officers, and I should not test that.
Scan and modify the op tree, eh? That's pretty neat.
--
Robrt: People can't win
Schwern: No, but they can riot after the game.