From d2f4d91960825416b8bc0db365a225e3cc6eaee3 Mon Sep 17 00:00:00 2001 From: feniljain Date: Sun, 26 Mar 2023 16:44:11 +0530 Subject: [PATCH 1/3] feat: implement jobs builtin command - added some resources - added some more tasks --- RESOURCES.md | 2 ++ TASKS_AND_BUGS.md | 6 ++++- src/command/mod.rs | 8 +++++++ src/engine.rs | 57 +++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/RESOURCES.md b/RESOURCES.md index df3cfe7..da46fe7 100644 --- a/RESOURCES.md +++ b/RESOURCES.md @@ -7,3 +7,5 @@ - https://www.howtogeek.com/440848/how-to-run-and-control-background-processes-on-linux/ - https://wiki.bash-hackers.org/howto/redirection_tutorial - https://stackoverflow.com/questions/8319484/regarding-background-processes-using-fork-and-child-processes-in-my-dummy-shel +- Job Control Signals: https://www.gnu.org/software/libc/manual/html_node/Job-Control-Signals.html +- Implementing a Job Control Shell: https://www.andrew.cmu.edu/course/15-310/applications/homework/homework4/lab4.pdf diff --git a/TASKS_AND_BUGS.md b/TASKS_AND_BUGS.md index 60d0f98..ed137d5 100644 --- a/TASKS_AND_BUGS.md +++ b/TASKS_AND_BUGS.md @@ -32,8 +32,12 @@ - [X] add support for redirect <& operator - [X] add support for fd in redirect operations - [ ] add support for $() in shell -- [ ] add support for & parsing for job control +- [ ] add support for background processes +- [X] add support for `jobs` command +- [ ] add support to bring commands in foreground again +- [ ] add support for proper process chains i.e. jobs - [ ] add support for another execution mode (job control) in engine +- [ ] seperate types from engine, and into a new file ## Bonus Tasks: diff --git a/src/command/mod.rs b/src/command/mod.rs index 52f6bbb..8f25666 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -27,6 +27,14 @@ impl Command { }) .collect() } + + pub fn as_string(&self) -> String { + self.tokens.iter().fold(String::new(), |mut acc, token| { + acc += " "; + acc += &token.lexeme; + return acc; + }) + } } // Old Lexing + Parsing diff --git a/src/engine.rs b/src/engine.rs index fa7a21a..84470ab 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -6,7 +6,7 @@ use nix::{ stat::Mode, wait::{waitpid, WaitStatus}, }, - unistd::{chdir, close, dup2, execve, fork, pipe, setpgid, ForkResult, Pid, getpid}, + unistd::{chdir, close, dup2, execve, fork, getpid, pipe, setpgid, ForkResult, Pid}, }; use signal_hook::consts; @@ -14,6 +14,7 @@ use std::{ collections::HashMap, convert::Infallible, ffi::{CStr, CString}, + fmt::{Display, Formatter}, io, os::unix::prelude::OsStrExt, path::{Path, PathBuf}, @@ -35,7 +36,41 @@ use crate::{ frontend::{write_error_to_shell, write_to_stderr, write_to_stdout, Prompt}, }; -const BUILTIN_COMMANDS: [&str; 2] = ["cd", "exec"]; +const BUILTIN_COMMANDS: [&str; 3] = ["cd", "exec", "jobs"]; + +#[derive(Clone, Debug)] +pub enum ProcessStatus { + Running, + Suspended +} + +impl Display for ProcessStatus { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + ProcessStatus::Running => write!(f, "running"), + ProcessStatus::Suspended => write!(f, "suspended"), + } + } +} + +#[derive(Clone, Debug)] +pub struct Process { + pid: Pid, + cmd: String, + status: ProcessStatus, +} + +impl Process { + fn new(pid: Pid, cmd: String, status: ProcessStatus) -> Self { + Process { pid, cmd, status } + } +} + +impl Display for Process { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{} {} {}", self.pid, self.status, self.cmd) + } +} #[derive(Clone, Debug)] pub struct Engine { @@ -44,6 +79,7 @@ pub struct Engine { execution_mode: ExecutionMode, // Operations to be done on different `fd`s fds_ops: HashMap, + pub job_processes: Vec, } #[derive(Copy, Clone, Debug)] @@ -68,6 +104,7 @@ impl Engine { env_paths: parse_paths(), execution_mode: ExecutionMode::Normal, fds_ops: HashMap::new(), + job_processes: Vec::new(), } } @@ -341,6 +378,17 @@ impl Engine { self.parse_and_execute(&command.tokens)?; Ok(()) } + "jobs" => { + self.job_processes + .iter() + .enumerate() + .for_each(|(idx, process)| { + // FIXME: use panic safe write_to_stdout here + println!("{} {}", idx, process); + }); + + Ok(()) + } _ => Err(ShellError::CommandNotFound(cmd_str.to_string()).into()), } } @@ -356,7 +404,10 @@ impl Engine { child: child_pid, .. }) => { if matches!(self.execution_mode, ExecutionMode::Background) { - setpgid(child_pid, child_pid)?; + setpgid(child_pid, child_pid)?; + let command = command.expect("invalid state: should have had command in background process execution flow"); + self.job_processes + .push(Process::new(child_pid, command.as_string(), ProcessStatus::Running)); } for (fd, value) in &self.fds_ops { From f885d49996de99d7ec65baf4d737050b1d6a68d2 Mon Sep 17 00:00:00 2001 From: feniljain Date: Wed, 29 Mar 2023 11:49:52 +0530 Subject: [PATCH 2/3] wip: fg command --- src/engine.rs | 79 ++++++++++++++++++++++++++++++++++++++++++++------- src/errors.rs | 2 ++ src/main.rs | 2 +- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 84470ab..5e8f173 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -6,7 +6,7 @@ use nix::{ stat::Mode, wait::{waitpid, WaitStatus}, }, - unistd::{chdir, close, dup2, execve, fork, getpid, pipe, setpgid, ForkResult, Pid}, + unistd::{chdir, close, dup2, execve, fork, getpid, isatty, pipe, setpgid, ForkResult, Pid, tcsetpgrp}, }; use signal_hook::consts; @@ -36,19 +36,20 @@ use crate::{ frontend::{write_error_to_shell, write_to_stderr, write_to_stdout, Prompt}, }; -const BUILTIN_COMMANDS: [&str; 3] = ["cd", "exec", "jobs"]; +const BUILTIN_COMMANDS: [&str; 4] = ["cd", "exec", "jobs", "fg"]; #[derive(Clone, Debug)] pub enum ProcessStatus { Running, - Suspended + // Suspended + // Done, } impl Display for ProcessStatus { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { ProcessStatus::Running => write!(f, "running"), - ProcessStatus::Suspended => write!(f, "suspended"), + // ProcessStatus::Suspended => write!(f, "suspended"), } } } @@ -72,6 +73,11 @@ impl Display for Process { } } +// #[derive(Clone, Debug)] +// pub struct Job { +// processes: Vec +// } + #[derive(Clone, Debug)] pub struct Engine { pub execution_successful: bool, @@ -79,7 +85,8 @@ pub struct Engine { execution_mode: ExecutionMode, // Operations to be done on different `fd`s fds_ops: HashMap, - pub job_processes: Vec, + job_processes: Vec, + tty_fd: i32, } #[derive(Copy, Clone, Debug)] @@ -98,14 +105,46 @@ enum ExecutionMode { } impl Engine { - pub fn new() -> Self { - Self { + pub fn new() -> anyhow::Result { + let tty_fd; + match open("/dev/tty", OFlag::O_RDWR, Mode::S_IRWXU) { + Ok(fd) => { + tty_fd = fd; + } + Err(_) => { + // Comment from busybox: + /* BTW, bash will try to open(ttyname(0)) if open("/dev/tty") fails. + * That sometimes helps to acquire controlling tty. + * Obviously, a workaround for bugs when someone + * failed to provide a controlling tty to bash! :) */ + + let mut fd = 2; + + loop { + if isatty(fd)? { + tty_fd = fd; + break; + } + + fd -= 1; + if fd < 0 { + return Err(ShellError::EngineError( + "can't access tty; job control turned off".into(), + ) + .into()); + } + } + } + } + + Ok(Self { execution_successful: true, env_paths: parse_paths(), execution_mode: ExecutionMode::Normal, fds_ops: HashMap::new(), job_processes: Vec::new(), - } + tty_fd, + }) } pub fn fire_on(&mut self) -> anyhow::Result<()> { @@ -389,6 +428,20 @@ impl Engine { Ok(()) } + "fg" => { + let pgrp = match self.job_processes.first() { + Some(first_process) => { + println!("first process pid: {}", first_process.pid); + first_process.pid + } + None => { + getpid() + } + }; + + tcsetpgrp(self.tty_fd, pgrp)?; + Ok(()) + } _ => Err(ShellError::CommandNotFound(cmd_str.to_string()).into()), } } @@ -406,8 +459,11 @@ impl Engine { if matches!(self.execution_mode, ExecutionMode::Background) { setpgid(child_pid, child_pid)?; let command = command.expect("invalid state: should have had command in background process execution flow"); - self.job_processes - .push(Process::new(child_pid, command.as_string(), ProcessStatus::Running)); + self.job_processes.push(Process::new( + child_pid, + command.as_string(), + ProcessStatus::Running, + )); } for (fd, value) in &self.fds_ops { @@ -590,7 +646,8 @@ mod tests { } fn check(input_str: &str) -> Engine { - let mut engine = Engine::new(); + let mut engine = + Engine::new().expect("engine should have been able to be created successfully"); let ip_str = input_str.to_string() + "\n"; let lexer = get_tokens(&ip_str).expect("lexer failed, check lexer tests"); diff --git a/src/errors.rs b/src/errors.rs index 5b0bab1..95e2ba3 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -10,6 +10,8 @@ pub enum ShellError { LexError(LexError), #[error("dss: internal error [BUG]: {0}\n")] InternalError(String), + #[error("dss: engine error: {0}\n")] + EngineError(String), } #[derive(Error, Debug)] diff --git a/src/main.rs b/src/main.rs index c6490a4..6464de8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,7 @@ use engine::Engine; // FIXME: Refine APIs exposed by Engine and Command fn main() -> anyhow::Result<()> { - let mut engine = Engine::new(); + let mut engine = Engine::new()?; engine.fire_on()?; From a14fdb16b874f055b1dbb787f6072e9beac2dcfe Mon Sep 17 00:00:00 2001 From: feniljain Date: Thu, 20 Jul 2023 22:39:43 +0530 Subject: [PATCH 3/3] wip: shot at using GNU impl guide for job control --- src/engine.rs | 198 ++++++++++++++++++++++++++++++++------------------ src/main.rs | 4 +- 2 files changed, 130 insertions(+), 72 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 5e8f173..5e581b8 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -6,7 +6,9 @@ use nix::{ stat::Mode, wait::{waitpid, WaitStatus}, }, - unistd::{chdir, close, dup2, execve, fork, getpid, isatty, pipe, setpgid, ForkResult, Pid, tcsetpgrp}, + unistd::{ + chdir, close, dup2, execve, fork, getpid, isatty, pipe, setpgid, tcsetpgrp, ForkResult, Pid, + }, }; use signal_hook::consts; @@ -73,19 +75,30 @@ impl Display for Process { } } -// #[derive(Clone, Debug)] -// pub struct Job { -// processes: Vec -// } +#[derive(Clone, Debug)] +pub struct Job { + processes: Vec, + pgrp: Pid, +} + +impl Job { + fn new() -> Self { + Self { + processes: Vec::new(), + pgrp: Pid::from_raw(0), + } + } +} #[derive(Clone, Debug)] pub struct Engine { pub execution_successful: bool, pub env_paths: Vec, execution_mode: ExecutionMode, + pub is_interactive: bool, // Operations to be done on different `fd`s fds_ops: HashMap, - job_processes: Vec, + job: Job, tty_fd: i32, } @@ -105,45 +118,17 @@ enum ExecutionMode { } impl Engine { - pub fn new() -> anyhow::Result { - let tty_fd; - match open("/dev/tty", OFlag::O_RDWR, Mode::S_IRWXU) { - Ok(fd) => { - tty_fd = fd; - } - Err(_) => { - // Comment from busybox: - /* BTW, bash will try to open(ttyname(0)) if open("/dev/tty") fails. - * That sometimes helps to acquire controlling tty. - * Obviously, a workaround for bugs when someone - * failed to provide a controlling tty to bash! :) */ - - let mut fd = 2; - - loop { - if isatty(fd)? { - tty_fd = fd; - break; - } - - fd -= 1; - if fd < 0 { - return Err(ShellError::EngineError( - "can't access tty; job control turned off".into(), - ) - .into()); - } - } - } - } + pub fn new(is_interactive: bool) -> anyhow::Result { + let tty_fd = get_tty_fd()?; Ok(Self { execution_successful: true, env_paths: parse_paths(), execution_mode: ExecutionMode::Normal, fds_ops: HashMap::new(), - job_processes: Vec::new(), + job: Job::new(), tty_fd, + is_interactive, }) } @@ -418,7 +403,8 @@ impl Engine { Ok(()) } "jobs" => { - self.job_processes + self.job + .processes .iter() .enumerate() .for_each(|(idx, process)| { @@ -429,16 +415,24 @@ impl Engine { Ok(()) } "fg" => { - let pgrp = match self.job_processes.first() { + let pgrp = match self.job.processes.first() { Some(first_process) => { println!("first process pid: {}", first_process.pid); first_process.pid } - None => { - getpid() - } + None => getpid(), }; + let curr_pid = getpid(); + println!( + "pid received from getpid for shell in foreground: {}", + curr_pid + ); + + // let curr_tty_pid = tcgetattr(self.tty_fd)?; + // curr_tty_pid. + + println!("TTY FD: {}", self.tty_fd); tcsetpgrp(self.tty_fd, pgrp)?; Ok(()) } @@ -456,15 +450,24 @@ impl Engine { Ok(ForkResult::Parent { child: child_pid, .. }) => { - if matches!(self.execution_mode, ExecutionMode::Background) { - setpgid(child_pid, child_pid)?; - let command = command.expect("invalid state: should have had command in background process execution flow"); - self.job_processes.push(Process::new( + if self.is_interactive { + if self.job.processes.len() == 0 { + self.job.pgrp = child_pid; + } + + let command = command.expect( + "invalid state: should have had command in background process execution flow", + ); + self.job.processes.push(Process::new( child_pid, command.as_string(), ProcessStatus::Running, )); + + setpgid(child_pid, self.job.pgrp)?; } + // if matches!(self.execution_mode, ExecutionMode::Background) { + // } for (fd, value) in &self.fds_ops { match value { @@ -493,6 +496,7 @@ impl Engine { // condition and let it wait on each command execution if !matches!(self.execution_mode, ExecutionMode::Pipeline) && !matches!(self.execution_mode, ExecutionMode::Background) + && !self.is_interactive { let wait_status = waitpid(child_pid, None).expect(&format!( "Expected to wait for child with pid: {:?}", @@ -516,34 +520,49 @@ impl Engine { } } } - Ok(ForkResult::Child) => match execute_mode { - ExecuteMode::Normal => { - if matches!(self.execution_mode, ExecutionMode::Background) { - let pgrp = getpid(); - setpgid(Pid::from_raw(0), pgrp)?; - } + Ok(ForkResult::Child) => { + self.tty_fd = get_tty_fd()?; + match execute_mode { + ExecuteMode::Normal => { + if self.is_interactive { + let curr_pid = getpid(); + + if self.job.processes.len() == 0 { + // first process + self.job.pgrp = curr_pid; + } - let command = - command.expect("internal error: should have contained valid command"); + println!("pgrp: {}", self.job.pgrp); + // passing frist as 0, makes it take it's own + // pid by default + setpgid(curr_pid, self.job.pgrp)?; + } + // tcsetpgrp(self.tty_fd, pgrp)?; - for (fd, op) in &self.fds_ops { - match op { - FdOperation::Set { to } => { - dup2(*to, *fd)?; - close(*to)?; - } - FdOperation::Close => { - close(*fd)?; + // if matches!(self.execution_mode, ExecutionMode::Background) {} + + let command = + command.expect("internal error: should have contained valid command"); + + for (fd, op) in &self.fds_ops { + match op { + FdOperation::Set { to } => { + dup2(*to, *fd)?; + close(*to)?; + } + FdOperation::Close => { + close(*fd)?; + } } } - } - execute_external_cmd(command.clone(), self.env_paths.clone())?; - } - ExecuteMode::Subshell(tokens) => { - self.parse_and_execute(&tokens)?; + execute_external_cmd(command.clone(), self.env_paths.clone())?; + } + ExecuteMode::Subshell(tokens) => { + self.parse_and_execute(&tokens)?; + } } - }, + } Err(err) => panic!("Fork failed: {err:?}"), } @@ -629,6 +648,42 @@ fn parse_paths() -> Vec { .collect(); } +fn get_tty_fd() -> anyhow::Result { + let tty_fd; + + match open("/dev/tty", OFlag::O_RDWR, Mode::S_IRWXU) { + Ok(fd) => { + tty_fd = fd; + } + Err(_) => { + // Comment from busybox: + /* BTW, bash will try to open(ttyname(0)) if open("/dev/tty") fails. + * That sometimes helps to acquire controlling tty. + * Obviously, a workaround for bugs when someone + * failed to provide a controlling tty to bash! :) */ + + let mut fd = 2; + + loop { + if isatty(fd)? { + tty_fd = fd; + break; + } + + fd -= 1; + if fd < 0 { + return Err(ShellError::EngineError( + "can't access tty; job control turned off".into(), + ) + .into()); + } + } + } + } + + Ok(tty_fd) +} + #[cfg(test)] mod tests { use crate::command::lexer::Lexer; @@ -646,8 +701,9 @@ mod tests { } fn check(input_str: &str) -> Engine { - let mut engine = - Engine::new().expect("engine should have been able to be created successfully"); + let is_interactive = true; + let mut engine = Engine::new(is_interactive) + .expect("engine should have been able to be created successfully"); let ip_str = input_str.to_string() + "\n"; let lexer = get_tokens(&ip_str).expect("lexer failed, check lexer tests"); diff --git a/src/main.rs b/src/main.rs index 6464de8..1ed3eea 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,9 @@ use engine::Engine; // FIXME: Refine APIs exposed by Engine and Command fn main() -> anyhow::Result<()> { - let mut engine = Engine::new()?; + // FIXME: calculate this using process described here: https://www.gnu.org/software/libc/manual/html_node/Initializing-the-Shell.html + let is_interactive = true; + let mut engine = Engine::new(is_interactive)?; engine.fire_on()?;