Skip to content

Conversation

@ChrisDryden
Copy link
Collaborator

From the feedback here, #10254 separating this into a separate PR. This is required to pass the GNU timeout tests.

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)

@Ecordonnier Ecordonnier self-assigned this Jan 24, 2026
@Ecordonnier
Copy link
Collaborator

Ecordonnier commented Jan 24, 2026

To be clear, it is about this behavior:

on linux, with the patch:
ecordonnier@LBM9V6C4:~/dev/coreutils$ trap '' PIPE
ecordonnier@LBM9V6C4:~/dev/coreutils$ ./target/debug/coreutils yes | head -n 2
y
y
yes: standard output: Broken pipe (os error 32)
ecordonnier@LBM9V6C4:~/dev/coreutils$ echo "${PIPESTATUS[@]}"
1 0


on linux, without the patch:
ecordonnier@LBM9V6C4:~/dev/coreutils$ trap '' PIPE
cordonnier@LBM9V6C4:~/dev/coreutils$ ./target/debug/coreutils yes | head -n 5
y
y
ecordonnier@LBM9V6C4:~/dev/coreutils$ echo "${PIPESTATUS[@]}"
0 0


GNU yes behavior:
ecordonnier@LBM9V6C4:~/dev/coreutils$ yes | head -n 2
y
y
yes: standard output: Broken pipe

Also the patch is keeping this line for windows, because on windows there is no default sigpipe handler which would terminate the process and return 141.

The requirements for handling BrokenPipe error on Windows are not clear to me. This code in tail.rs returns 13 on Windows, same thing in tr/src/simd.rs and cat.rs. Do you know where the requirement to return 0 on BrokenPipe in yes.rs come from? What's our reference implementation for this on windows?

See this code of tail.rs

    // SIGPIPE is not available on Windows.
    #[cfg(target_os = "windows")]
    writer.flush().inspect_err(|err| {
        if err.kind() == ErrorKind::BrokenPipe {
            std::process::exit(13);
        }
    })?;

The de-facto requirement we have for windows at the moment is that it must return 0 on BrokenPipe so that the tests which are calling run() -> run_stdout_starts_with() -> child.close_stdout() succeed on Windows (since the code added in 5c13e88 checks assert!(result.success() for non-unix platforms).

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