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.

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

Tracking issue for std::io::set_panic and set_print #31343

Open
SimonSapin opened this Issue Feb 1, 2016 · 13 comments

Comments

Projects
None yet
@SimonSapin
Contributor

SimonSapin commented Feb 1, 2016

std::io contains these two public unstable functions:

/// Resets the thread-local stderr handle to the specified writer
///
/// This will replace the current thread's stderr handle, returning the old
/// handle. All future calls to `panic!` and friends will emit their output to
/// this specified handle.
///
/// Note that this does not need to be called for all new threads; the default
/// output handle is to the process's stderr stream.
pub fn set_panic(sink: Box<Write + Send>) -> Option<Box<Write + Send>> { … }

/// Resets the thread-local stdout handle to the specified writer
///
/// This will replace the current thread's stdout handle, returning the old
/// handle. All future calls to `print!` and friends will emit their output to
/// this specified handle.
///
/// Note that this does not need to be called for all new threads; the default
/// output handle is to the process's stdout stream.
pub fn set_print(sink: Box<Write + Send>) -> Option<Box<Write + Send>> { … }

They both have #[doc(hidden)]. Anyone knows why?

The also both have:

#[unstable(feature = "set_stdio",
           reason = "this function may disappear completely or be replaced \
                     with a more general mechanism",
           issue = "0")]

Their functionality is evidently useful, since the test crate uses them to capture the output of tests on only show it when they fail. External test harnesses would likely want to have similar functionality.

This issue is about eventually stabilizing either these functions or a more general mechanism.

For set_panic, that mechanism might be panic handlers (#30449) though it would be nice to be able to not duplicate most of the work done in the default handler just to change the output stream. This still leaves set_print.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Feb 1, 2016

This is legacy behavior on behalf of libtest. The set_panic function should be subsumed by panic::set_handler, and set_print is just to have a nice testing experience.

I don't think we should stabilize either of these (as evident by issue = "0"), and this just requires design work to figure out what we can do to replace this functionality.

@retep998

This comment has been minimized.

Member

retep998 commented Feb 1, 2016

@DanielKeep was specifically interested in set_print as he was working on a library that automatically interprets all escape codes written to stdout and replaces them with calls to the Windows Console API.

@DanielKeep

This comment has been minimized.

Contributor

DanielKeep commented Feb 2, 2016

Currently, the code for doing this (replacing stdout) is platform-specific and really brittle (see ansi_interpreter win32/intercept.rs). It basically has to do an end-run-around behind libstd's back, swapping out the OS level handles and hoping that no one has touched IO yet... because if they have, then libstd will have already cached the "true" handles, and none of this will work.

Because of libstd's caching behaviour, the only way I can see to do this properly is to be able to explicitly replace the output writer.

Also, consider that you often want to redirect stdout/stderr when working with GUI applications, either to an internal buffer or a log file. Remember that on Windows, GUI applications don't have the standard IO handles connected to anything.

One point to note, however, is that the interception I'm doing is process-wide, whereas I believe set_print is per-thread. Both are useful, though; something like this would be handy:

fn set_process_stdout<W: Write + IntoRawFd>(write: W) -> io::Result<()> { ... }
fn set_thread_stdout<W: Write>(write: W) -> io::Result<()> { ... }

@steveklabnik steveklabnik added the A-libs label Feb 2, 2016

@nagisa

This comment has been minimized.

Contributor

nagisa commented Feb 2, 2016

The two functions in the original report were added as a short term hack and are not intended for stabilisation consideration. If the functionality is desired, an RFC with a proper proposal should be created IMO.

@SimonSapin

This comment has been minimized.

Contributor

SimonSapin commented Feb 3, 2016

If this is a short term hack, what will the test crate use long term to capture test output?

@nagisa

This comment has been minimized.

Contributor

nagisa commented Feb 3, 2016

What will the test crate use long term to capture test output?

What test crate is using is an implementation detail of (currently) internal component. I suppose test crate would start using the newly introduced stable functions once those become available.

I do not argue against including this sort of functionality into libstd, but to me it sounds like there should be more design process for this and the process in question happens with RFCs (e.g. one obvious solution is to have something based on dup2 on unixes).

@tomaka

This comment has been minimized.

Contributor

tomaka commented Apr 18, 2016

For the record, the stdout and stderr are ignored when running an Android app.
android-rs-glue is using this feature to redirect stdout/stderr to Android's logging system.

EDIT: in fact no longer the case.

@BartMassey

This comment has been minimized.

BartMassey commented Jul 31, 2016

For the record, see issue #35136 for an example where the test runner fouls up my test via set_print redirection.

I would prefer to see set_print and set_panic replaced by the test runner running its crates in a child process it controls, ala http://gitlab.com/BartMassey/ptyknot, but I'm not sure how that would work on Windows.

@SecondNature

This comment has been minimized.

SecondNature commented Jul 18, 2018

set_print() doesn't capture io through std::io::write(). Consider adding set_write() or perhaps making internal set_stdio() , if that would achieve the same result, public would provide io::write() capture.

@SimonSapin

This comment has been minimized.

Contributor

SimonSapin commented Jul 18, 2018

@SecondNature There is no write() function in std::io. Are you referring to something else? (Perhaps provide a code example?)

@SecondNature

This comment has been minimized.

SecondNature commented Jul 18, 2018

Yes std::io::write() doesn't exist. My mistake. std::writeln!() redirection is required. Can capture println!() with set_print() but can't capture std::writeln!() as no set_write().

@SimonSapin

This comment has been minimized.

Contributor

SimonSapin commented Jul 18, 2018

The writeln! macro takes a "destination" as a first parameter. If that destination is std::io::stdout(), then the redirection applies today. (Because it is part of stdout(), unrelated to writeln!.) When using writeln! with a different destination, such as an open file, I think the redirection should not apply. Why would it?

@KodrAus

This comment has been minimized.

KodrAus commented Sep 24, 2018

We ran into a quirk of set_print in sebasmagri/env_logger#107. There's some surprising behaviour when using the stdout and stderr functions to write to the stream directly during tests, because they aren't using the potentially shimmed LOCAL_STDOUT and LOCAL_STDERR and won't be captured. I think we'd definitely need to explore what the role of the set_print function would be, beyond being a hacked implementation detail so that tests can capture output in a more robust way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment