Skip to content

Conversation

@mattsu2020
Copy link
Contributor

@mattsu2020 mattsu2020 commented Nov 8, 2025

Summary

Add a constructor hook that records whether SIGPIPE was already set to SIG_IGN before the std runtime mutates it, so seq can detect when the shell explicitly ignores the signal.
Have sigpipe_is_ignored() simply read the captured flag, letting the utility treat EPIPE as a write error only when SIGPIPE is being ignored externally (matching the GNU seq expectation and the seq-epipe.sh test).
Document the safety of the capture_sigpipe_state initializer so Clippy is satisfied.

Testing

cargo build -p uu_seq --bin seq
trap "" PIPE && { ./target/debug/seq inf 2>err; echo $? >code; } | head -n1

related
#9127

Add dependency on 'nix' crate and implement sigpipe_is_ignored() function to detect if SIGPIPE is ignored. Modify error handling in uumain to only ignore BrokenPipe errors when SIGPIPE is not ignored, ensuring correct behavior when the signal is handled externally. This prevents improper exits on broken pipes in environments where SIGPIPE is masked.
Add the Unix signal handling system call "sigaction" to the VSCode spell check dictionary to avoid false positives in technical documentation and code comments.
- Bump cc from 1.2.44 to 1.2.45
- Bump crc-fast from 1.6.0 to 1.7.0, removing rand and regex deps
- Bump jiff and jiff-static from 0.2.15 to 0.2.16, updating serde to serde_core
- Bump quote from 1.0.41 to 1.0.42
- Bump syn from 2.0.108 to 2.0.109
- Remove aho-corasick, regex, and related packages
- Update windows-sys to 0.60.2 and add nix dependency

These changes refresh dependencies for fuzzing components, potentially improving security and performance.
Previously, sigpipe_is_ignored() queried the signal handler on each call, which could be inefficient. Now, capture the state once at program initialization using platform-specific init arrays and store it in a static AtomicBool for fast access.
- Documents the safety invariants for the unsafe extern "C" function
- Ensures clarity on why the function is safe to call during process initialization
- Improves code maintainability and adheres to Rust documentation standards
Simplify code formatting by placing the 'ignored' variable assignment on a single line, improving readability without altering functionality.
@github-actions
Copy link

github-actions bot commented Nov 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/seq/seq-epipe is no longer failing!

@sylvestre
Copy link
Contributor

i guess it is pretty but would it be possible to add a test? thanks

Add a new test to verify that on Unix systems, when SIGPIPE is ignored, seq properly reports a write error ("Broken pipe") and exits with code 1 when its output is piped to head. This ensures robust behavior in piping scenarios.
@github-actions
Copy link

github-actions bot commented Nov 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/seq/seq-epipe is no longer failing!

@sylvestre
Copy link
Contributor

some lints are failing

- Split long script string in test_sigpipe_ignored_reports_write_error using concat! for improved readability
- Added #[cfg(unix)] attributes to TestScenario and util_name imports to restrict to Unix platforms, where SIGPIPE handling is applicable
…s_write_error

Remove the `concat!` macro from the script variable, as the string is short enough to be defined directly without concatenation. This improves code readability and reduces unnecessary macro usage.
- Combined multi-line string assignment and method chaining into single lines for conciseness and improved readability in test_sigpipe_ignored_reports_write_error.
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 8, 2025

CodSpeed Performance Report

Merging #9184 will not alter performance

Comparing mattsu2020:seq_compatibility (02875d0) with main (5be7f83)

Summary

✅ 126 untouched
⏩ 6 skipped1

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link

github-actions bot commented Nov 8, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/seq/seq-epipe is no longer failing!

let mut current = MaybeUninit::<libc::sigaction>::uninit();
if unsafe { libc::sigaction(libc::SIGPIPE, ptr::null(), current.as_mut_ptr()) } == 0 {
let ignored = unsafe { current.assume_init() }.sa_sigaction == libc::SIG_IGN;
SIGPIPE_WAS_IGNORED.store(ignored, Ordering::Relaxed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need to be in the unsafe block ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@Ecordonnier Ecordonnier Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be generalized and moved to src/uucore/src/lib/lib.rs ? In principle this is needed for all utilities. If it can be generalized, it could be used for #8919

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting this PR is the better approach.
Implementing it as-is in this PR will make things complicated.

@mattsu2020 mattsu2020 requested a review from sylvestre November 13, 2025 10:34
@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

…pe_ignored variable

Removed unused `ErrorKind` from std::io import to clean up imports. Prefixed `sigpipe_ignored` with underscore to suppress unused variable warnings, indicating it's not utilized in the current logic.
Adjust error handling for BrokenPipe in seq to match GNU seq behavior:
- Exit with status 0 if SIGPIPE was not ignored (prints error message).
- Fail with error if SIGPIPE was explicitly ignored.
Balance error checking logic to consistently return errors for write failures, ensuring proper propagation without conditional SIGPIPE handling.
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/seq/seq-epipe is no longer failing!

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.

3 participants