On Thu, 27 Apr 2023 11:21:04 GMT, Jan Lahoda <jlah...@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.

make/modules/jdk.internal.le/Lib.gmk line 38:

> 36:       LDFLAGS := $(LDFLAGS_JDKLIB), \
> 37:       LIBS_windows := $(JDKLIB_LIBS) user32.lib, \
> 38:       LIBS_unix := $(JDKLIB_LIBS) -lstdc++, \

If this is a C++ library, I think it's better to specify `TOOLCHAIN := 
TOOLCHAIN_LINK_CXX` than adding `-lstdc++`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13687#discussion_r1183721435

Reply via email to