diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 572d2900b..b2be3ed81 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Scripting improvements ---------------------- - ``math`` now has a ``log2`` function like the documentation already claimed. - Shebang lines are no longer required within shell scripts, improving support for scripts with concatenated binary contents. If a file fails to execute and passes a binary safety check, fish will re-invoke the file using `/bin/sh` (:issue:`7802`). +- Exit codes are better aligned with bash. A failed exec now reports ``$status`` of 127 if the file is not found, and 126 if it is not executable. Interactive improvements ------------------------- diff --git a/src/common.h b/src/common.h index ce78bcf09..a7d33f851 100644 --- a/src/common.h +++ b/src/common.h @@ -657,13 +657,9 @@ enum { /// The status code used when a command was not found. STATUS_CMD_UNKNOWN = 127, - /// TODO: Figure out why we have two distinct failure codes for when an external command cannot - /// be run. - /// + /// The status code used when an external command can not be run. STATUS_NOT_EXECUTABLE = 126, - /// The status code used when an external command can not be run. - STATUS_EXEC_FAIL = 125, /// The status code used when a wildcard had no matches. STATUS_UNMATCHED_WILDCARD = 124, diff --git a/src/exec.cpp b/src/exec.cpp index 6268ad199..95af8b9e0 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -63,6 +63,32 @@ enum class launch_result_t { failed, } __warn_unused_type; +/// Given an error \p err returned from either posix_spawn or exec, \return a process exit code. +static int exit_code_from_exec_error(int err) { + assert(err && "Zero is success, not an error"); + switch (err) { + case ENOENT: + case ENOTDIR: + // This indicates either the command was not found, or a file redirection was not found. + // We do not use posix_spawn file redirections so this is always command-not-found. + return STATUS_CMD_UNKNOWN; + + case EACCES: + case ENOEXEC: + // The file is not executable for various reasons. + return STATUS_NOT_EXECUTABLE; + +#ifdef EBADARCH + case EBADARCH: + // This is for e.g. running ARM app on Intel Mac. + return STATUS_NOT_EXECUTABLE; +#endif + default: + // Generic failure. + return EXIT_FAILURE; + } +} + /// This is a 'looks like text' check. /// \return true if either there is no NUL byte, or there is a line containing a lowercase letter /// before the first NUL byte. @@ -149,7 +175,7 @@ bool is_thompson_shell_script(const char *path) { errno = err; safe_report_exec_error(errno, actual_cmd, argv, envv); - exit_without_destructors(STATUS_EXEC_FAIL); + exit_without_destructors(exit_code_from_exec_error(err)); } /// This function is similar to launch_process, except it is not called after a fork (i.e. it only @@ -582,6 +608,7 @@ static launch_result_t exec_external_command(parser_t &parser, const std::shared const_cast(envv)); if (int err = spawner.get_error()) { safe_report_exec_error(err, actual_cmd, argv, envv); + p->status = proc_status_t::from_exit_code(exit_code_from_exec_error(err)); return launch_result_t::failed; } assert(pid.has_value() && *pid > 0 && "Should have either a valid pid, or an error"); diff --git a/src/proc.cpp b/src/proc.cpp index 70f7b9a62..d9728ca93 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -271,8 +271,12 @@ void process_t::check_generations_before_launch() { } void process_t::mark_aborted_before_launch() { - completed = true; - status = proc_status_t::from_exit_code(EXIT_FAILURE); + this->completed = true; + // The status may have already been set to e.g. STATUS_NOT_EXECUTABLE. + // Only stomp a successful status. + if (this->status.is_success()) { + this->status = proc_status_t::from_exit_code(EXIT_FAILURE); + } } bool process_t::is_internal() const { diff --git a/tests/checks/noshebang.fish b/tests/checks/noshebang.fish index c5353fed1..f80a8b324 100644 --- a/tests/checks/noshebang.fish +++ b/tests/checks/noshebang.fish @@ -11,11 +11,13 @@ chmod a+x file function runfile # Run our file twice, printing status. # Arguments are passed to exercise the re-execve code paths; they have no other effect. - set -g fish_use_posix_spawn 0 + true # clear status + set -g fish_use_posix_spawn 1 ./file arg1 arg2 arg3 echo $status - set -g fish_use_posix_spawn 1 + true # clear status + set -g fish_use_posix_spawn 0 ./file arg1 arg2 arg3 arg4 arg5 echo $status end @@ -42,12 +44,12 @@ set -g fish_use_posix_spawn 1 ./file.fish echo $status rm file.fish -#CHECK: 125 +#CHECK: 126 #CHECKERR: Failed {{.*}} #CHECKERR: exec: {{.*}} #CHECKERR: {{.*}} -#CHECK: 125 +#CHECK: 126 #CHECKERR: Failed {{.*}} #CHECKERR: exec: {{.*}} #CHECKERR: {{.*}} @@ -63,12 +65,12 @@ runfile # Doesn't meet our heuristic as there is no newline. echo -n -e 'true\x00' >file runfile -#CHECK: 125 +#CHECK: 126 #CHECKERR: Failed {{.*}} #CHECKERR: exec: {{.*}} #CHECKERR: {{.*}} -#CHECK: 125 +#CHECK: 126 #CHECKERR: Failed {{.*}} #CHECKERR: exec: {{.*}} #CHECKERR: {{.*}} @@ -76,12 +78,12 @@ runfile # Doesn't meet our heuristic as there is no lowercase before newline. echo -n -e 'NOPE\n\x00' >file runfile -#CHECK: 125 +#CHECK: 126 #CHECKERR: Failed {{.*}} #CHECKERR: exec: {{.*}} #CHECKERR: {{.*}} -#CHECK: 125 +#CHECK: 126 #CHECKERR: Failed {{.*}} #CHECKERR: exec: {{.*}} #CHECKERR: {{.*}}