Re: [PATCH 2/2] job.c: implementing child_execute_job() using posix_spawn(), and use it if present

[Paul Smith]

On Mon, 2018-07-09 at 09:05 +0200, Aron Barath wrote:
> ---

Thanks for the work you put into this!

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.

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:

  all:
          -barbbler
          echo hi

With current make we get correct behavior in that the non-existent
command failure is ignored, and the "hi" is still echo'd:

  barbbler
  /tmp/x1.mk:2: recipe for target 'all' failed
  make: [all] Error 127 (ignored)
  echo hi
  hi

while after the posix_spawn change the failure is no longer ignored as
it should be:

  barbbler
  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.

I'll look into this today but it could take some effort to clean all
this up.

Cheers!