Property sun.stdout.encoding is undefined in recent Java versions#407
Property sun.stdout.encoding is undefined in recent Java versions#407wfouche wants to merge 12 commits intojython:masterfrom
Conversation
| String output = Py.getCommandResultWindows("chcp"); | ||
| String output = Py.getCommandResultWindows("chcp.com"); | ||
| /* | ||
| * The output will be like "Active code page: 850" or maybe "Aktive Codepage: 1252." or |
There was a problem hiding this comment.
I'm explaining why the only reliable bit of the message is the number. ISTR this one is German.
Ja. So stimmt es. https://www.mikrocontroller.net/topic/561065
There was a problem hiding this comment.
Reverted back to Aktive.
|
As I commented on the original issue, testing console handling is hard because our CI doesn't really exercise it. So it's down to conscientious local testing. |
| encoding = props.getProperty("sun.stdout.encoding"); | ||
| if (encoding != null) { | ||
| // Windows: these versions of Java return "cp65001" for UTF-8 | ||
| if (encoding.equals("cp65001")) { |
There was a problem hiding this comment.
Shouldn't we only bother with this inside the "if it's Windows" clause?
| if (os != null && os.startsWith("Windows")) { | ||
| // Go via the Windows code page built-in command "chcp". | ||
| String output = Py.getCommandResultWindows("chcp"); | ||
| String output = Py.getCommandResultWindows("chcp.com"); |
There was a problem hiding this comment.
When running chcp from Git-Bash on Windows, it cannot find "chcp" and one has to specify the full name which is "chcp.com". I thought Jython might experience the same issue, but it does not. So will revert this change.
There was a problem hiding this comment.
C:\> where chcp
C:\Windows\System32\chcp.com
|
@jeff5 , the following code fragment is unlikely to be executed on Windows given that stdout.encoding and sun.stdout.encoding seems to cover all bases. if (isWindows) {
// Go via the Windows code page built-in command "chcp".
String output = Py.getCommandResultWindows("chcp");
/*
* The output will be like "Active code page: 850" or maybe "Aktive Codepage: 1252." or
* "활성 코드 페이지: 949". Assume the first number with 2 or more digits is the code page.
*/
final Pattern DIGITS_PATTERN = Pattern.compile("[1-9]\\d+");
Matcher matcher = DIGITS_PATTERN.matcher(output);
if (matcher.find()) {
encoding = "cp".concat(output.substring(matcher.start(), matcher.end()));
if (encoding.equals("cp65001")) {
encoding = "utf-8";
}
return encoding;
}
} |
|
@jeff5 , I installed SDKMAN using Git-Bash on Windows, and this makes it a lot easier to do Jython console testing using different versions of Java. With SDKMAN I can easily install and switch between diffrent JDK versions. First install scoop - https://scoop.sh/ Then run these commands to install Git/Bash, curl, zip and unzip
Lastly, install SDKMAN
Now one can start bash.exe from inside the Jython project folder cd IdeaProjects\jython Bash is just a different Windows shell, so when Jython runs it still runs as it normally would as a Windows process. Don't you think this would make a good addition to the README.md file? |
|
Results of tests performanced on Linux, MacOS and Windows. We can see that for Java 21 and higher, property stdout.encoding is always defined. Older versions of Java don't define property stdout.encoding, and may or may not defined property sun.stdout.encoding. Linux
MacOS
Windows
test.cmd test.sh test.kt val javaVersion = System.getProperty("java.version") ?: "null"
val sun_stdout_encoding = System.getProperty("sun.stdout.encoding") ?: "null"
val stdout_encoding = System.getProperty("stdout.encoding") ?: "null"
fun main() {
print("java.version = " + javaVersion)
print(", sun.stdout.encoding = " + sun_stdout_encoding)
println(", stdout.encoding = " + stdout_encoding)
} |
|
@jeff5 , testing has been completed. Please review once more. Could we eventually release this as Jython 2.7.5? |
|
@Stewori . please review and if possible merge this PR next. How these two properties below are managed in newer versions of Java have changed:
The old logic failed with Java 25, but with this PR it now it works again. This code has been extensively tested using different major versions of Java. |
| // From Java 8 onwards, the answer may already be to hand in the registry: | ||
| String encoding = props.getProperty("sun.stdout.encoding"); | ||
| String os = props.getProperty("os.name"); | ||
| boolean isWindows = os != null && os.startsWith("Windows"); |
There was a problem hiding this comment.
Please move these two lines (String os = .... and boolean is Windows....) a bit downwards, so they are created only if stdout.encoding is null.
| // Java 19+ | ||
| String encoding = props.getProperty("stdout.encoding"); | ||
| if (encoding != null) { | ||
| // Windows: cp65001 is automatically mapped to UTF-8, no additional processing is needed |
There was a problem hiding this comment.
I verified this claim by googling, but struggle to find an authoritative source. I do not doubt this fact. However, if you have a link to an authoritative source for this claim, please add it as second line to this comment. E.g. something in the msdn docs or so.
This is not critical, just nice to have for reference.
There was a problem hiding this comment.
(I mean that cp65001 is or used to be Window's name for utf-8)
There was a problem hiding this comment.
The low-level C code in the JVM 19+ is only the documentation for this Windows specific mapping.
static char* getConsoleEncoding()
{
char* buf = malloc(16);
int cp;
if (buf == NULL) {
return NULL;
}
cp = GetConsoleCP();
if (cp >= 874 && cp <= 950)
sprintf(buf, "ms%d", cp);
else if (cp == 65001)
sprintf(buf, "UTF-8");
else
sprintf(buf, "cp%d", cp);
return buf;
}
|
Re-reading #404 it seems that cp65001 might be more subtle than just aliasing it to utf-8. I'll have to do more background research to be able to make an educated decision on this. In any case, please add a line to NEWS in bugs fixed 2.7.5, similar to
|
|
Okay, after a bit research and AI-talk, what about using |
|
Checking more carefully, |
|
I think the main drawback of using cp65001 is that some utf-8 characters can be misinterpreted or can't be displayed. By contrast, the behavior when using one of the common non-utf-8 code pages, having a wrong character in the output may cause an error. This is for Windows 10+. One could still get errors pre-windows 10 depending on the string, the Java version, etc. There's no perfect solution when the shell itself isn't fully utf-8 compatible, especially the cmd.exe shell pre-Windows 10, which is limited by its long history. |
|
Okay, this is about representing cp65001 as utf-8, not about permitting or encouraging its use in the first place. |
|
According to Mr. Chatbot Claude, for whatever it is worth:
If we say, as I think we agreed on, to say the next edition of Jython will only support Windows 10+, we ought to be all right. |
|
Thank you for cross-checking this. Looks like it aligns with my info, more specific though (at least AIs agree here^^). I'm not really sure how official the Windows 10+ agreement is, but I think using Windows <10 became so impracticable nowadays, that we don't need to consider it much. Even then - distinguish "support" from "bug-compatibility". IMHO to support something does not mean to compensate (every single of) its bugs. So I wouldn't even frame this as "support for Win < 10 dropped". An then, the problematic API is not even used by Jython directly. This should be safe enough. |
|
I remember years ago, probably with Python 2+ rather than Jython, I used to monkey patch the code that selected the right encoder/decoder pair so that it would choose cp65001 when the encoding was utf-8. That simplified life quite a bit, encoding-wise. I had forgotten all about that until today. I think that recent versions of CPython 2.7 have changed how they work in that respect. |
|
The changes look good to me, technically. |
5 participants
This PR was tested with a simple script that displays a Unicode character.
on Java 8 and 25.
Fixes #404