Skip to content

Commit 8e5da28

Browse files
committed
Allow job control without controlling terminal
When job control is enabled and the shell does not have a controlling terminal, the shell now proceeds without managing foreground-ness of process groups. Jobs are still assigned to their own process groups. Previously, the shell would abort command execution in this case. Closes #480
1 parent fd61f0d commit 8e5da28

File tree

5 files changed

+71
-15
lines changed

5 files changed

+71
-15
lines changed

yash-builtin/CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ The `wait` built-in no longer treats suspended jobs as terminated jobs.
8383
enabled.
8484
- The `fg::main` function now returns with `Divert::Interrupt` when the resumed
8585
job is suspended.
86+
- The `fg::main` function no longer errors out if `yash_env::Env::get_tty`
87+
fails.
8688
- The `kill::print::print` function now shows signals in the ascending order of
8789
their numbers when given no signals.
8890
- The `read::syntax::parse` function now accepts the `-d` (`--delimiter`) option.

yash-builtin/src/fg.rs

+35-14
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ use yash_env::system::SystemEx as _;
115115
///
116116
/// This function panics if there is no job at the specified index.
117117
async fn resume_job_by_index(env: &mut Env, index: usize) -> Result<ProcessResult, ResumeError> {
118-
let tty = env.get_tty()?;
118+
let tty = env.get_tty().ok();
119119

120120
let job = &env.jobs[index];
121121
if !job.is_owned {
@@ -141,7 +141,9 @@ async fn resume_job_by_index(env: &mut Env, index: usize) -> Result<ProcessResul
141141

142142
// Make sure to put the target job in the foreground before sending the
143143
// SIGCONT signal, or the job may be immediately re-suspended.
144-
env.system.tcsetpgrp_without_block(tty, job.pid)?;
144+
if let Some(tty) = tty {
145+
env.system.tcsetpgrp_without_block(tty, job.pid)?;
146+
}
145147

146148
let pgid = -job.pid;
147149
let sigcont = env.system.signal_number_from_name(signal::Name::Cont);
@@ -152,7 +154,9 @@ async fn resume_job_by_index(env: &mut Env, index: usize) -> Result<ProcessResul
152154
let result = env.wait_for_subshell_to_halt(job.pid).await?.1;
153155

154156
// Move the shell back to the foreground.
155-
env.system.tcsetpgrp_with_block(tty, env.main_pgid)?;
157+
if let Some(tty) = tty {
158+
env.system.tcsetpgrp_with_block(tty, env.main_pgid)?;
159+
}
156160

157161
result
158162
}
@@ -265,10 +269,35 @@ mod tests {
265269
})
266270
}
267271

