Skip to content

Commit

Permalink
test
Browse files Browse the repository at this point in the history
  • Loading branch information
reubeno committed Jan 6, 2025
1 parent ae12c14 commit da5fd25
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 65 deletions.
8 changes: 2 additions & 6 deletions brush-core/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,7 @@ impl ExecutionContext<'_> {
/// Returns the file descriptor with the given number.
#[allow(clippy::unwrap_in_result)]
pub fn fd(&self, fd: u32) -> Option<openfiles::OpenFile> {
self.params
.open_files
.files
.get(&fd)
.map(|f| f.try_dup().unwrap())
self.params.open_files.files.get(&fd).cloned()
}

pub(crate) fn should_cmd_lead_own_process_group(&self) -> bool {
Expand Down Expand Up @@ -562,7 +558,7 @@ pub(crate) async fn invoke_command_in_subshell_and_get_output(
subshell
.open_files
.files
.insert(1, openfiles::OpenFile::PipeWriter(writer));
.insert(1, openfiles::OpenFile::PipeWriter(Arc::new(writer)));

let mut params = subshell.default_exec_params();
params.process_group_policy = ProcessGroupPolicy::SameProcessGroup;
Expand Down
26 changes: 14 additions & 12 deletions brush-core/src/interp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,7 @@ fn setup_pipeline_redirection(
// Set up stdin of this process to take stdout of the preceding process.
open_files
.files
.insert(0, OpenFile::PipeReader(preceding_output_reader));
.insert(0, OpenFile::PipeReader(Arc::new(preceding_output_reader)));
} else {
open_files.files.insert(0, OpenFile::Null);
}
Expand All @@ -1266,7 +1266,9 @@ fn setup_pipeline_redirection(
// Set up stdout of this process to go to stdin of the succeeding process.
let (reader, writer) = sys::pipes::pipe()?;
context.output_pipes.push(reader);
open_files.files.insert(1, OpenFile::PipeWriter(writer));
open_files
.files
.insert(1, OpenFile::PipeWriter(Arc::new(writer)));
}

Ok(())
Expand Down Expand Up @@ -1301,8 +1303,8 @@ pub(crate) async fn setup_redirect(
)
})?;

let stdout_file = OpenFile::File(opened_file);
let stderr_file = stdout_file.try_dup()?;
let stdout_file = OpenFile::File(Arc::new(opened_file));
let stderr_file = stdout_file.clone();

