Clean up errors for invalid commands and add regression tests.

* src/function.c (func_shell_base): Use error() instead of recreating
the error output.
* src/job.c (exec_command): Show more standard error messages.
* src/load.c (unload_file): Fix whitespace in the error message.
* tests/scripts/features/errors: Add tests for starting non-
existent commands and new error message formats.
* tests/scripts/features/output-sync: New error message formats.
* tests/scripts/functions/shell: Ditto.

src/function.c

@@ -1699,7 +1699,6 @@ func_shell_base (char *o, char **argv, int trim_newlines)
   FILE *fpipe;
 #endif
   char **command_argv = NULL;
-  const char *error_prefix;
   char **envp;
   int pipedes[2];
   pid_t pid;
@@ -1739,17 +1738,6 @@ func_shell_base (char *o, char **argv, int trim_newlines)
 
   envp = environ;
 
-  /* For error messages.  */
-  if (reading_file && reading_file->filenm)
-    {
-      char *p = alloca (strlen (reading_file->filenm)+11+4);
-      sprintf (p, "%s:%lu: ", reading_file->filenm,
-               reading_file->lineno + reading_file->offset);
-      error_prefix = p;
-    }
-  else
-    error_prefix = "";
-
   /* Set up the output in case the shell writes something.  */
   output_start ();
 
@@ -1760,7 +1748,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
   fpipe = msdos_openpipe (pipedes, &pid, argv[0]);
   if (pipedes[0] < 0)
     {
-      perror_with_name (error_prefix, "pipe");
+      OS (error, reading_file, "pipe: %s", strerror (errno));
       pid = -1;
       goto done;
     }
@@ -1774,7 +1762,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
     {
       /* Open of the pipe failed, mark as failed execution.  */
       shell_completed (127, 0);
-      perror_with_name (error_prefix, "pipe");
+      OS (error, reading_file, "pipe: %s", strerror (errno));
       pid = -1;
       goto done;
     }
@@ -1782,7 +1770,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
 #else
   if (pipe (pipedes) < 0)
     {
-      perror_with_name (error_prefix, "pipe");
+      OS (error, reading_file, "pipe: %s", strerror (errno));
       pid = -1;
       goto done;
     }
@@ -1801,7 +1789,10 @@ func_shell_base (char *o, char **argv, int trim_newlines)
   }
 
   if (pid < 0)
-    goto done;
+    {
+      shell_completed (127, 0);
+      goto done;
+    }
 #endif
 
   {

src/job.c

@@ -2213,6 +2213,9 @@ child_execute_job (struct output *out, int good_stdin, char **argv, char **envp)
         close (save_fderr);
     }
 
+  if (pid < 0)
+    OSS (error, NILF, "%s: %s", argv[0], strerror (errno));
+
   return pid;
 }
 
@@ -2286,7 +2289,7 @@ exec_command (char **argv, char **envp)
 #endif
   /* Run the program.  */
   execve (argv[0], argv, envp);
-  perror_with_name ("execve: ", argv[0]);
+  OSS (error, NILF, "%s: %s", argv[0], strerror (errno));
   _exit (EXIT_FAILURE);
 #else
 #ifdef WINDOWS32
@@ -2375,13 +2378,7 @@ exec_command (char **argv, char **envp)
   switch (errno)
     {
     case ENOENT:
-      /* We are in the child: don't use the output buffer.
+      OSS (error, NILF, "%s: %s", argv[0], strerror (errno));
-         It's not right to run fprintf() here!  */
-      if (makelevel == 0)
-        fprintf (stderr, _("%s: %s: Command not found\n"), program, argv[0]);
-      else
-        fprintf (stderr, _("%s[%u]: %s: Command not found\n"),
-                 program, makelevel, argv[0]);
       break;
     case ENOEXEC:
       {
@@ -2439,10 +2436,7 @@ exec_command (char **argv, char **envp)
 # else
         execvp (shell, new_argv);
 # endif
-        if (errno == ENOENT)
+        OSS (error, NILF, "%s: %s", new_argv[0], strerror (errno));
-          OS (error, NILF, _("%s: Shell program not found"), shell);
-        else
-          perror_with_name ("execvp: ", shell);
         break;
       }
 
@@ -2454,7 +2448,7 @@ exec_command (char **argv, char **envp)
 # endif
 
     default:
-      perror_with_name ("execvp: ", argv[0]);
+      OSS (error, NILF, "%s: %s", argv[0], strerror (errno));
       break;
     }
 

src/load.c

@@ -240,7 +240,7 @@ unload_file (const char *name)
     if (streq (d->name, name) && d->dlp)
       {
         if (dlclose (d->dlp))
-          perror_with_name ("dlclose", d->name);
+          perror_with_name ("dlclose: ", d->name);
         d->dlp = NULL;
         break;
       }

tests/scripts/features/errors

@@ -1,107 +1,74 @@
 #                                                                    -*-perl-*-
 
-$description = "The following tests the -i option and the '-' in front of \n"
+$description = "Test ignored failures in recipe command lines";
-              ."commands to test that make ignores errors in these commands\n"
-              ."and continues processing.";
 
