8261920: [AIX] jshell command throws java.io.IOError on non English locales #2622

takiguc · 2021-02-18T03:56:25Z

jshell uses JLine library.
JLine checks word "columns" in "stty -a" command output to find out terminal size.
The word "columns" was translated on AIX's Japanese locale (other locales also), so above error was happened.

OpenJDK for AIX developer could not find this issue because he/she requires AIX Toolbox's coreutils rpm package as build tools.
Another stty command is in /opt/freeware/bin/stty, it's in coreutils rpm package.

Standard users' system may not have coreutils rpm package or they may not set /opt/freeware/bin in PATH environment variable.
stty command should be executed with C locale to get English message.
Additionally, JLine library requires -F option. AIX's stty command does not support -F option,
but stty command which is in coreutils rpm package supports -F option.
On standard jshell usage, -F option may not be used. But when the system has coreutils rpm package, jshell should use this one.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8261920: [AIX] jshell command throws java.io.IOError on non English locales

Reviewers

Thomas Stuefe (@tstuefe - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2622/head:pull/2622
$ git checkout pull/2622

bridgekeeper · 2021-02-18T03:56:43Z

👋 Welcome back itakiguchi! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-02-18T03:58:18Z

@takiguc The following label will be automatically applied to this pull request:

kulla

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-02-18T04:05:52Z

Webrevs

00: Full (127052c)

takiguc · 2021-02-21T13:13:26Z

/label ppc-aix-port-dev

openjdk · 2021-02-21T13:13:42Z

@takiguc The label ppc-aix-port is not a valid label. These labels are valid:

serviceability
hotspot
sound
hotspot-compiler
kulla
i18n
shenandoah
jdk
javadoc
2d
security
swing
hotspot-runtime
jmx
build
nio
beans
core-libs
compiler
net
hotspot-gc
hotspot-jfr
awt

tstuefe · 2021-02-22T10:52:28Z

This seems reasonable. Which tests did you run?

takiguc · 2021-02-22T11:06:10Z

No testcase is available for this issue, because it's related Locale setting and translated message on AIX platform.
I just execute jshell command on X11's dtterm and aixterm with Ja_JP locale.
(\uXXXX is actually, Japanese character)

$ ./jshell
| JShell\u3078\u3088\u3046\u3053\u305d -- \u30d0\u30fc\u30b8\u30e7\u30f315.0.2
| \u6982\u8981\u306b\u3064\u3044\u3066\u306f\u3001\u6b21\u3092\u5165\u529b\u3057\u3066\u304f\u3060\u3055\u3044: /help intro
Exception in thread "main" java.io.IOError: java.io.IOException: Unable to parse columns
        at jdk.internal.le/jdk.internal.org.jline.terminal.impl.AbstractPosixTerminal.getSize(AbstractPosixTerminal.java:62)
        at jdk.internal.le/jdk.internal.org.jline.terminal.Terminal.getBufferSize(Terminal.java:216)
        at jdk.internal.le/jdk.internal.org.jline.reader.impl.LineReaderImpl.doDisplay(LineReaderImpl.java:741)
        at jdk.internal.le/jdk.internal.org.jline.reader.impl.LineReaderImpl.<init>(LineReaderImpl.java:298)
        at jdk.jshell/jdk.internal.jshell.tool.ConsoleIOContext$2.<init>(ConsoleIOContext.java:133)
        at jdk.jshell/jdk.internal.jshell.tool.ConsoleIOContext.<init>(ConsoleIOContext.java:133)
        at jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:978)
        at jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:254)
        at jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
Caused by: java.io.IOException: Unable to parse columns
        at jdk.internal.le/jdk.internal.org.jline.terminal.impl.ExecPty.doGetInt(ExecPty.java:278)
        at jdk.internal.le/jdk.internal.org.jline.terminal.impl.ExecPty.doGetSize(ExecPty.java:263)
        at jdk.internal.le/jdk.internal.org.jline.terminal.impl.ExecPty.getSize(ExecPty.java:170)
        at jdk.internal.le/jdk.internal.org.jline.terminal.impl.AbstractPosixTerminal.getSize(AbstractPosixTerminal.java:60)
        ... 8 more
$

tstuefe · 2021-02-24T06:18:50Z

Im sorry, what I meant is how you make sure no regressions are happening. From what I can see you modify a general purpose function which executes Unix commands to always run with C Locale on AIX. Which unix commands are effected by this, and which JDK functionality? From looking at the source I find it difficult to see whether this does not introduce regressions, sorry. Did you run jtreg tests; if yes which tiers?

takiguc · 2021-02-24T12:51:57Z

I'm sorry I'm confused.
I rerun tier1 and tier2 on AIX platform.
No regression was there.

tstuefe

