Skip to content

Commit

Permalink
feat: implement command hashing (#206)
Browse files Browse the repository at this point in the history
* Implement hash builtin
* Update simple command resolution to go through hashed paths (when appropriate)
* Simple optimization for executable lookup in completion code path
  • Loading branch information
reubeno authored Oct 15, 2024
1 parent 45d95c1 commit 7976b18
Show file tree
Hide file tree
Showing 15 changed files with 363 additions and 29 deletions.
1 change: 1 addition & 0 deletions brush-core/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ mod factory;
mod false_;
mod fg;
mod getopts;
mod hash;
mod help;
mod jobs;
#[cfg(unix)]
Expand Down
5 changes: 2 additions & 3 deletions brush-core/src/builtins/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl Display for FoundCommand {

impl CommandCommand {
#[allow(clippy::unwrap_in_result)]
fn try_find_command(&self, shell: &shell::Shell) -> Option<FoundCommand> {
fn try_find_command(&self, shell: &mut shell::Shell) -> Option<FoundCommand> {
// Look in path.
if self.command_name.contains(std::path::MAIN_SEPARATOR) {
let candidate_path = shell.get_absolute_path(Path::new(&self.command_name));
Expand All @@ -105,8 +105,7 @@ impl CommandCommand {
}

shell
.find_executables_in_path(self.command_name.as_str())
.first()
.find_first_executable_in_path_using_cache(&self.command_name)
.map(|path| FoundCommand::External(path.to_string_lossy().to_string()))
}
}
Expand Down
2 changes: 1 addition & 1 deletion brush-core/src/builtins/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ pub(crate) fn get_default_builtins(
m.insert("false".into(), builtin::<false_::FalseCommand>());
m.insert("fg".into(), builtin::<fg::FgCommand>());
m.insert("getopts".into(), builtin::<getopts::GetOptsCommand>());
m.insert("hash".into(), builtin::<hash::HashCommand>());
m.insert("help".into(), builtin::<help::HelpCommand>());
m.insert("jobs".into(), builtin::<jobs::JobsCommand>());
#[cfg(unix)]
Expand All @@ -251,7 +252,6 @@ pub(crate) fn get_default_builtins(

// TODO: Unimplemented non-special builtins
m.insert("fc".into(), builtin::<unimp::UnimplementedCommand>());
m.insert("hash".into(), builtin::<unimp::UnimplementedCommand>());
m.insert("ulimit".into(), builtin::<unimp::UnimplementedCommand>());

if !options.sh_mode {
Expand Down
99 changes: 99 additions & 0 deletions brush-core/src/builtins/hash.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
use clap::Parser;
use std::{io::Write, path::PathBuf};

use crate::{builtins, commands};

#[derive(Parser)]
pub(crate) struct HashCommand {
/// Remove entries associated with the given names.
#[arg(short = 'd')]
remove: bool,

/// Display paths in a format usable for input.
#[arg(short = 'l')]
display_as_usable_input: bool,

/// The path to associate with the names.
#[arg(short = 'p')]
path_to_use: Option<PathBuf>,

/// Remove all entries.
#[arg(short = 'r')]
remove_all: bool,

/// Display the paths associated with the names.
#[arg(short = 't')]
display_paths: bool,

/// Names to process.
names: Vec<String>,
}

impl builtins::Command for HashCommand {
async fn execute(
&self,
context: commands::ExecutionContext<'_>,
) -> Result<crate::builtins::ExitCode, crate::error::Error> {
let mut result = builtins::ExitCode::Success;

if self.remove_all {
context.shell.program_location_cache.reset();
} else if self.remove {
for name in &self.names {
if !context.shell.program_location_cache.unset(name) {
writeln!(context.stderr(), "{name}: not found")?;
result = builtins::ExitCode::Custom(1);
}
}
} else if self.display_paths {
for name in &self.names {
if let Some(path) = context.shell.program_location_cache.get(name) {
if self.display_as_usable_input {
writeln!(
context.stdout(),
"builtin hash -p {} {name}",
path.to_string_lossy()
)?;
} else {
let mut prefix = String::new();

if self.names.len() > 1 {
prefix.push_str(name.as_str());
prefix.push('\t');
}

writeln!(
context.stdout(),
"{prefix}{}",
path.to_string_lossy().as_ref()
)?;
}
} else {
writeln!(context.stderr(), "{name}: not found")?;
result = builtins::ExitCode::Custom(1);
}
}
} else if let Some(path) = &self.path_to_use {
for name in &self.names {
context.shell.program_location_cache.set(name, path.clone());
}
} else {
for name in &self.names {
// Remove from the cache if already hashed.
let _ = context.shell.program_location_cache.unset(name);

// Hash the path.
if context
.shell
.find_first_executable_in_path_using_cache(name)
.is_none()
{
writeln!(context.stderr(), "{name}: not found")?;
result = builtins::ExitCode::Custom(1);
}
}
}

Ok(result)
}
}
3 changes: 2 additions & 1 deletion brush-core/src/builtins/printf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ impl PrintfCommand {
}

fn evaluate_format_with_percent_q(prefix: Option<&str>, arg: &str) -> String {
let mut result = escape::quote_if_needed(arg, escape::QuoteMode::BackslashEscape);
let mut result =
escape::quote_if_needed(arg, escape::QuoteMode::BackslashEscape).to_string();

if let Some(prefix) = prefix {
result.insert_str(0, prefix);
Expand Down
36 changes: 29 additions & 7 deletions brush-core/src/builtins/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ enum ResolvedType {
Keyword,
Function(Arc<ast::FunctionDefinition>),
Builtin,
File(PathBuf),
File { path: PathBuf, hashed: bool },
}

impl builtins::Command for TypeCommand {
Expand All @@ -64,7 +64,7 @@ impl builtins::Command for TypeCommand {
}

for resolved_type in resolved_types {
if self.show_path_only && !matches!(resolved_type, ResolvedType::File(_)) {
if self.show_path_only && !matches!(resolved_type, ResolvedType::File { .. }) {
// Do nothing.
} else if self.type_only {
match resolved_type {
Expand All @@ -80,7 +80,7 @@ impl builtins::Command for TypeCommand {
ResolvedType::Builtin => {
writeln!(context.stdout(), "builtin")?;
}
ResolvedType::File(path) => {
ResolvedType::File { path, .. } => {
if self.show_path_only || self.force_path_search {
writeln!(context.stdout(), "{}", path.to_string_lossy())?;
} else {
Expand All @@ -103,9 +103,21 @@ impl builtins::Command for TypeCommand {
ResolvedType::Builtin => {
writeln!(context.stdout(), "{name} is a shell builtin")?;
}
ResolvedType::File(path) => {
if self.show_path_only || self.force_path_search {
ResolvedType::File { path, hashed } => {
if hashed && self.all_locations && !self.force_path_search {
// Do nothing.
} else if self.show_path_only || self.force_path_search {
writeln!(context.stdout(), "{}", path.to_string_lossy())?;
if hashed {
break;
}
} else if hashed {
writeln!(
context.stdout(),
"{name} is hashed ({path})",
name = name,
path = path.to_string_lossy()
)?;
} else {
writeln!(
context.stdout(),
Expand Down Expand Up @@ -160,11 +172,21 @@ impl TypeCommand {
// Look in path.
if name.contains(std::path::MAIN_SEPARATOR) {
if shell.get_absolute_path(Path::new(name)).executable() {
types.push(ResolvedType::File(PathBuf::from(name)));
types.push(ResolvedType::File {
path: PathBuf::from(name),
hashed: false,
});
}
} else {
if let Some(path) = shell.program_location_cache.get(name) {
types.push(ResolvedType::File { path, hashed: true });
}

for item in shell.find_executables_in_path(name) {
types.push(ResolvedType::File(item));
types.push(ResolvedType::File {
path: item,
hashed: false,
});
}
}

Expand Down
38 changes: 31 additions & 7 deletions brush-core/src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#[cfg(unix)]
use std::os::unix::process::CommandExt;
use std::{ffi::OsStr, fmt::Display, process::Stdio, sync::Arc};
use std::{borrow::Cow, ffi::OsStr, fmt::Display, process::Stdio, sync::Arc};

use brush_parser::ast;
#[cfg(unix)]
Expand Down Expand Up @@ -178,7 +178,7 @@ impl From<&String> for CommandArg {
}

impl CommandArg {
pub fn quote_for_tracing(&self) -> String {
pub fn quote_for_tracing(&self) -> Cow<'_, str> {
match self {
CommandArg::String(s) => escape::quote_if_needed(s, escape::QuoteMode::Quote),
CommandArg::Assignment(a) => {
Expand All @@ -189,7 +189,7 @@ impl CommandArg {
a.value.to_string().as_str(),
escape::QuoteMode::Quote,
));
s
s.into()
}
}
}
Expand Down Expand Up @@ -320,15 +320,39 @@ pub(crate) async fn execute(
return execute_builtin_command(&builtin, cmd_context, args).await;
}
}
}

// Strip the command name off args.
execute_external_command(cmd_context, process_group_id, &args[1..])
if let Some(path) = cmd_context
.shell
.find_first_executable_in_path_using_cache(&cmd_context.command_name)
{
let resolved_path = path.to_string_lossy();
execute_external_command(
cmd_context,
resolved_path.as_ref(),
process_group_id,
&args[1..],
)
} else {
tracing::error!("{}: command not found", cmd_context.command_name);
Ok(CommandSpawnResult::ImmediateExit(127))
}
} else {
let resolved_path = cmd_context.command_name.clone();

// Strip the command name off args.
execute_external_command(
cmd_context,
resolved_path.as_str(),
process_group_id,
&args[1..],
)
}
}

#[allow(clippy::too_many_lines)]
pub(crate) fn execute_external_command(
context: ExecutionContext<'_>,
executable_path: &str,
process_group_id: &mut Option<i32>,
args: &[CommandArg],
) -> Result<CommandSpawnResult, error::Error> {
Expand All @@ -355,7 +379,7 @@ pub(crate) fn execute_external_command(
#[allow(unused_mut)]
let mut cmd = compose_std_command(
context.shell,
context.command_name.as_str(),
executable_path,
context.command_name.as_str(),
cmd_args.as_slice(),
context.params.open_files,
Expand Down
4 changes: 4 additions & 0 deletions brush-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ pub enum Error {
#[error("function not found: {0}")]
FunctionNotFound(String),

/// Command was not found.
#[error("command not found: {0}")]
CommandNotFound(String),

/// The requested functionality has not yet been implemented in this shell.
#[error("UNIMPLEMENTED: {0}")]
Unimplemented(&'static str),
Expand Down
22 changes: 14 additions & 8 deletions brush-core/src/escape.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use itertools::Itertools;

use crate::error;
Expand Down Expand Up @@ -171,14 +173,18 @@ pub(crate) enum QuoteMode {
Quote,
}

pub(crate) fn quote_if_needed<S: AsRef<str>>(s: S, mode: QuoteMode) -> String {
pub(crate) fn quote_if_needed(s: &str, mode: QuoteMode) -> Cow<'_, str> {
match mode {
QuoteMode::BackslashEscape => escape_with_backslash(s.as_ref()),
QuoteMode::Quote => escape_with_quoting(s.as_ref()),
QuoteMode::BackslashEscape => escape_with_backslash(s),
QuoteMode::Quote => escape_with_quoting(s),
}
}

fn escape_with_backslash(s: &str) -> String {
fn escape_with_backslash(s: &str) -> Cow<'_, str> {
if !s.chars().any(needs_escaping) {
return s.into();
}

let mut output = String::new();

// TODO: Handle other interesting sequences.
Expand All @@ -192,15 +198,15 @@ fn escape_with_backslash(s: &str) -> String {
}
}

output
output.into()
}

fn escape_with_quoting(s: &str) -> String {
fn escape_with_quoting(s: &str) -> Cow<'_, str> {
// TODO: Handle single-quote!
if s.is_empty() || s.chars().any(needs_escaping) {
std::format!("'{s}'")
std::format!("'{s}'").into()
} else {
s.to_owned()
s.into()
}
}

Expand Down
1 change: 1 addition & 0 deletions brush-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ mod keywords;
mod namedoptions;
mod openfiles;
mod options;
mod pathcache;
mod patterns;
mod processes;
mod prompt;
Expand Down
Loading

0 comments on commit 7976b18

Please sign in to comment.