Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include initially ignored traps in trap built-in output #450

Merged
merged 17 commits into from
Feb 5, 2025
+918 −344
Merged

Include initially ignored traps in trap built-in output #450

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e835f71
Document trap built-in's -p option
magicant Jan 29, 2025
4598c6a
System::get_sigaction
magicant Jan 29, 2025
26636a7
SignalSystem::get_disposition
magicant Jan 29, 2025
11b3095
Clean up the documentation of the trap module
magicant Jan 29, 2025
bd5dd72
Make TrapState::origin optional
magicant Jan 29, 2025
c1be124
Retain TrapState in GrandState
magicant Jan 30, 2025
9ba7989
Remove unused Setting enum
magicant Jan 30, 2025
502fef8
Rename clear_parent_settings to clear_parent_states
magicant Jan 30, 2025
d78af38
Change TrapState::origin to Origin
magicant Jan 31, 2025
988dc4b
Set TrapState::origin to Subshell when entering a subshell
magicant Jan 31, 2025
b9c9579
Reflect EnterSubshellOption::Ignore to current_state.action
magicant Jan 31, 2025
352df03
Set correct origin in ignored TrapState
magicant Feb 1, 2025
73bce48
Always return current state from GrandState::get_state
magicant Feb 2, 2025
28abcfe
Replace GrandState::get_state with current_state and parent_state
magicant Feb 2, 2025
5200185
TrapSet::peek_state
magicant Feb 2, 2025
e013042
Accept Disposition::Catch as default for SIGSEGV and SIGBUS
magicant Feb 5, 2025
3c09339
Include initially ignored traps in trap built-in output
magicant Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Set TrapState::origin to Subshell when entering a subshell 
This commit changes the origin of the current trap state to Subshell
when entering a subshell. Previously, the origin was set to Inherited
for backward compatibility but was incorrect. This commit also updates
the tests to reflect the new behavior. Especially, TrapSet::get_state
now returns a reference to the current state instead of None after
entering a subshell.
@magicant
magicant committed Jan 31, 2025
commit 988dc4b89bc0a9415810cb2d8c3885c095ad6f83
4 changes: 2 additions & 2 deletions yash-builtin/src/trap.rs
View file
Original file line number Diff line number Diff line change
@@ -187,8 +187,8 @@ pub fn display_traps<S: SignalSystem>(traps: &TrapSet, system: &S) -> String {
let mut output = String::new();
for (cond, current, parent) in traps {
let trap = match (current, parent) {
(Some(trap), _) => trap,
(None, Some(trap)) => trap,
(_, Some(trap)) => trap,
(Some(trap), None) => trap,
(None, None) => continue,
};
let command = match &trap.action {
8 changes: 3 additions & 5 deletions yash-env/src/subshell.rs
View file
Original file line number Diff line number Diff line change
@@ -407,12 +407,10 @@ mod tests {
.unwrap();
let subshell = Subshell::new(|env, _job_control| {
Box::pin(async {
let trap_state = assert_matches!(
env.traps.get_state(SIGCHLD),
(None, Some(trap_state)) => trap_state
);
let (current, parent) = env.traps.get_state(SIGCHLD);
assert_eq!(current.unwrap().action, Action::Default);
assert_matches!(
&trap_state.action,
&parent.unwrap().action,
Action::Command(body) => assert_eq!(&**body, "echo foo")
);
})
98 changes: 83 additions & 15 deletions yash-env/src/trap.rs
View file
Original file line number Diff line number Diff line change
@@ -625,7 +625,8 @@ mod tests {
assert_eq!(first.2, None);
let second = i.next().unwrap();
assert_eq!(second.0, &Condition::Signal(SIGUSR2));
assert_eq!(second.1, None);
assert_eq!(second.1.unwrap().action, Action::Default);
assert_eq!(second.1.unwrap().origin, Origin::Subshell);
assert_eq!(second.2.unwrap().action, command);
assert_eq!(second.2.unwrap().origin, Origin::User(origin_2));
assert_eq!(i.next(), None);
@@ -655,10 +656,15 @@ mod tests {

let mut i = trap_set.iter();
let first = i.next().unwrap();
assert_eq!(first.0, &Condition::Signal(SIGUSR2));
assert_eq!(first.1.unwrap().action, command);
assert_eq!(first.1.unwrap().origin, Origin::User(origin_2));
assert_eq!(first.0, &Condition::Signal(SIGUSR1));
assert_eq!(first.1.unwrap().action, Action::Default);
assert_eq!(first.1.unwrap().origin, Origin::Subshell);
assert_eq!(first.2, None);
let second = i.next().unwrap();
assert_eq!(second.0, &Condition::Signal(SIGUSR2));
assert_eq!(second.1.unwrap().action, command);
assert_eq!(second.1.unwrap().origin, Origin::User(origin_2));
assert_eq!(second.2, None);
assert_eq!(i.next(), None);
}

@@ -676,7 +682,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGCHLD),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -728,7 +738,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGCHLD),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -756,7 +770,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGINT),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -784,7 +802,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGTERM),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -812,7 +834,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGQUIT),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -840,7 +866,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGTSTP),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -868,7 +898,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGTTIN),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -896,7 +930,11 @@ mod tests {
assert_eq!(
trap_set.get_state(SIGTTOU),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -946,7 +984,17 @@ mod tests {
None
)
);
assert_eq!(trap_set.get_state(SIGUSR2), (None, None));
assert_eq!(
trap_set.get_state(SIGUSR2),
(
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
None
)
);
assert_eq!(system.0[&SIGUSR1], Disposition::Catch);
assert_eq!(system.0[&SIGUSR2], Disposition::Default);
}
@@ -968,8 +1016,28 @@ mod tests {
trap_set.enter_subshell(&mut system, false, false);
trap_set.enter_subshell(&mut system, false, false);

assert_eq!(trap_set.get_state(SIGUSR1), (None, None));
assert_eq!(trap_set.get_state(SIGUSR2), (None, None));
assert_eq!(
trap_set.get_state(SIGUSR1),
(
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
None
)
);
assert_eq!(
trap_set.get_state(SIGUSR2),
(
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
None
)
);
assert_eq!(system.0[&SIGUSR1], Disposition::Default);
assert_eq!(system.0[&SIGUSR2], Disposition::Default);
}
38 changes: 32 additions & 6 deletions yash-env/src/trap/state.rs
View file
Original file line number Diff line number Diff line change
@@ -320,7 +320,7 @@ impl GrandState {
&mut self.current_state,
TrapState {
action: Action::Default,
origin: Origin::Inherited, // TODO Should be Origin::Subshell
origin: Origin::Subshell,
pending: false,
},
));
@@ -795,7 +795,11 @@ mod tests {
assert_eq!(
map[&cond].get_state(),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -828,7 +832,11 @@ mod tests {
assert_eq!(
map[&cond].get_state(),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -861,7 +869,11 @@ mod tests {
assert_eq!(
map[&cond].get_state(),
(
None,
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -892,7 +904,11 @@ mod tests {
assert_eq!(
map[&cond].get_state(),
(
None,
Some(&TrapState {
action: Action::Default, // TODO Should be Action::Ignore,
origin: Origin::Subshell,
pending: false
}),
Some(&TrapState {
action,
origin: Origin::User(origin),
@@ -963,7 +979,17 @@ mod tests {
.unwrap();

state.clear_parent_state();
assert_eq!(state.get_state(), (None, None));
assert_eq!(
state.get_state(),
(
Some(&TrapState {
action: Action::Default,
origin: Origin::Subshell,
pending: false
}),
None
)
);
}

#[test]