Thanks! I'm fine with this change then.

openjdk · 2021-02-24T13:06:39Z

@takiguc This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8261920: [AIX] jshell command throws java.io.IOError on non English locales

Reviewed-by: stuefe

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1043 new commits pushed to the master branch:

8c07063: 8262259: Remove unused variable in MethodLiveness::BasicBlock::compute_gen_kill_single
0d2dbd2: 8262027: Improve how HttpConnection detects a closed channel when taking/returning a connection to the pool
382e38d: 8256438: AArch64: Implement match rules with ROR shift register value
fac37bf: 8262269: javadoc test TestGeneratedClasses.java fails on Windows
3e13b66: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails
c769388: 8262266: JDK-8262049 fails validate-source
03e781b: 8262265: ProblemList jdk/javadoc/doclet/testGeneratedClasses/TestGeneratedClasses.java on Windows
c6eae06: 8262049: [TESTBUG] Fix TestReferenceRefersTo.java for Shenandoah IU mode
e5304b3: 8253409: Double-rounding possibility in float fma
3132b1c: 8261665: Clean up naming of StringContent and FixedStringContent
... and 1033 more: https://git.openjdk.java.net/jdk/compare/ecd7e476f838d6585d352488905a671bf2e41df3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

takiguc · 2021-02-24T13:13:13Z

@tstuefe - Thanks.

/integrate

openjdk · 2021-02-24T13:14:21Z

@takiguc Since your change was applied there have been 1043 commits pushed to the master branch:

8c07063: 8262259: Remove unused variable in MethodLiveness::BasicBlock::compute_gen_kill_single
0d2dbd2: 8262027: Improve how HttpConnection detects a closed channel when taking/returning a connection to the pool
382e38d: 8256438: AArch64: Implement match rules with ROR shift register value
fac37bf: 8262269: javadoc test TestGeneratedClasses.java fails on Windows
3e13b66: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails
c769388: 8262266: JDK-8262049 fails validate-source
03e781b: 8262265: ProblemList jdk/javadoc/doclet/testGeneratedClasses/TestGeneratedClasses.java on Windows
c6eae06: 8262049: [TESTBUG] Fix TestReferenceRefersTo.java for Shenandoah IU mode
e5304b3: 8253409: Double-rounding possibility in float fma
3132b1c: 8261665: Clean up naming of StringContent and FixedStringContent
... and 1033 more: https://git.openjdk.java.net/jdk/compare/ecd7e476f838d6585d352488905a671bf2e41df3...master

Your commit was automatically rebased without conflicts.

Pushed as commit 2c99bad.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

AIX stty command should be executed on C locale to get English message

127052c

openjdk bot added the kulla kulla-dev@openjdk.org label Feb 18, 2021

takiguc changed the title ~~[AIX] jshell command throws java.io.IOError on non English locales~~ 8261920: [AIX] jshell command throws java.io.IOError on non English locales Feb 18, 2021

openjdk bot added the rfr Pull request is ready for review label Feb 18, 2021

tstuefe approved these changes Feb 24, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 24, 2021

openjdk bot closed this Feb 24, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8261920: [AIX] jshell command throws java.io.IOError on non English locales #2622

8261920: [AIX] jshell command throws java.io.IOError on non English locales #2622

takiguc commented Feb 18, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 18, 2021

openjdk bot commented Feb 18, 2021

mlbridge bot commented Feb 18, 2021

takiguc commented Feb 21, 2021

openjdk bot commented Feb 21, 2021

tstuefe commented Feb 22, 2021

takiguc commented Feb 22, 2021

tstuefe commented Feb 24, 2021

takiguc commented Feb 24, 2021

tstuefe left a comment

openjdk bot commented Feb 24, 2021 •

edited

Loading

takiguc commented Feb 24, 2021

openjdk bot commented Feb 24, 2021

8261920: [AIX] jshell command throws java.io.IOError on non English locales #2622

8261920: [AIX] jshell command throws java.io.IOError on non English locales #2622

Conversation

takiguc commented Feb 18, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Feb 18, 2021

openjdk bot commented Feb 18, 2021

mlbridge bot commented Feb 18, 2021

Webrevs

takiguc commented Feb 21, 2021

openjdk bot commented Feb 21, 2021

tstuefe commented Feb 22, 2021

takiguc commented Feb 22, 2021

tstuefe commented Feb 24, 2021

takiguc commented Feb 24, 2021

tstuefe left a comment

Choose a reason for hiding this comment

openjdk bot commented Feb 24, 2021 • edited Loading

takiguc commented Feb 24, 2021

openjdk bot commented Feb 24, 2021

takiguc commented Feb 18, 2021 •

edited by openjdk bot

Loading

openjdk bot commented Feb 24, 2021 •

edited

Loading