make: return non-zero exit status when a command fails

When a build command returned a non-zero exit status 'make'
reported a warning and returned an exit code of zero.

This was due to the misuse of the status returned by system(3).
As the man page says:

   the return value is a "wait status" that can be examined using
   the macros described in waitpid(2).  (i.e., WIFEXITED(),
   WEXITSTATUS(), and so on).

Use the error() function to correctly report the problem on stderr
and return an exit status of 2.

Some additional changes in the same area:

- When a target is removed report the diagnostic on stderr, as
  required by POSIX.

- When a build command receives a signal GNU make removes the target.
  bmake doesn't and it isn't required by POSIX.  Implement this as an
  extension.

- Expand the error message when a build command fails so it includes
  the exit status or signal number, as obtained from the value
  returned by system(3).

- Alter the WIN32 implementation of system(3) to handle exit codes
  which represent termination as if by a signal.

Adds 200-240 bytes.

(GitHub issue #354)

3 files changed, 48 insertions, 10 deletions

diff --git a/miscutils/make.c b/miscutils/make.c
index d554a5f7f..8feec1202 100644
--- a/miscutils/make.c
+++ b/miscutils/make.c
@@ -293,6 +293,19 @@ vwarning(FILE *stream, const char *msg, va_list list)
 }
 
 /*
+ * Diagnostic handler.  Print message to standard error.
+ */
+static void
+diagnostic(const char *msg, ...)
+{
+        va_list list;
+
+        va_start(list, msg);
+        vwarning(stderr, msg, list);
+        va_end(list);
+}
+
+/*
  * Error handler.  Print message and exit.
  */
 static void error(const char *msg, ...) NORETURN;
@@ -2111,7 +2124,7 @@ remove_target(void)
         if (!dryrun && !print && !precious &&
                         target && !(target->n_flag & (N_PRECIOUS | N_PHONY)) &&
                         unlink(target->n_name) == 0) {
-                warning("'%s' removed", target->n_name);
+                diagnostic("'%s' removed", target->n_name);
         }
 }
 
@@ -2164,23 +2177,35 @@ docmds(struct name *np, struct cmd *cp)
 
                         target = np;
                         status = system(cmd);
-                        target = NULL;
                         // If this command was being run to create an include file
                         // or bring it up-to-date errors should be ignored and a
                         // failure status returned.
                         if (status == -1 && !doinclude) {
                                 error("couldn't execute '%s'", q);
                         } else if (status != 0 && !signore) {
-                                if (!doinclude)
-                                        warning("failed to build '%s'", np->n_name);
-#if !ENABLE_PLATFORM_MINGW32
-                                if (status == SIGINT || status == SIGQUIT)
-#endif
+                                if (!posix && WIFSIGNALED(status))
                                         remove_target();
-                                if (errcont || doinclude)
+                                if (errcont || doinclude) {
+                                        warning("failed to build '%s'", np->n_name);
                                         estat |= MAKE_FAILURE;
-                                else
-                                        exit(status);
+                                } else {
+                                        const char *err_type = NULL;
+                                        int err_value;
+
+                                        if (WIFEXITED(status)) {
+                                                err_type = "exit";
+                                                err_value = WEXITSTATUS(status);
+                                        } else if (WIFSIGNALED(status)) {
+                                                err_type = "signal";
+                                                err_value = WTERMSIG(status);
+                                        }
+
+                                        if (err_type)
+                                                error("failed to build '%s' %s %d", np->n_name,
+                                                                err_type, err_value);
+                                        else
+                                                error("failed to build '%s'", np->n_name);
+                                }
                         }
                 }
                 if (sdomake || dryrun || dotouch)
diff --git a/testsuite/make.tests b/testsuite/make.tests
index b53b4a2ea..60bb78406 100755
--- a/testsuite/make.tests
+++ b/testsuite/make.tests
@@ -86,6 +86,14 @@ test.k:
 '
 cd .. || exit 1; rm -rf make.tempdir 2>/dev/null
 
+# There was a bug where the failure of a build command didn't result
+# in make returning a non-zero exit status.
+testing "make return error if command fails" \
+        "make -f - >/dev/null 2>&1; echo \$?" "2\n" "" '
+target:
+        @exit 42
+'
+
 # A macro created using ::= remembers it's of type immediate-expansion.
 # Immediate expansion also occurs when += is used to append to such a macro.
 testing "make appending to immediate-expansion macro" \
diff --git a/win32/system.c b/win32/system.c
index 44a47f861..00b594242 100644
--- a/win32/system.c
+++ b/win32/system.c
@@ -6,6 +6,7 @@ int mingw_system(const char *cmd)
         intptr_t proc;
         HANDLE h;
         DWORD ret = 0;
+        int sig;
 
         if (cmd == NULL)
                 return 1;
@@ -18,5 +19,9 @@ int mingw_system(const char *cmd)
         GetExitCodeProcess(h, &ret);
         CloseHandle(h);
 
+        // Was process terminated as if by a signal?
+        sig = ret >> 24;
+        if (sig != 0 && ret == sig << 24 && is_valid_signal(sig))
+                return sig;
         return ret << 8;
 }