Skip to content

fix(trap): report signals ignored on shell entry in trap -p - #1268

Open
lu-zero wants to merge 2 commits into
reubeno:mainfrom
lu-zero:fix/trap-p-ignored-signals
Open

lu-zero wants to merge 2 commits into
reubeno:mainfrom
lu-zero:fix/trap-p-ignored-signals

Conversation

@lu-zero

@lu-zero lu-zero commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Bash reports signals that were ignored (SIG_IGN disposition) when the shell started — e.g. trap -- '' SIGRTMIN — via trap -p. Brush currently only prints explicitly registered handlers, so environments that ignore a signal on entry (like the GitHub Actions runner ignoring SIGRTMIN) produce a trap -p diff against bash.

This PR makes brush capture the ignored-on-entry signals at shell creation and report them in trap -p, matching bash.

Changes

  • brush-core/src/sys/unix/signal.rs: new ignored_signals_on_entry() queries each signal's disposition via sigaction, including real-time signals (SIGRTMIN..SIGRTMAX) on Linux/Android.
    • SIGPIPE is excluded: the Rust runtime always sets it to SIG_IGN at startup for every Rust program, which does not reflect the parent's disposition, while bash (a C program) starts with SIG_DFL in comparable spawn scenarios.
  • brush-core/src/sys/stubs/signal.rs: stub returns an empty set.
  • brush-core/src/traps.rs: TrapHandlerConfig stores the ignored-on-entry signal numbers; new signal_name_for_number() names signals the way bash does, including real-time naming (SIGRTMIN, SIGRTMIN+1, SIGRTMAX-1, …).
  • brush-core/src/shell.rs: captures ignored-on-entry signals in Shell::new.
  • brush-builtins/src/trap.rs: trap -p and trap -p <sig> report ignored-on-entry signals as trap -- '' <SIG>.

Validation

  • Full compat suite run under a parent that ignores SIGRTMIN (replicating the CI environment): 2174 test cases, 0 failed (the previously-failing trap -p - with no args shows all traps case now passes).
  • cargo clippy and cargo fmt --check clean; brush-core unit tests pass.

Fixes the CI failure seen on the OS-target test jobs in e.g. #1267.

Bash reports signals that were ignored (SIG_IGN disposition) when the
shell started, e.g. `trap -- '' SIGRTMIN`, via `trap -p`. Brush now
captures the ignored-on-entry signals at shell creation and displays
them, matching bash output in environments where such signals are
ignored (e.g. SIGRTMIN in CI runners).

SIGPIPE is excluded from the captured set because the Rust runtime
always sets it to SIG_IGN at startup, which does not reflect the parent
process's disposition.

Assisted-by: opencode:deepseek-v4-flash-free
@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

Public API changes for crate: brush-core

Added items

+pub fn brush_core::sys::signal::ignored_signals_on_entry() -> alloc::vec::Vec<i32>
+pub fn brush_core::traps::TrapHandlerConfig::is_signal_ignored_on_entry(&self, i32) -> bool
+pub fn brush_core::traps::TrapHandlerConfig::iter_signals_ignored_on_entry(&self) -> impl core::iter::traits::iterator::Iterator<Item = i32> + '_
+pub fn brush_core::traps::TrapHandlerConfig::set_signals_ignored_on_entry(&mut self, alloc::vec::Vec<i32>)
+pub fn brush_core::traps::signal_name_for_number(i32) -> alloc::string::String

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 18.59 μs 18.53 μs -0.07 μs ⚪ Unchanged
eval_arithmetic 0.13 μs 0.13 μs 0.00 μs ⚪ Unchanged
expand_one_string 1.71 μs 1.65 μs -0.06 μs ⚪ Unchanged
for_loop 29.30 μs 28.92 μs -0.39 μs 🟢 -1.32%
full_peg_complex 54.78 μs 54.70 μs -0.08 μs ⚪ Unchanged
full_peg_for_loop 6.28 μs 6.37 μs 0.08 μs ⚪ Unchanged
full_peg_nested_expansions 15.60 μs 15.76 μs 0.16 μs ⚪ Unchanged
full_peg_pipeline 4.14 μs 4.24 μs 0.09 μs 🟠 +2.24%
full_peg_simple 1.69 μs 1.73 μs 0.04 μs 🟠 +2.49%
function_call 3.71 μs 3.63 μs -0.08 μs ⚪ Unchanged
instantiate_shell 54.38 μs 63.57 μs 9.19 μs 🟠 +16.90%
instantiate_shell_with_init_scripts 25124.78 μs 24593.48 μs -531.30 μs ⚪ Unchanged
parse_peg_bash_completion 2224.14 μs 2245.13 μs 20.99 μs 🟠 +0.94%
parse_peg_complex 19.08 μs 19.15 μs 0.07 μs ⚪ Unchanged
parse_peg_for_loop 1.89 μs 1.89 μs 0.00 μs ⚪ Unchanged
parse_peg_pipeline 2.06 μs 1.94 μs -0.12 μs 🟢 -5.83%
parse_peg_simple 1.05 μs 1.05 μs 0.00 μs ⚪ Unchanged
run_echo_builtin_command 13.16 μs 13.14 μs -0.02 μs ⚪ Unchanged
tokenize_sample_script 3.44 μs 3.50 μs 0.07 μs 🟠 +1.95%

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-builtins/src/trap.rs 🟢 98.51% 🟢 82.76% 🔴 -15.75%
brush-core/src/results.rs 🟢 83.33% 🟢 80.16% 🔴 -3.17%
brush-core/src/shell.rs 🟢 94.85% 🟢 94.92% 🟢 0.07%
brush-core/src/sys/unix/signal.rs 🟠 60% 🟠 71.43% 🟢 11.43%
brush-core/src/traps.rs 🟢 86.87% 🟢 91.1% 🟢 4.23%
Overall Coverage 🟢 76.18% 🟢 76.19% 🟢 0.01%

Minimum allowed coverage is 70%, this run produced 76.19%
Maximum allowed coverage difference is -5%, this run produced 0.01%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1582 75.01
❗️ Error 17 0.81
❌ Fail 156 7.40
⏩ Skip 339 16.07
❎ Expected Fail 13 0.62
✔️ Unexpected Pass 2 0.09
📊 Total 2109 100.00

The signal-number-to-name mapping for standard signals only holds where a
real `Signal` enum exists (unix); the stubbed platforms (e.g., Windows)
fall back to a numeric name. Also make the stub `real_time_signal_name`
a const fn to satisfy clippy's missing-const-for-fn lint.

Assisted-by: opencode:deepseek-v4-flash-free
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant