Skip to content

Commit e9d1df2

Browse files
authored
which without application param now lists only executables (nushell#16895)
## Release notes summary - What our users need to know The `which` command now filters files found in `$PATH` to include only executables. ## Tasks after submitting --- Done: 1. Optimized search on Windows. 2. Add is_executable check for found files in PATH. Should resolve controversy in nushell#16140
1 parent 8f92e89 commit e9d1df2

File tree

1 file changed

+146
-45
lines changed

1 file changed

+146
-45
lines changed

crates/nu-command/src/system/which_.rs

Lines changed: 146 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use itertools::Itertools;
21
use nu_engine::{command_prelude::*, env};
32
use nu_protocol::engine::CommandType;
3+
use std::collections::HashSet;
44
use std::fs;
55
use std::{ffi::OsStr, path::Path};
66
use which::sys;
@@ -121,44 +121,50 @@ fn list_all_executables(
121121
all: bool,
122122
) -> Vec<Value> {
123123
let decls = engine_state.get_decls_sorted(false);
124-
let commands = decls
125-
.into_iter()
126-
.map(|x| {
127-
let decl = engine_state.get_decl(x.1);
128-
(
129-
String::from_utf8_lossy(&x.0).to_string(),
130-
String::new(),
131-
decl.command_type(),
132-
)
133-
})
134-
.chain(
135-
sys::RealSys
136-
.env_split_paths(paths.as_ref())
137-
.into_iter()
138-
.filter_map(|dir| fs::read_dir(dir).ok())
139-
.flat_map(|entries| entries.flatten())
140-
.map(|entry| entry.path())
141-
.filter(|path| path.is_file())
142-
.filter_map(|path| {
143-
let filename = path.file_name()?.to_string_lossy().to_string();
144-
Some((
145-
filename,
146-
path.to_string_lossy().to_string(),
147-
CommandType::External,
148-
))
149-
}),
150-
);
151-
152-
if all {
153-
commands
154-
.map(|(filename, path, cmd_type)| entry(filename, path, cmd_type, Span::new(0, 0)))
155-
.collect()
156-
} else {
157-
commands
158-
.unique_by(|x| x.0.clone())
159-
.map(|(filename, path, cmd_type)| entry(filename, path, cmd_type, Span::new(0, 0)))
160-
.collect()
124+
125+
let mut results = Vec::with_capacity(decls.len());
126+
let mut seen_commands = HashSet::with_capacity(decls.len());
127+
128+
for (name_bytes, decl_id) in decls {
129+
let name = String::from_utf8_lossy(&name_bytes).to_string();
130+
seen_commands.insert(name.clone());
131+
let decl = engine_state.get_decl(decl_id);
132+
results.push(entry(
133+
name,
134+
String::new(),
135+
decl.command_type(),
136+
Span::unknown(),
137+
));
161138
}
139+
140+
// Add PATH executables
141+
let path_iter = sys::RealSys
142+
.env_split_paths(paths.as_ref())
143+
.into_iter()
144+
.filter_map(|dir| fs::read_dir(dir).ok())
145+
.flat_map(|entries| entries.flatten())
146+
.map(|entry| entry.path())
147+
.filter_map(|path| {
148+
if !path.is_executable() {
149+
return None;
150+
}
151+
let filename = path.file_name()?.to_string_lossy().to_string();
152+
153+
if !all && !seen_commands.insert(filename.clone()) {
154+
return None;
155+
}
156+
157+
let full_path = path.to_string_lossy().to_string();
158+
Some(entry(
159+
filename,
160+
full_path,
161+
CommandType::External,
162+
Span::unknown(),
163+
))
164+
});
165+
166+
results.extend(path_iter);
167+
results
162168
}
163169

164170
#[derive(Debug)]
@@ -233,13 +239,7 @@ fn which(
233239
}
234240

235241
for app in which_args.applications {
236-
let values = which_single(
237-
app,
238-
which_args.all,
239-
engine_state,
240-
cwd.clone(),
241-
paths.clone(),
242-
);
242+
let values = which_single(app, which_args.all, engine_state, &cwd, &paths);
243243
output.extend(values);
244244
}
245245

@@ -257,3 +257,104 @@ mod test {
257257
crate::test_examples(Which)
258258
}
259259
}
260+
261+
// --------------------
262+
// Copied from https://docs.rs/is_executable/ v1.0.5
263+
// Removed path.exists() check in `mod windows`.
264+
265+
/// An extension trait for `std::fs::Path` providing an `is_executable` method.
266+
///
267+
/// See the module documentation for examples.
268+
pub trait IsExecutable {
269+
/// Returns `true` if there is a file at the given path and it is
270+
/// executable. Returns `false` otherwise.
271+
///
272+
/// See the module documentation for details.
273+
fn is_executable(&self) -> bool;
274+
}
275+
276+
#[cfg(unix)]
277+
mod unix {
278+
use std::os::unix::fs::PermissionsExt;
279+
use std::path::Path;
280+
281+
use super::IsExecutable;
282+
283+
impl IsExecutable for Path {
284+
fn is_executable(&self) -> bool {
285+
let metadata = match self.metadata() {
286+
Ok(metadata) => metadata,
287+
Err(_) => return false,
288+
};
289+
let permissions = metadata.permissions();
290+
metadata.is_file() && permissions.mode() & 0o111 != 0
291+
}
292+
}
293+
}
294+
295+
#[cfg(target_os = "windows")]
296+
mod windows {
297+
use std::os::windows::ffi::OsStrExt;
298+
use std::path::Path;
299+
300+
use windows::Win32::Storage::FileSystem::GetBinaryTypeW;
301+
use windows::core::PCWSTR;
302+
303+
use super::IsExecutable;
304+
305+
impl IsExecutable for Path {
306+
fn is_executable(&self) -> bool {
307+
// Check using file extension
308+
if let Some(pathext) = std::env::var_os("PATHEXT") {
309+
if let Some(extension) = self.extension() {
310+
let extension = extension.to_string_lossy();
311+
312+
// Originally taken from:
313+
// https://github.com/nushell/nushell/blob/93e8f6c05e1e1187d5b674d6b633deb839c84899/crates/nu-cli/src/completion/command.rs#L64-L74
314+
return pathext
315+
.to_string_lossy()
316+
.split(';')
317+
// Filter out empty tokens and ';' at the end
318+
.filter(|f| f.len() > 1)
319+
.any(|ext| {
320+
// Cut off the leading '.' character
321+
let ext = &ext[1..];
322+
extension.eq_ignore_ascii_case(ext)
323+
});
324+
}
325+
}
326+
327+
// Check using file properties
328+
// This code is only reached if there is no file extension or retrieving PATHEXT fails
329+
let windows_string: Vec<u16> = self.as_os_str().encode_wide().chain(Some(0)).collect();
330+
let mut binary_type: u32 = 0;
331+
332+
let result =
333+
unsafe { GetBinaryTypeW(PCWSTR(windows_string.as_ptr()), &mut binary_type) };
334+
if result.is_ok() {
335+
if let 0..=6 = binary_type {
336+
return true;
337+
}
338+
}
339+
340+
false
341+
}
342+
}
343+
}
344+
345+
// For WASI, we can't check if a file is executable
346+
// Since wasm and wasi
347+
// is not supposed to add executables ideologically,
348+
// specify them collectively
349+
#[cfg(any(target_os = "wasi", target_family = "wasm"))]
350+
mod wasm {
351+
use std::path::Path;
352+
353+
use super::IsExecutable;
354+
355+
impl IsExecutable for Path {
356+
fn is_executable(&self) -> bool {
357+
false
358+
}
359+
}
360+
}

0 commit comments

Comments
 (0)