Simplify is_executable in nu-completion. (#3742)

On Windows, we used the `is-exeuctable` crate but on Unix, we
duplicated the check that it did, with one difference: We also
looked at whether or not it was a symlink.

The `is-executable` crate uses `std::fs::metadata` which follows
symlinks, so this scenario should never occur here, as it will
return the metadata for the target file.

Using the `is-executable` crate on both Unix and Windows lets us
make it non-optional. This lets us remove the `executable-support`
feature. (It is worth noting that this code didn't compile on
Windows when the `executable-support` feature was not specified.)

Right now, there is an alternate code path for `target_arch` being
`wasm32`. This isn't exactly correct as it should probably handle
something different for when the `target_os` is `wasi`.
This commit is contained in:
Bruce Mitchener 2021-07-07 19:53:07 +07:00 committed by GitHub
parent 08c624576c
commit 1943071d12
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 6 additions and 22 deletions

View File

@ -71,7 +71,6 @@ rustyline-support = ["nu-cli/rustyline-support", "nu-command/rustyline-support"]
term-support = ["nu-command/term"] term-support = ["nu-command/term"]
uuid-support = ["nu-command/uuid_crate"] uuid-support = ["nu-command/uuid_crate"]
which-support = ["nu-command/which", "nu-engine/which"] which-support = ["nu-command/which", "nu-engine/which"]
executable-support = ["nu-completion/is_executable"]
default = [ default = [
"nu-cli/shadow-rs", "nu-cli/shadow-rs",
@ -85,7 +84,6 @@ default = [
"post", "post",
"fetch", "fetch",
"zip-support", "zip-support",
"executable-support",
"dataframe", "dataframe",
] ]

View File

@ -21,7 +21,9 @@ nu-test-support = { version="0.33.1", path="../nu-test-support" }
dirs-next = "2.0.0" dirs-next = "2.0.0"
indexmap = { version="1.6.1", features=["serde-1"] } indexmap = { version="1.6.1", features=["serde-1"] }
is_executable = { version="1.0.1", optional=true }
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
is_executable = "1.0.1"
[dev-dependencies] [dev-dependencies]
parking_lot = "0.11.1" parking_lot = "0.11.1"

View File

@ -1,8 +1,7 @@
use super::matchers::Matcher; use super::matchers::Matcher;
use crate::{Completer, CompletionContext, Suggestion}; use crate::{Completer, CompletionContext, Suggestion};
use indexmap::set::IndexSet; use indexmap::set::IndexSet;
#[cfg(feature = "is_executable")] #[cfg(not(target_arch = "wasm32"))]
#[allow(unused)]
use is_executable::IsExecutable; use is_executable::IsExecutable;
use nu_test_support::NATIVE_PATH_ENV_VAR; use nu_test_support::NATIVE_PATH_ENV_VAR;
use std::iter::FromIterator; use std::iter::FromIterator;
@ -56,11 +55,12 @@ where
} }
} }
#[cfg(windows)] #[cfg(not(target_arch = "wasm32"))]
fn is_executable(path: &Path) -> bool { fn is_executable(path: &Path) -> bool {
// This call to a crate essentially checks the PATHEXT on Windows and does some // This call to a crate essentially checks the PATHEXT on Windows and does some
// low level WinAPI calls to determine if the file is executable. It seems quite // low level WinAPI calls to determine if the file is executable. It seems quite
// a bit faster than calling path.metadata(). // a bit faster than calling path.metadata().
// On Unix, this checks the file metadata. The underlying code traverses symlinks.
path.is_executable() path.is_executable()
} }
@ -69,22 +69,6 @@ fn is_executable(_path: &Path) -> bool {
false false
} }
#[cfg(unix)]
fn is_executable(path: &Path) -> bool {
use std::os::unix::fs::PermissionsExt;
if let Ok(metadata) = path.metadata() {
let filetype = metadata.file_type();
let permissions = metadata.permissions();
// The file is executable if it is a directory or a symlink and the permissions are set for
// owner, group, or other
(filetype.is_file() || filetype.is_symlink()) && (permissions.mode() & 0o111 != 0)
} else {
false
}
}
// TODO cache these, but watch for changes to PATH // TODO cache these, but watch for changes to PATH
fn find_path_executables() -> Option<IndexSet<String>> { fn find_path_executables() -> Option<IndexSet<String>> {
let path_var = std::env::var_os(NATIVE_PATH_ENV_VAR)?; let path_var = std::env::var_os(NATIVE_PATH_ENV_VAR)?;