RFR: JDK-8306983: Do not invoke external programs when switch terminal to raw mode on selected platforms

Naoto Sato naoto at openjdk.org
Wed May 3 17:51:19 UTC 2023


On Thu, 27 Apr 2023 11:21:04 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> To support JShell and other usecases, the JDK uses JLine, which provides line-editing functionality inside a terminal.
> 
> JLine has several ways to work with the terminal, based on various additional libraries and tools. Most of them are not directly usable inside the JDK, so currently, on non-Windows platforms, the JLine inside the JDK will use external programs, like `stty`, to inspect the terminal's properties and to switch the terminal to raw mode.
> 
> This is slightly problematic, as the external programs might be missing, and while this is not that big problem for JShell, it might be a problem for other potential uses of JLine, like using it inside `System.console()`.
> 
> So, the proposal here is to call the corresponding native methods directly, on selected platforms for now (Linux and Mac OS/X), instead of invoking the external programs. On Windows, this has always been the case, we are using a custom implementation of the interface that maps native and Java function for JNA there, and the proposal is to do the same here. We take the appropriate mapping interface for JNA, and provide hand-written implementation for it, using JNI.
> 
> The Windows implementation is mostly unchanged, the changes are mostly non-Windows only. The overview of the changes is:
>  - `LastErrorException` is moved from the Windows-specific code to the platform-neutral code, as it is used by all the native implementations. This is the only change that directly affects the Windows-specific code
>  -  `unix/classes/jdk/internal/org/jline/terminal/impl/jna/JnaNativePty.java` and `unix/classes/jdk/internal/org/jline/terminal/impl/jna/JnaTerminalProvider.java` are created based on the corresponding files from JLine. In JLine, these files directly call platform-specific implementations, but in this patch, the platform specific code is commented out, and replaced with calls to `JDKNativePty`, which is a new platform-specific class, that delegates to the actual platform-specific implementations. Note the `JnaNativePty.java` and `JnaTerminalProvider.java` already exist in the Windows part, but have different pieces of code commented out, which makes them substantially different.
>  - for Linux and Mac OS/X, there are platform-specific implementations based on the corresponding code from JLine, with the hand-written JNI-based implementation of the JNA-based existing interfaces. They also have an implementation of `JDKNativePty`, which just delegates to the given platform's `"NativePty"` implementation.
>  - the `JdkConsoleProviderImpl.java` has been tweaked to not allow the implementation based on the external programs, and gracefully falling back to the standard implementation. JShell is kept unchanged.
> 
> The reasons for this organization are: limiting duplication, mostly adhering to the JDK's platform specific build (which is different from the normal JLine's platform-neutral build) and limiting the difference between standard JLine and JLine inside JDK, to help future upgrades.

Changes in `JdkConsoleProviderImpl.java` look good to me. Didn't look at other diffs closely, but it seems some other files are missing Oracle copyright text.

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

PR Review: https://git.openjdk.org/jdk/pull/13687#pullrequestreview-1411449102



More information about the build-dev mailing list