From f13979bfbbcacb7240f2d00e507df0e12280a341 Mon Sep 17 00:00:00 2001 From: Fabian Homborg Date: Thu, 31 Mar 2022 15:10:45 +0200 Subject: [PATCH] Move executable-check to C++ This was already apparently supposed to work, but didn't because we just overrode errno again. This now means that, if a correctly named candidate exists, we don't start the command-not-found handler. See #8804 --- share/config.fish | 6 ------ share/functions/fish_command_not_found.fish | 6 ------ src/parse_execution.cpp | 8 ++++++-- src/path.cpp | 9 +++++++++ tests/checks/command-not-found.fish | 4 +--- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/share/config.fish b/share/config.fish index f811b3aa0..952cd58b8 100644 --- a/share/config.fish +++ b/share/config.fish @@ -13,12 +13,6 @@ or set -g __fish_added_user_paths # function __fish_default_command_not_found_handler printf (_ "fish: Unknown command: %s\n") (string escape -- $argv[1]) >&2 - for file in $PATH/$argv[1] - if test -e $file -a ! -x $file - printf (_ "fish: %s exists but isn't executable\n") (string escape -- $file) >&2 - break - end - end end diff --git a/share/functions/fish_command_not_found.fish b/share/functions/fish_command_not_found.fish index d5f5b3f73..36ca21905 100644 --- a/share/functions/fish_command_not_found.fish +++ b/share/functions/fish_command_not_found.fish @@ -13,12 +13,6 @@ end function __fish_default_command_not_found_handler printf (_ "fish: Unknown command: %s\n") (string escape -- $argv[1]) >&2 - for file in $PATH/$argv[1] - if test -e $file -a ! -x $file - printf (_ "fish: %s exists but isn't executable\n") (string escape -- $file) >&2 - break - end - end end # If an old handler already exists, defer to that. diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 67a8031b6..f80d5d267 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -731,8 +731,12 @@ end_execution_reason_t parse_execution_context_t::handle_command_not_found( const wchar_t *const cmd = cmd_str.c_str(); if (err_code != ENOENT) { + // TODO: We currently handle all errors here the same, + // but this mainly applies to EACCES. We could also feasibly get: + // ELOOP + // ENAMETOOLONG return this->report_error(STATUS_NOT_EXECUTABLE, statement, - _(L"The file '%ls' is not executable by this user"), cmd); + _(L"Unknown command. '%ls' exists but is not an executable file."), cmd); } // Handle unrecognized commands with standard command not found handler that can make better @@ -872,7 +876,7 @@ end_execution_reason_t parse_execution_context_t::populate_plain_process( if (!has_command && !use_implicit_cd) { // No command. If we're --no-execute return okay - it might be a function. if (no_exec()) return end_execution_reason_t::ok; - return this->handle_command_not_found(cmd, statement, no_cmd_err_code); + return this->handle_command_not_found(path_to_external_command.empty() ? cmd : path_to_external_command, statement, no_cmd_err_code); } } diff --git a/src/path.cpp b/src/path.cpp index 33afd7a96..828689a6e 100644 --- a/src/path.cpp +++ b/src/path.cpp @@ -87,6 +87,15 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path, return true; } err = EACCES; + if (out_path) *out_path = std::move(next_path); + } else if (errno != ENOENT && err == ENOENT) { + // Keep the first *interesting* error and path around. + // ENOENT isn't interesting because not having a file is the normal case. + auto tmperr = errno; + // Ignore if the parent directory is already inaccessible. + if (access(wcs2string(wdirname(next_path)).c_str(), X_OK) != 0) continue; + err = tmperr; + if (out_path) *out_path = std::move(next_path); } } diff --git a/tests/checks/command-not-found.fish b/tests/checks/command-not-found.fish index 841f33c4a..2dbec5648 100644 --- a/tests/checks/command-not-found.fish +++ b/tests/checks/command-not-found.fish @@ -35,9 +35,7 @@ echo $status set -g PATH . echo banana > foobar foobar --banana -# CHECKERR: fish: Unknown command: foobar -# CHECKERR: fish: ./foobar exists but isn't executable -# CHECKERR: checks/command-not-found.fish (line 37): +# CHECKERR: checks/command-not-found.fish (line {{\d+}}): Unknown command. './foobar' exists but is not an executable file. # CHECKERR: foobar --banana # CHECKERR: ^