[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] job.c: implementing child_execute_job() using posix_spaw
Re: [PATCH 2/2] job.c: implementing child_execute_job() using posix_spawn(), and use it if present
Mon, 23 Jul 2018 07:10:06 +0200
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1
On 07/22/2018 09:58 PM, Paul Smith wrote:
On Mon, 2018-07-09 at 09:05 +0200, Aron Barath wrote:
Thanks for the work you put into this!
You're welcome! :)
Unfortunately this change reveals some deeper problems that I will need
to address. Basically, the old code never really expected fork() to
fail: if it did we intended to just give up completely. Thus, it
expected child_execute_job() to always succeed except in exceptional
situations, where make itself was not able to continue.
That's good news and bad news.
Now that we use posix_spawn() it's quite possible that
child_execute_job() can fail for simple reasons, such as the program we
attempt to invoke does not exist or similar.
This reveals that the error handling around child_execute_job() is just
not right and needs to be reworked... this is some hairy code however.
As an example of the kind of problem we get, consider a make recipe
that is marked to be ignored, and the command does not exist:
With current make we get correct behavior in that the non-existent
command failure is ignored, and the "hi" is still echo'd:
/tmp/x1.mk:2: recipe for target 'all' failed
make: [all] Error 127 (ignored)
while after the posix_spawn change the failure is no longer ignored as
it should be:
make: barbbler: Command not found
make: fork: No such file or directory
We get a nicer error message here (except for the extraneous fork
error), but note how the "echo hi" is no longer run: the failure is not
ignored as it should be.
Didn't we have a test case for that?
I'll look into this today but it could take some effort to clean all
If there is anything I can help, let me know.