-$details = "This test runs two makes.  The first runs on a target with a \n"
+run_make_test(qq!
-          ."command that has a '-' in front of it (and a command that is \n"
+one:
-          ."intended to fail) and then a delete command after that is \n"
+\t-exit 1
-          ."intended to succeed.  If make ignores the failure of the first\n"
+\texit 0
-          ."command as it is supposed to, then the second command should \n"
+two:
-          ."delete a file and this is what we check for.  The second make\n"
+\texit 1
-          ."that is run in this test is identical except that the make \n"
+\texit 0
-          ."command is given with the -i option instead of the '-' in \n"
+!,
-          ."front of the command.  They should run the same. ";
+              "one", "exit 1\n#MAKE#: [#MAKEFILE#;3: one] Error 1 (ignored)\nexit 0\n");
-
-if ($vos)
-{
-   $rm_command = "delete_file";
-}
-else
-{
-   $rm_command = "rm";
-}
-
-open(MAKEFILE,"> $makefile");
-
-# The Contents of the MAKEFILE ...
-
-print MAKEFILE "clean:\n"
-              ."\t-$rm_command cleanit\n"
-              ."\t$rm_command foo\n"
-              ."clean2: \n"
-              ."\t$rm_command cleanit\n"
-              ."\t$rm_command foo\n";
-
-# END of Contents of MAKEFILE
-
-close(MAKEFILE);
-
-&touch("foo");
-
-unlink("cleanit");
-$cleanit_error = `sh -c "$rm_command cleanit 2>&1"`;
-chomp $cleanit_error;
-$delete_error_code = $? >> 8;
 
 # TEST #1
 # -------
 
-$answer = "$rm_command cleanit
+run_make_test(undef, " -i two",
-$cleanit_error
+              "exit 1\n#MAKE#: [#MAKEFILE#;6: two] Error 1 (ignored)\nexit 0\n");
-$make_name: [$makefile;2: clean] Error $delete_error_code (ignored)
-$rm_command foo\n";
-
-&run_make_with_options($makefile,"",&get_logfile);
-
-# If make acted as planned, it should ignore the error from the first
-# command in the target and execute the second which deletes the file "foo"
-# This file, therefore, should not exist if the test PASSES.
-if (-f "foo") {
-  $test_passed = 0;
-}
-
-# The output for this on VOS is too hard to replicate, so we only check it
-# on unix.
-if (!$vos)
-{
-   &compare_output($answer,&get_logfile(1));
-}
-
-
-&touch("foo");
 
 # TEST #2
 # -------
 
-$answer = "$rm_command cleanit
-$cleanit_error
-$make_name: [$makefile;5: clean2] Error $delete_error_code (ignored)
-$rm_command foo\n";
-
-&run_make_with_options($makefile,"clean2 -i",&get_logfile);
-
-if (-f "foo") {
-  $test_passed = 0;
-}
-
-if (!$vos) {
-   &compare_output($answer,&get_logfile(1));
-}
-
 # Test that error line offset works
 
-run_make_test(q!
+run_make_test(qq!
 all:
-	@echo hi
+\t\@echo hi
-	@echo there
+\t\@echo there
-	@exit 1
+\t\@exit 1
 !,
               '', "hi\nthere\n#MAKE#: *** [#MAKEFILE#;5: all] Error 1", 512);
 
-1;
+# TEST #3
+# -------
 
-### Local Variables:
+# Try failing due to unknown command
-### eval: (setq whitespace-action (delq 'auto-cleanup whitespace-action))
+my $unk = './foobarbazbozblat';
-### End:
+unlink($unk);
+
+my $err = $ERR_no_such_file;
+
+run_make_test(qq!
+one: ; -$unk xx yy
+two: ; $unk aa bb
+!,
+              'one', "$unk xx yy\n#MAKE#: $unk: $err\n#MAKE#: [#MAKEFILE#;2: one] Error 127 (ignored)\n");
+
+# TEST #4
+# -------
+
+run_make_test(undef, 'two -i',
+              "$unk aa bb\n#MAKE#: $unk: $err\n#MAKE#: [#MAKEFILE#;3: two] Error 127 (ignored)\n");
+
+# TEST #5
+# -------
+
+run_make_test(undef, 'two',
+              "$unk aa bb\n#MAKE#: $unk: $err\n#MAKE#: *** [#MAKEFILE#;3: two] Error 127\n", 512);
+
+# Try failing due to non-executable file
+
+my $noexe = './barfooblatboz';
+touch($noexe);
+
+run_make_test(qq!
+one: ; -$noexe xx yy
+two: ; $noexe aa bb
+!,
+              'one', "$noexe xx yy\n#MAKE#: $noexe: Permission denied\n#MAKE#: [#MAKEFILE#;2: one] Error 127 (ignored)\n");
+
+unlink($noexe);
+
+1;

tests/scripts/features/output-sync

@@ -343,7 +343,7 @@ if ($port_type ne 'W32') {
     run_make_test(q!
 all:: ; @./foo bar baz
 !,
-              '-O', "#MAKE#: ./foo: Command not found\n#MAKE#: *** [#MAKEFILE#;2: all] Error 127\n", 512);
+              '-O', "#MAKE#: ./foo: $ERR_no_such_file\n#MAKE#: *** [#MAKEFILE#;2: all] Error 127\n", 512);
 }
 
 # This tells the test driver that the perl test script executed properly.

tests/scripts/functions/shell

@@ -49,9 +49,15 @@ if ($port_type ne 'W32') {
 all:
 	@echo hi
 	$(shell ./basdfdfsed there)
-	@echo there
+	@echo $(.SHELLSTATUS)
 ',
-                  '', "#MAKE#: ./basdfdfsed: Command not found\nhi\nthere\n");
+                  '', "#MAKE#: ./basdfdfsed: $ERR_no_such_file\nhi\n127\n");
+
+    run_make_test('
+$(shell ./basdfdfsed where)
+all: ; @echo $(.SHELLSTATUS)
+',
+                  '', "#MAKE#: ./basdfdfsed: $ERR_no_such_file\n127\n");
 
     # Test SHELLSTATUS for kill.
     # This test could be ported to Windows, using taskkill ... ?