open_files.files.insert(1, stdout_file);
open_files.files.insert(2, stderr_file);
Expand Down Expand Up @@ -1382,7 +1384,7 @@ pub(crate) async fn setup_redirect(
err,
)
})?;
target_file = OpenFile::File(opened_file);
target_file = OpenFile::File(Arc::new(opened_file));
}
ast::IoFileRedirectTarget::Fd(fd) => {
let default_fd_if_unspecified = match kind {
Expand All @@ -1396,7 +1398,7 @@ pub(crate) async fn setup_redirect(
fd_num = specified_fd_num.unwrap_or(default_fd_if_unspecified);

if let Some(f) = open_files.files.get(fd) {
target_file = f.try_dup()?;
target_file = f.clone();
} else {
tracing::error!("{}: Bad file descriptor", fd);
return Ok(None);
Expand All @@ -1416,7 +1418,7 @@ pub(crate) async fn setup_redirect(
subshell_cmd,
)?;

target_file = substitution_file.try_dup()?;
target_file = substitution_file.clone();
open_files.files.insert(substitution_fd, substitution_file);

fd_num = specified_fd_num
Expand Down Expand Up @@ -1491,15 +1493,15 @@ fn setup_process_substitution(
subshell
.open_files
.files
.insert(1, openfiles::OpenFile::PipeWriter(writer));
OpenFile::PipeReader(reader)
.insert(1, openfiles::OpenFile::PipeWriter(Arc::new(writer)));
OpenFile::PipeReader(Arc::new(reader))
}
ast::ProcessSubstitutionKind::Write => {
subshell
.open_files
.files
.insert(0, openfiles::OpenFile::PipeReader(reader));
OpenFile::PipeWriter(writer)
.insert(0, openfiles::OpenFile::PipeReader(Arc::new(reader)));
OpenFile::PipeWriter(Arc::new(writer))
}
};

Expand Down Expand Up @@ -1545,5 +1547,5 @@ fn setup_open_file_with_contents(contents: &str) -> Result<OpenFile, error::Erro
writer.write_all(bytes)?;
drop(writer);

Ok(OpenFile::PipeReader(reader))
Ok(OpenFile::PipeReader(Arc::new(reader)))
}
59 changes: 15 additions & 44 deletions brush-core/src/openfiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ use std::os::fd::AsRawFd;
#[cfg(unix)]
use std::os::fd::OwnedFd;
use std::process::Stdio;
use std::sync::Arc;

use crate::error;
use crate::sys;

/// Represents a file open in a shell context.
#[derive(Clone)]
pub enum OpenFile {
/// The original standard input this process was started with.
Stdin,
Expand All @@ -21,35 +23,14 @@ pub enum OpenFile {
/// A null file that discards all input.
Null,
/// A file open for reading or writing.
File(std::fs::File),
File(Arc<std::fs::File>),
/// A read end of a pipe.
PipeReader(sys::pipes::PipeReader),
PipeReader(Arc<sys::pipes::PipeReader>),
/// A write end of a pipe.
PipeWriter(sys::pipes::PipeWriter),
}

impl Clone for OpenFile {
fn clone(&self) -> Self {
self.try_dup().unwrap()
}
PipeWriter(Arc<sys::pipes::PipeWriter>),
}

impl OpenFile {
/// Tries to duplicate the open file.
pub fn try_dup(&self) -> Result<OpenFile, error::Error> {
let result = match self {
OpenFile::Stdin => OpenFile::Stdin,
OpenFile::Stdout => OpenFile::Stdout,
OpenFile::Stderr => OpenFile::Stderr,
OpenFile::Null => OpenFile::Null,
OpenFile::File(f) => OpenFile::File(f.try_clone()?),
OpenFile::PipeReader(f) => OpenFile::PipeReader(f.try_clone()?),
OpenFile::PipeWriter(f) => OpenFile::PipeWriter(f.try_clone()?),
};

Ok(result)
}

/// Converts the open file into an `OwnedFd`.
#[cfg(unix)]
pub(crate) fn into_owned_fd(self) -> Result<std::os::fd::OwnedFd, error::Error> {
Expand All @@ -58,9 +39,9 @@ impl OpenFile {
OpenFile::Stdout => Ok(std::io::stdout().as_fd().try_clone_to_owned()?),
OpenFile::Stderr => Ok(std::io::stderr().as_fd().try_clone_to_owned()?),
OpenFile::Null => error::unimp("to_owned_fd for null open file"),
OpenFile::File(f) => Ok(f.into()),
OpenFile::PipeReader(r) => Ok(OwnedFd::from(r)),
OpenFile::PipeWriter(w) => Ok(OwnedFd::from(w)),
OpenFile::File(f) => Ok((*f).try_clone()?.into()),
OpenFile::PipeReader(r) => Ok(OwnedFd::from((*r).try_clone()?)),
OpenFile::PipeWriter(w) => Ok(OwnedFd::from((*w).try_clone()?)),
}
}

Expand Down Expand Up @@ -137,7 +118,7 @@ impl OpenFile {

impl From<std::fs::File> for OpenFile {
fn from(file: std::fs::File) -> Self {
OpenFile::File(file)
OpenFile::File(Arc::new(file))
}
}

Expand All @@ -148,9 +129,9 @@ impl From<OpenFile> for Stdio {
OpenFile::Stdout => Stdio::inherit(),
OpenFile::Stderr => Stdio::inherit(),
OpenFile::Null => Stdio::null(),
OpenFile::File(f) => f.into(),
OpenFile::PipeReader(f) => f.into(),
OpenFile::PipeWriter(f) => f.into(),
OpenFile::File(f) => (*f).try_clone().unwrap().into(),
OpenFile::PipeReader(f) => (*f).try_clone().unwrap().into(),
OpenFile::PipeWriter(f) => (*f).try_clone().unwrap().into(),
}
}
}
Expand All @@ -169,7 +150,7 @@ impl std::io::Read for OpenFile {
)),
OpenFile::Null => Ok(0),
OpenFile::File(f) => f.read(buf),
OpenFile::PipeReader(reader) => reader.read(buf),
OpenFile::PipeReader(reader) => reader.as_ref().read(buf),
OpenFile::PipeWriter(_) => Err(std::io::Error::new(
std::io::ErrorKind::Other,
error::Error::OpenFileNotReadable("pipe writer"),
Expand All @@ -193,7 +174,7 @@ impl std::io::Write for OpenFile {
std::io::ErrorKind::Other,
error::Error::OpenFileNotWritable("pipe reader"),
)),
OpenFile::PipeWriter(writer) => writer.write(buf),
OpenFile::PipeWriter(writer) => writer.as_ref().write(buf),
}
}

Expand All @@ -205,7 +186,7 @@ impl std::io::Write for OpenFile {
OpenFile::Null => Ok(()),
OpenFile::File(f) => f.flush(),
OpenFile::PipeReader(_) => Ok(()),
OpenFile::PipeWriter(writer) => writer.flush(),
OpenFile::PipeWriter(writer) => writer.as_ref().flush(),
}
}
}
Expand All @@ -230,16 +211,6 @@ impl Default for OpenFiles {
}

impl OpenFiles {
/// Tries to clone the open files.
pub fn try_clone(&self) -> Result<OpenFiles, error::Error> {
let mut files = im::HashMap::new();
for (fd, file) in &self.files {
files.insert(*fd, file.try_dup()?);
}

Ok(OpenFiles { files })
}

/// Retrieves the file backing standard input in this context.
pub fn stdin(&self) -> Option<&OpenFile> {
self.files.get(&0)
Expand Down
6 changes: 3 additions & 3 deletions brush-core/src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ impl Shell {
if let Some(filename) = path_to_open.file_name() {
if let Ok(fd_num) = filename.to_string_lossy().to_string().parse::<u32>() {
if let Some(open_file) = params.open_files.files.get(&fd_num) {
return open_file.try_dup();
return Ok(open_file.clone());
}
}
}
Expand Down Expand Up @@ -1151,13 +1151,13 @@ impl Shell {
/// Returns a value that can be used to write to the shell's currently configured
/// standard output stream using `write!` at al.
pub fn stdout(&self) -> openfiles::OpenFile {
self.open_files.files.get(&1).unwrap().try_dup().unwrap()
self.open_files.files.get(&1).unwrap().clone()
}

/// Returns a value that can be used to write to the shell's currently configured
/// standard error stream using `write!` et al.
pub fn stderr(&self) -> openfiles::OpenFile {
self.open_files.files.get(&2).unwrap().try_dup().unwrap()
self.open_files.files.get(&2).unwrap().clone()
}

/// Outputs `set -x` style trace output for a command.
Expand Down

0 comments on commit da5fd25

Please sign in to comment.