RFR: 8330998: System.console() writes to stderr when stdout is redirected [v2]

[Naoto Sato]

On Mon, 29 Apr 2024 14:56:23 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> Consider code like:
>> 
>> public class ConsoleTest {
>>     public static void main(String... args) {
>>         System.console().printf("Hello!");
>>     }
>> }
>> 
>> 
>> When run as:
>> 
>> $ java ConsoleTest.java >/dev/null
>> 
>> 
>> it prints `Hello!` to stderr, instead of to stdout (where it would be redirected).
>> 
>> The proposed fix is to simply force the use of stdout. Sadly, this cannot be done solely using JLine configuration, we actually need to change the JLine's code for that.
>> 
>> The most tricky part is a test. There are two sub-tests, one effectively testing a case where all of stdin/out/err are redirected, the other is attempting to test the case where stdin is attached to a terminal, while stdout is redirected. The second sub-test using a native functions to create a pty and to attach to it, and should run in a separate VM, as it leaves the VM attached to the terminal.
>
> Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8330998
>  - Fixing test.
>  - Attempting to stabilize the test.
>  - Improving test to really test the redirect while stdin is connected to a terminal.
>  - Fixing typo.
>  - 8330998: System.console() writes to stderr when stdout is redirected

Looks good to me. Left some minor suggestions.
BTW, should we file an issue at the `JLine` project, not to redirect to stderr, or suggest a new config (sorry if it has already been taken care of)?

test/jdk/jdk/internal/jline/RedirectedStdOut.java line 29:

> 27:  * @summary Verify that even if the stdout is redirected java.io.Console will
> 28:  *          use it for writing.
> 29:  * @run main RedirectedStdOut runRedirectAllTest

`@modules jdk.internal.le` is needed, as the test relies on it.

test/jdk/jdk/internal/jline/RedirectedStdOut.java line 59:

> 57:     void runRedirectAllTest() throws Exception {
> 58:         String testJDK = System.getProperty("test.jdk");
> 59:         Path javaLauncher = Path.of(testJDK, "bin", "java");

Could utilize `ProcessTools` test library to start the test jdk process (applies to the other location as well)

-------------

PR Review: https://git.openjdk.org/jdk/pull/18996#pullrequestreview-2029297831
PR Review Comment: https://git.openjdk.org/jdk/pull/18996#discussion_r1583564260
PR Review Comment: https://git.openjdk.org/jdk/pull/18996#discussion_r1583565422