272+
#[test]
273+
fn resume_job_by_index_resume_job_even_without_tty() {
274+
in_virtual_system(async |mut env, _| {
275+
env.options.set(Monitor, On);
276+
let reached = Rc::new(Cell::new(false));
277+
let reached2 = Rc::clone(&reached);
278+
let subshell = Subshell::new(|env, _| {
279+
Box::pin(async move {
280+
suspend(env).await;
281+
reached2.set(true);
282+
})
283+
})
284+
.job_control(JobControl::Foreground);
285+
let (pid, subshell_result) = subshell.start_and_wait(&mut env).await.unwrap();
286+
assert_eq!(subshell_result, ProcessResult::Stopped(SIGSTOP));
287+
let mut job = Job::new(pid);
288+
job.job_controlled = true;
289+
job.state = subshell_result.into();
290+
let index = env.jobs.add(job);
291+
292+
resume_job_by_index(&mut env, index).await.unwrap();
293+
294+
assert!(reached.get());
295+
})
296+
}
297+
268298
#[test]
269299
fn resume_job_by_index_prints_job_name() {
270300
in_virtual_system(|mut env, state| async move {
271-
stub_tty(&state);
272301
env.options.set(Monitor, On);
273302
let subshell =
274303
Subshell::new(|env, _| Box::pin(suspend(env))).job_control(JobControl::Foreground);
@@ -289,7 +318,6 @@ mod tests {
289318
#[test]
290319
fn resume_job_by_index_returns_after_job_exits() {
291320
in_virtual_system(|mut env, state| async move {
292-
stub_tty(&state);
293321
env.options.set(Monitor, On);
294322
let subshell = Subshell::new(|env, _| {
295323
Box::pin(async move {
@@ -318,7 +346,6 @@ mod tests {
318346
#[test]
319347
fn resume_job_by_index_returns_after_job_suspends() {
320348
in_virtual_system(|mut env, state| async move {
321-
stub_tty(&state);
322349
env.options.set(Monitor, On);
323350
let subshell = Subshell::new(|env, _| {
324351
Box::pin(async move {
@@ -369,7 +396,6 @@ mod tests {
369396
#[test]
370397
fn resume_job_by_index_sends_no_sigcont_to_dead_process() {
371398
let system = VirtualSystem::new();
372-
stub_tty(&system.state);
373399
let mut env = Env::with_system(Box::new(system.clone()));
374400
env.options.set(Monitor, On);
375401
let pid = Pid(123);
@@ -402,7 +428,6 @@ mod tests {
402428
#[test]
403429
fn resume_job_by_index_rejects_unowned_job() {
404430
let system = VirtualSystem::new();
405-
stub_tty(&system.state);
406431
let mut env = Env::with_system(Box::new(system));
407432
env.options.set(Monitor, On);
408433
let mut job = Job::new(Pid(123));
@@ -417,7 +442,6 @@ mod tests {
417442
#[test]
418443
fn resume_job_by_index_rejects_unmonitored_job() {
419444
let system = VirtualSystem::new();
420-
stub_tty(&system.state);
421445
let mut env = Env::with_system(Box::new(system));
422446
env.options.set(Monitor, On);
423447
let index = env.jobs.add(Job::new(Pid(123)));
@@ -484,7 +508,6 @@ mod tests {
484508
#[test]
485509
fn main_with_operand_resumes_specified_job() {
486510
in_virtual_system(|mut env, state| async move {
487-
stub_tty(&state);
488511
env.options.set(Monitor, On);
489512
// previous job
490513
let subshell =
@@ -545,8 +568,7 @@ mod tests {
545568

546569
#[test]
547570
fn main_returns_exit_status_if_job_suspends_if_not_interactive() {
548-
in_virtual_system(|mut env, state| async move {
549-
stub_tty(&state);
571+
in_virtual_system(|mut env, _| async move {
550572
env.options.set(Monitor, On);
551573
env.exit_status = ExitStatus(42);
552574
let subshell = Subshell::new(|env, _| {
@@ -572,8 +594,7 @@ mod tests {
572594

573595
#[test]
574596
fn main_returns_interrupt_if_job_suspends_if_interactive() {
575-
in_virtual_system(|mut env, state| async move {
576-
stub_tty(&state);
597+
in_virtual_system(|mut env, _| async move {
577598
env.options.set(Interactive, On);
578599
env.options.set(Monitor, On);
579600
env.exit_status = ExitStatus(42);

yash-cli/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2929
the next command line. This behavior basically conforms to POSIX.1-2024, but
3030
differs in that the shell does not resume with the remaining commands
3131
following the next asynchronous and-or list.
32+
- If job control is enabled and the shell does not have a controlling terminal,
33+
the shell now proceeds without managing foreground-ness of process groups.
34+
Jobs are still assigned to their own process groups. Previously, the shell
35+
would abort command execution in this case.
3236
- The `cd` built-in now errors out when a given operand is an empty string.
3337
- The `cd` built-in now returns different exit statuses for different errors.
3438
- The `fg` and `bg` built-ins now error out if job control is not enabled.

yash-env/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4545
device file.
4646
- `System::getpwnam_dir` now takes a `&CStr` parameter instead of a `&str`.
4747
- The `builtin::Builtin` struct is now `non_exhaustive`.
48+
- The `subshell::Subshell::start` method no longer fails if `Env::get_tty` fails.
4849
- The `origin` field of the `trap::TrapState` struct is now `trap::Origin`.
4950
- The `TrapSet::get_state` method now returns a `TrapState` reference even if
5051
the current action was not set by the user.

yash-env/src/subshell.rs

+29-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ where
152152
let tty = match job_control {
153153
None | Some(JobControl::Background) => None,
154154
// Open the tty in the parent process so we can reuse the FD for other jobs
155-
Some(JobControl::Foreground) => Some(env.get_tty()?),
155+
Some(JobControl::Foreground) => env.get_tty().ok(),
156156
};
157157
// Block SIGINT and SIGQUIT before forking the child process to prevent
158158
// the child from being killed by those signals until the child starts
@@ -518,6 +518,34 @@ mod tests {
518518
});
519519
}
520520

521+
#[test]
522+
fn job_control_without_tty() {
523+
// When /dev/tty is not available, the shell cannot bring the subshell to
524+
// the foreground. The subshell should still be in a new process group.
525+
// This is the behavior required by POSIX.
526+
in_virtual_system(async |mut parent_env, state| {
527+
parent_env.options.set(Monitor, On);
528+
529+
let state_2 = Rc::clone(&state);
530+
let (child_pid, job_control) = Subshell::new(move |child_env, job_control| {
531+
Box::pin(async move {
532+
let child_pid = child_env.system.getpid();
533+
assert_eq!(state_2.borrow().processes[&child_pid].pgid, child_pid);
534+
assert_eq!(job_control, Some(JobControl::Foreground));
535+
})
536+
})
537+
.job_control(JobControl::Foreground)
538+
.start(&mut parent_env)
539+
.await
540+
.unwrap();
541+
assert_eq!(job_control, Some(JobControl::Foreground));
542+
assert_eq!(state.borrow().processes[&child_pid].pgid, child_pid);
543+
544+
parent_env.wait_for_subshell(child_pid).await.unwrap();
545+
assert_eq!(state.borrow().processes[&child_pid].pgid, child_pid);
546+
})
547+
}
548+
521549
#[test]
522550
fn no_job_control_with_option_disabled() {
523551
in_virtual_system(|mut parent_env, state| async move {

0 commit comments

Comments
 (0)