Update native encoding detection for JEP400 #1393
Conversation
|
@dbwiddis could you please have a look at this? |
| // native.encoding, prior versions don't have that property and will | ||
| // report NULL for it. | ||
| // The algorithm is simple: If native.encoding is set, it will be used | ||
| // else the original implementation of Charset#defaultCharset is used | ||
| String nativeEncoding = System.getProperty("native.encoding"); | ||
| Charset nativeCharset = null; | ||
| if (nativeEncoding != null) { | ||
| try { | ||
| nativeCharset = Charset.forName(nativeEncoding); | ||
| } catch (Exception ex) { | ||
| LOG.log(Level.WARNING, "Failed to get charset for native.encoding value : '" + nativeEncoding + "'", ex); | ||
| } | ||
| } |
There was a problem hiding this comment.
Won't this introduce a warning on initialization of Native class for all JDKs older than JDK18? Is that intended? Would this be a custom property like the boot.library.path where JNA should introduce a custom property? Otherwise, there would be no way to remove runtime warnings, right?
There was a problem hiding this comment.
On JDKs prior to 18 native.encoding will not be set. In that case nativeEncoding will be null and the if block will not be entered (see the check in line 131). From my POV the only way to get a warning, is when native.encoding is set to an invalid value. This can only happen
- if there is a bug in the JDK
- you tried to override the property in an invalid way
both cases are valid warning reasons.
| @@ -768,6 +768,8 @@ osname=macosx;processor=aarch64 | |||
| <property name="file.reference.jna.build" location="${build}"/> | |||
| <property name="file.reference.jna.jar" location="${build}/${jar}"/> | |||
| <property name="libs.junit.classpath" refid="test.libs"/> | |||
| <property name="javac.source" value="${compatibility}" /> | |||
There was a problem hiding this comment.
Can you help explain where the variable ${compatibility} introduced in the codebase?
There was a problem hiding this comment.
It is determined in the build script itself:
Lines 98 to 110 in 65019cc
There was a problem hiding this comment.
It is determined in the build script itself:
Thanks!
|
@matthiasblaesing what is the idea behind introducing the JDK source and target properties in each of the contrib build files (e.g. rather than simply providing this as a single property?) |
The properties |
…mine default charset JNA used the defaultCharset to determine which encoding to use when converting strings to native char*. The defaultCharset is set from the system property file.encoding. Up to JDK 17 its value defaulted to the system default encoding. From JDK 18 onwards its default value changed to UTF-8. JDK 18+ exposes the native encoding as the new system property native.encoding, prior versions don't have that property and will report NULL for it. The algorithm is simple: If native.encoding is set, it will be used else the original implementation of Charset#defaultCharset is used.
matthiasblaesing
force-pushed
the
native_encoding_jdk18
branch
from
65019cc
to
06ec1e4
Compare
Ok, it just seems odd to declare a constant used by all ant scripts redundantly versus in a dedicated property file. It's not a matter of reinvention, but centralization, ant doesn't care if the property comes from a shared file, command line, etc, it will always be immutable and the first instance (e.g. NetBeans via CLI) will override what's in the xml/properties. |
JEP 400 will make UTF-8 the default value for
file.encodingfor java:https://openjdk.java.net/jeps/400
For JNA this means on windows (on mac OS and Linux the encoding for native can be expected to be UTF-8 already) the native codepage would not be used anymore.
See the comments of the first commit for an explanation how it will be reworked.