Rationalize exit codes for failed execs

This cleans up some exit code processing. Previously a failed exec
would produce exit code 125 unconditionally, while a failed posix_spawn
would produce exit code 1 (!).

With this change, fish reports exit code 126 for not-executable, and 127
for file-not-found. This matches bash.

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

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,

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<char *const *>(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");

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 {

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: {{.*}}