From fa937343f5e0c8ea131d321118b31621413ab1d2 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 4 Aug 2018 12:20:11 -0400 Subject: [PATCH] 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 | 23 ++--- src/job.c | 20 ++-- src/load.c | 2 +- tests/scripts/features/errors | 145 +++++++++++------------------ tests/scripts/features/output-sync | 2 +- tests/scripts/functions/shell | 10 +- 6 files changed, 80 insertions(+), 122 deletions(-) diff --git a/src/function.c b/src/function.c index 12e0ef2f..4602ebb2 100644 --- a/src/function.c +++ b/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 { diff --git a/src/job.c b/src/job.c index 0cd13eb5..d8d5b7ce 100644 --- a/src/job.c +++ b/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. - 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]); + OSS (error, NILF, "%s: %s", argv[0], strerror (errno)); break; case ENOEXEC: { @@ -2439,10 +2436,7 @@ exec_command (char **argv, char **envp) # else execvp (shell, new_argv); # endif - if (errno == ENOENT) - OS (error, NILF, _("%s: Shell program not found"), shell); - else - perror_with_name ("execvp: ", shell); + OSS (error, NILF, "%s: %s", new_argv[0], strerror (errno)); 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; } diff --git a/src/load.c b/src/load.c index ecf66cba..5ec87c6b 100644 --- a/src/load.c +++ b/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; } diff --git a/tests/scripts/features/errors b/tests/scripts/features/errors index 5c570790..eac64ed9 100644 --- a/tests/scripts/features/errors +++ b/tests/scripts/features/errors @@ -1,107 +1,74 @@ # -*-perl-*- -$description = "The following tests the -i option and the '-' in front of \n" - ."commands to test that make ignores errors in these commands\n" - ."and continues processing."; +$description = "Test ignored failures in recipe command lines"; -$details = "This test runs two makes. The first runs on a target with a \n" - ."command that has a '-' in front of it (and a command that is \n" - ."intended to fail) and then a delete command after that is \n" - ."intended to succeed. If make ignores the failure of the first\n" - ."command as it is supposed to, then the second command should \n" - ."delete a file and this is what we check for. The second make\n" - ."that is run in this test is identical except that the make \n" - ."command is given with the -i option instead of the '-' in \n" - ."front of the command. They should run the same. "; - -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; +run_make_test(qq! +one: +\t-exit 1 +\texit 0 +two: +\texit 1 +\texit 0 +!, + "one", "exit 1\n#MAKE#: [#MAKEFILE#;3: one] Error 1 (ignored)\nexit 0\n"); # TEST #1 # ------- -$answer = "$rm_command cleanit -$cleanit_error -$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"); +run_make_test(undef, " -i two", + "exit 1\n#MAKE#: [#MAKEFILE#;6: two] Error 1 (ignored)\nexit 0\n"); # 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 - @echo there - @exit 1 +\t\@echo hi +\t\@echo there +\t\@exit 1 !, '', "hi\nthere\n#MAKE#: *** [#MAKEFILE#;5: all] Error 1", 512); -1; +# TEST #3 +# ------- -### Local Variables: -### eval: (setq whitespace-action (delq 'auto-cleanup whitespace-action)) -### End: +# Try failing due to unknown command +my $unk = './foobarbazbozblat'; +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; diff --git a/tests/scripts/features/output-sync b/tests/scripts/features/output-sync index 1407e81a..49d3be2c 100644 --- a/tests/scripts/features/output-sync +++ b/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. diff --git a/tests/scripts/functions/shell b/tests/scripts/functions/shell index bbb25dd9..3ffa1d8d 100644 --- a/tests/scripts/functions/shell +++ b/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 ... ?