github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2024-11-26 02:13:38 +08:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

wildcard: Rationalize file/command completions (#10052)

Browse Source 
* wildcard: Remove file size from the description

We no longer add descriptions for normal file completions, so this was
only ever reached if this was a command completion, and then it was
only added if the file wasn't a regular file... in which case it can't
be an executable.

So this was dead.

* Make possible_link() a maybe

This gives us the full information, not just "no" or "maybe"

* wildcard: Rationalize file/command completions

This keeps the entry_t as long as possible, and asks it, so especially
on systems with working d_type we can get by without a single stat in
most cases.

Then it guts file_get_desc, because that is only used for command
completions - we have been disabling file descriptions for *years*,
and so this is never called there.

That means we have no need to print descriptions about e.g. broken symlinks, because those are not executable.

Put together, what this means is that we, in most cases, only do
an *access(2)* call instead of a stat, because that might be checking
more permissions.

So we have the following constellations:

- If we have d_type:
  - We need a stat() for every _symlink_ to get the type (e.g. dir or regular)
    (this is for most symlinks, if we want to know if it's a dir or executable)
  - We need an access() for every file for executables
- If we do not have d_type:
  - We need a stat() for every file
  - We need an lstat() for every file if we do descriptions
    (i.e. just for command completion)
  - We need an access() for every file for executables

As opposed to the current way, where every file gets one lstat whether
with d_type or not, and an additional stat() for links, *and* an
access.

So we go from two syscalls to one for executables.

* Some more comments

* rust link option

* rust remove size

* rust accessovaganza

* Check for .dll first for WSL

This saves quite a few checks if e.g. System32 is in $PATH (which it
is if you inherit windows paths, IIRC).

Note: Our WSL check currently fails for WSL2, where this would
be *more* important because of how abysmal the filesystem performance
on that is.
This commit is contained in:
Fabian Boehm 2023-10-14 08:45:15 +02:00
parent 6be5b02231
commit 85c03e4b67
3 changed files with 60 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

132
src/wildcard.cpp
View File

@ -327,58 +327,27 @@ wildcard_result_t wildcard_complete(const wcstring &str, const wchar_t *wc,
/// Obtain a description string for the file specified by the filename.
///
/// It assumes the file exists and won't run stat() to confirm.
/// The returned value is a string constant and should not be free'd.
///
/// \param filename The file for which to find a description string
/// \param lstat_res The result of calling lstat on the file
/// \param lbuf The struct buf output of calling lstat on the file
/// \param stat_res The result of calling stat on the file
/// \param buf The struct buf output of calling stat on the file
/// \param err The errno value after a failed stat call on the file.
static const wchar_t *file_get_desc(const wcstring &filename, int lstat_res,
const struct stat &lbuf, int stat_res, const struct stat &buf,
int err, bool definitely_executable) {
if (lstat_res) {
return COMPLETE_FILE_DESC;
}
if (S_ISLNK(lbuf.st_mode)) {
if (!stat_res) {
if (S_ISDIR(buf.st_mode)) {
/// \param is_dir Whether the file is a directory or not (might be behind a link)
/// \param is_link Whether it's a link (that might point to a directory)
/// \param definitely_executable Whether we know that it is executable, or don't know
static const wchar_t *file_get_desc(const wcstring &filename, bool is_dir,
bool is_link, bool definitely_executable) {
if (is_link) {
if (is_dir) {
return COMPLETE_DIRECTORY_SYMLINK_DESC;
}
if (definitely_executable || (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH) && waccess(filename, X_OK) == 0)) {
// Weird group permissions and other such issues make it non-trivial to find out if
// we can actually execute a file using the result from stat. It is much safer to
// use the access function, since it tells us exactly what we want to know.
//
// We skip this check in case the caller tells us the file is definitely executable.
if (definitely_executable || waccess(filename, X_OK) == 0) {
return COMPLETE_EXEC_LINK_DESC;
}
return COMPLETE_SYMLINK_DESC;
}
if (err == ENOENT) return COMPLETE_BROKEN_SYMLINK_DESC;
if (err == ELOOP) return COMPLETE_LOOP_SYMLINK_DESC;
// On unknown errors we do nothing. The file will be given the default 'File'
// description or one based on the suffix.
} else if (S_ISCHR(buf.st_mode)) {
return COMPLETE_CHAR_DESC;
} else if (S_ISBLK(buf.st_mode)) {
return COMPLETE_BLOCK_DESC;
} else if (S_ISFIFO(buf.st_mode)) {
return COMPLETE_FIFO_DESC;
} else if (S_ISSOCK(buf.st_mode)) {
return COMPLETE_SOCKET_DESC;
} else if (S_ISDIR(buf.st_mode)) {
} else if (is_dir) {
return COMPLETE_DIRECTORY_DESC;
} else if (definitely_executable || (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH) && waccess(filename, X_OK) == 0)) {
// Weird group permissions and other such issues make it non-trivial to find out if we can
// actually execute a file using the result from stat. It is much safer to use the access
// function, since it tells us exactly what we want to know.
//
// We skip this check in case the caller tells us the file is definitely executable.
} else if (definitely_executable || waccess(filename, X_OK) == 0) {
return COMPLETE_EXEC_DESC;
}
@ -390,14 +359,14 @@ static const wchar_t *file_get_desc(const wcstring &filename, int lstat_res,
/// up. Note that the filename came from a readdir() call, so we know it exists.
static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wcstring &filename,
const wchar_t *wc, expand_flags_t expand_flags,
completion_receiver_t *out, bool known_dir) {
completion_receiver_t *out, const dir_iter_t::entry_t &entry) {
const bool executables_only = expand_flags & expand_flag::executables_only;
const bool need_directory = expand_flags & expand_flag::directories_only;
// Fast path: If we need directories, and we already know it is one,
// and we don't need to do anything else, just return it.
// This is a common case for cd completions, and removes the `stat` entirely in case the system
// supports it.
if (known_dir && !executables_only && !(expand_flags & expand_flag::gen_descriptions)) {
if (entry.is_dir() && !executables_only && !(expand_flags & expand_flag::gen_descriptions)) {
return wildcard_complete(filename + L'/', wc, const_desc(L""), out, expand_flags,
COMPLETE_NO_SPACE) == wildcard_result_t::match;
}
@ -406,34 +375,7 @@ static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wc
return false;
}
struct stat lstat_buf = {}, stat_buf = {};
int stat_res = -1;
int stat_errno = 0;
int lstat_res = lwstat(filepath, &lstat_buf);
if (lstat_res >= 0) {
if (S_ISLNK(lstat_buf.st_mode)) {
stat_res = wstat(filepath, &stat_buf);
if (stat_res < 0) {
// In order to differentiate between e.g. broken symlinks and symlink loops, we also
// need to know the error status of wstat.
stat_errno = errno;
}
} else {
stat_buf = lstat_buf;
stat_res = lstat_res;
}
}
const long long file_size = stat_res == 0 ? stat_buf.st_size : 0;
const bool is_directory = stat_res == 0 && S_ISDIR(stat_buf.st_mode);
const bool is_executable = stat_res == 0 && S_ISREG(stat_buf.st_mode);
if (need_directory && !is_directory) {
return false;
}
if (executables_only && (!is_executable || waccess(filepath, X_OK) != 0)) {
if (need_directory && !entry.is_dir()) {
return false;
}
@ -442,23 +384,44 @@ static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wc
return false;
}
// regular file *excludes* broken links - we have no use for them as commands.
const bool is_regular_file = entry.check_type() == dir_entry_type_t::reg;
if (executables_only && (!is_regular_file || waccess(filepath, X_OK) != 0)) {
return false;
}
// Compute the description.
// This is effectively only for command completions,
// because we disable descriptions for regular file completions.
wcstring desc;
if (expand_flags & expand_flag::gen_descriptions) {
bool is_link = false;
if (!entry.is_possible_link().has_value()) {
// We do not know it's a link from the d_type,
// so we will have to do an lstat().
struct stat lstat_buf = {};
int lstat_res = lwstat(filepath, &lstat_buf);
if (lstat_res < 0) {
// This file is no longer be usable, skip it.
return false;
}
if (S_ISLNK(lstat_buf.st_mode)) {
is_link = true;
}
} else {
is_link = entry.is_possible_link().value();
}
// If we have executables_only, we already checked waccess above,
// so we tell file_get_desc that this file is definitely executable so it can skip the check.
desc = file_get_desc(filepath, lstat_res, lstat_buf, stat_res, stat_buf, stat_errno, executables_only);
if (!is_directory && !is_executable && file_size >= 0) {
if (!desc.empty()) desc.append(L", ");
desc.append(format_size(file_size));
}
desc = file_get_desc(filepath, entry.is_dir(), is_link, executables_only);
}
// Append a / if this is a directory. Note this requirement may be the only reason we have to
// call stat() in some cases.
auto desc_func = const_desc(desc);
if (is_directory) {
if (entry.is_dir()) {
return wildcard_complete(filename + L'/', wc, desc_func, out, expand_flags,
COMPLETE_NO_SPACE) == wildcard_result_t::match;
}
@ -592,7 +555,7 @@ class wildcard_expander_t {
void try_add_completion_result(const wcstring &filepath, const wcstring &filename,
const wcstring &wildcard, const wcstring &prefix,
bool known_dir) {
const dir_iter_t::entry_t &entry) {
// This function is only for the completions case.
assert(this->flags & expand_flag::for_completions);
@ -604,7 +567,7 @@ class wildcard_expander_t {
size_t before = this->resolved_completions->size();
if (wildcard_test_flags_then_complete(abs_path, filename, wildcard.c_str(), this->flags,
this->resolved_completions, known_dir)) {
this->resolved_completions, entry)) {
// Hack. We added this completion result based on the last component of the wildcard.
// Prepend our prefix to each wildcard that replaces its token.
// Note that prepend_token_prefix is a no-op unless COMPLETE_REPLACES_TOKEN is set
@ -708,7 +671,7 @@ void wildcard_expander_t::expand_trailing_slash(const wcstring &base_dir, const
if (need_dir && !known_dir) continue;
if (!entry->name.empty() && entry->name.at(0) != L'.') {
this->try_add_completion_result(base_dir + entry->name, entry->name, L"",
prefix, known_dir);
prefix, *entry);
}
}
}
@ -740,7 +703,7 @@ void wildcard_expander_t::expand_intermediate_segment(const wcstring &base_dir,
//
// We only do this when we are the last `*/` component,
// because we're a bit inconsistent on when we will enter loops.
if (is_final && !entry->is_possible_link()) {
if (is_final && !entry->is_possible_link().value_or(true)) {
// We made it through.
// Perform normal wildcard expansion on this new directory,
// starting at our tail_wc
@ -826,7 +789,6 @@ void wildcard_expander_t::expand_literal_intermediate_segment_with_fuzz(const wc
void wildcard_expander_t::expand_last_segment(const wcstring &base_dir, dir_iter_t &base_dir_iter,
const wcstring &wc, const wcstring &prefix) {
bool is_dir = false;
bool need_dir = flags & expand_flag::directories_only;
const dir_iter_t::entry_t *entry{};
@ -834,7 +796,7 @@ void wildcard_expander_t::expand_last_segment(const wcstring &base_dir, dir_iter
if (need_dir && !entry->is_dir()) continue;
if (flags & expand_flag::for_completions) {
this->try_add_completion_result(base_dir + entry->name, entry->name, wc, prefix,
is_dir);
*entry);
} else {
// Normal wildcard expansion, not for completions.
if (wildcard_match(entry->name, wc, true /* skip files with leading dots */)) {

8
src/wutil.cpp
View File

@ -231,9 +231,15 @@ const dir_iter_t::entry_t *dir_iter_t::next() {
// Do not store symlinks as type as we will need to resolve them.
if (type != dir_entry_type_t::lnk) {
entry_.type_ = type;
} else {
entry_.type_ = none();
}
// This entry could be a link if it is a link or unknown.
entry_.possible_link_ = !type.has_value() || type == dir_entry_type_t::lnk;
if (type.has_value()) {
entry_.possible_link_ = type == dir_entry_type_t::lnk;
} else {
entry_.possible_link_ = none();
}
#endif
return &entry_;
}

4
src/wutil.h
View File

@ -222,7 +222,7 @@ class dir_iter_t : noncopyable_t {
bool is_dir() const { return check_type() == dir_entry_type_t::dir; }
/// \return false if we know this can't be a link via d_type, true if it could be.
bool is_possible_link() const { return possible_link_; }
maybe_t<bool> is_possible_link() const { return possible_link_; }
/// \return the stat buff for this entry, invoking stat() if necessary.
const maybe_t<struct stat> &stat() const;
@ -243,7 +243,7 @@ class dir_iter_t : noncopyable_t {
mutable maybe_t<dir_entry_type_t> type_{};
/// whether this entry could be a link, false if we know definitively it isn't.
bool possible_link_ = true;
mutable maybe_t<bool> possible_link_{};
// fd of the DIR*, used for fstatat().
int dirfd_{-1};