Skip to content
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

Adapt new behavior of System.console() since JDK22 #40087

Closed
Eng-Fouad opened this issue Apr 15, 2024 · 6 comments · Fixed by #40127
Closed

Adapt new behavior of System.console() since JDK22 #40087

Eng-Fouad opened this issue Apr 15, 2024 · 6 comments · Fixed by #40127
Labels
area/core kind/enhancement New feature or request
Milestone

Comments

@Eng-Fouad
Copy link
Contributor

Eng-Fouad commented Apr 15, 2024

Description

Since JDK22:

In JDK 22, System.console() has been changed to return a Console with enhanced editing features that improve the experience of programs that use the Console API. In addition, System.console() now returns a Console object when the standard streams are redirected or connected to a virtual terminal. Prior to JDK 22, System.console() instead returned null for these cases. This change may impact code that checks the return from System.console() to test if the JVM is connected to a terminal. If required, the -Djdk.console=java.base flag will restore the old behavior where the console is only returned when it is connected to a terminal. Starting JDK 22, one could also use the new Console.isTerminal() method to test if the console is connected to a terminal.

I looked into Quarkus codebase and I found the following snippets that might need to be changed:

// on sane operating systems having a console is a good indicator
// you are attached to a TTY with colors.
return System.console() != null;

if (console != null) {
console.writer().print(s);
console.writer().flush();
} else {
printStream.print(s);
}

Optional<Console> console = Optional.ofNullable(System.console());
String response = console
.map(c -> c.readLine(prompt + choices + optionalQuestionMark, args).trim().toLowerCase())
.orElse(defaultValue ? "y" : "n");
if (response.isBlank()) {
return defaultValue;
}

Implementation ideas

Change System.console() != null to System.console() != null && System.console().isTerminal().

Since isTerminal() is a new method introduced in JDK22, we can either:

  • Call it via reflection.
  • Use "multi-release jar" feature (JEP238).
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 16, 2024

/cc @Sanne (core), @aloubyansky (core), @gsmet (core), @radcortez (core), @stuartwdouglas (core)

@stuartwdouglas
Copy link
Member

I think using reflection would be the simplest approach here, ideally guarded by a version check so we don't make the reflection calls for older versions.

@geoand
Copy link
Contributor

geoand commented Apr 17, 2024

I think using reflection would be the simplest approach here, ideally guarded by a version check so we don't make the reflection calls for older versions.

+1 as I am pretty sure we are not setup for MR-Jars in the main Quarkus repo.

@geoand
Copy link
Contributor

geoand commented Apr 17, 2024

@Eng-Fouad would you like to contribute this?

@Eng-Fouad
Copy link
Contributor Author

@Eng-Fouad would you like to contribute this?

Submitted a PR. Do we need to register isTerminal for reflection in native image?

@stuartwdouglas
Copy link
Member

AFAIK graal should be able to detect this case automatically because it is basically hard coded.

gsmet added a commit that referenced this issue Apr 22, 2024
Adapt new behavior of System.console() since JDK22
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone Apr 22, 2024
@gsmet gsmet modified the milestones: 3.11 - main, 3.10.0, 3.9.5 Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants