8291978: jpackage: allow to override primary l10n files on Windows

[akashche]

This change is a follow-up to this comment.

Override implementation is based on this comment.

As suggested in this comment only English translation is provided for new resource.wxl-file label.

getTempdirectory utility was moved from BasicTest to WindowsHelper to be able to provide --temp and inspect the contents of config dir in verifier. It is not clear if WindowsHelper is an appropriate place for this utility (BasicTest is not Windows-specific), I've assumed that --temp flag itself is only used on Windows.

Testing:

• enhanced WinL10nTest adding checks for -loc arguments to light.exe and additional @Parameter run that overrides one of the primary .wxl files
• ran jtreg:jdk/tools/jpackage/windows and a BasicTest in unpack mode

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8291978: jpackage: allow to override primary l10n files on Windows

Reviewers

• Alexey Semenyuk (@alexeysemenyukoracle - Reviewer)
• Alexander Matveev (@sashamatveev - Reviewer) ⚠️ Review applies to d4ba8bf0
• Naoto Sato (@naotoj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9780/head:pull/9780
$ git checkout pull/9780

Update a local copy of the PR:
$ git checkout pull/9780
$ git pull https://git.openjdk.org/jdk pull/9780/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9780

View PR using the GUI difftool:
$ git pr show -t 9780

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9780.diff

[bridgekeeper]

👋 Welcome back akasko! 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]

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

• core-libs

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]

Webrevs

• 02: Full - Incremental (a6e6ee1c)
• 01: Full - Incremental (097ae46e)
• 00: Full (d4ba8bf0)

[openjdk]

@akashche 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:

8291978: jpackage: allow to override primary l10n files on Windows

Reviewed-by: asemenyuk, almatvee, naoto

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 49 new commits pushed to the master branch:

• 68af7c1: 8291734: Return accurate ACC_SUPER access flag for classes
• e4925a3: 8291924: jpackage: l10n for Windows context menu label
• 7b029ea: 8227651: Tests fail with SSLProtocolException: Input record too big
• 35fd5d8: 8292200: Add java/io/File/GetXSpace.java to Windows problem list
• 9825c33: 8291641: Optimize StackTraceElement.toString()
• cb37282: 8291752: AArch64: Remove check_emit_size parameter from trampoline_call
• 37d3146: 8289002: Minimal x86_64 VM build fails with GCC 11: 'this' pointer is null
• ecfa38f: 8281966: Absolute path of symlink is null in JFileChooser
• 83dc2e3: 8292062: misc javax/swing tests failing
• 4913380: 8292007: Do not include vmSymbol.hpp in method.hpp
• ... and 39 more: https://git.openjdk.org/jdk/compare/88c96dd3eb28a273305d07fb2b0a36122484189c...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@alexeysemenyukoracle, @sashamatveev, @naotoj) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[akashche]

Thanks for the review!

[akashche]

/integrate

[openjdk]

@akashche
Your change (at version d4ba8bf) is now ready to be sponsored by a Committer.

[sashamatveev]

This needs to be added to all WinResources*.properties files.

[alexeysemenyukoracle]

Good point. We have a discussion of how l10n files should be changed at #9753. Can you confirm that if some property is missing in a resource bundle its value is picked from the English resource bundle?

[akashche]

I've just pushed the commit with EN entries for all files before reading the latest comment. I'll verify the behaviour with .wxl and .properties files and will comment in this issue.

[naotoj]

I believe for the .properties case, modifications for the localized files are not required, as ResourceBundles fall back to the root (English) bundle.

[alexeysemenyukoracle]

/sponsor

[openjdk]

@alexeysemenyukoracle The PR has been updated since the change author (@akashche) issued the integrate command - the author must perform this command again.

[akashche]

Sorry for delay, I've found multiple problems while checking l10n handling manually.

Overview:

1. I've verified, that if an entry is missed in WinResources_*.properties file, fallback value from WinResources.properties file is actually used. Removed EN entries from non-EN .properties files in the patch.

2. I've found a problem in primary .wxl file selection, the logic in code uses resource.wxl-file-name to determine the file, but it appeared that all WinResources_*.properties files had the same resource.wxl-file-name=MsiInstallerStrings_en.wxl entry. I assumed this was an oversight and changed these entries to corresponding localized ones in the patch.

3. I've verified, that if an entry is missed in .wxl file, there is no fallback and light.exe invocation will fail like this:

InstallDirNotEmptyDlg.wxs(44) : error LGHT0102 : The localization variable !(loc.message.install.dir.exist) is unknown.  Please ensure the variable is defined.

This is caused because when -J-Duser.language=de -J-Duser.country=DE are specified to jpackage.exe , it will result with the following cultures in light.exe invocation: -cultures:de-de.

4. Continuing with the problem in "3.", I've tried to add en-us culture as a fallback one, and found that it is not possible, light.exe fails with the following:

light.exe : error LGHT0100 : The localization identifier 'message.install.dir.exist' has been duplicated in multiple locations.  Please resolve the conflict.

when -cultures:de-de;en-us is specified to it.

5. Culture names in all primary .wxl files except en-us are specified using only "language" part, like de. I've found that this usage works correctly, when only WixUtilExtension is used (default jpackage behaviour), but it is not accepted by light.exe when WixUIExtension is also used, it fails like this (not Wix-internal build-time paths):

    C:\agent\_work\66\s\src\ext\UIExtension\wixlib\BrowseDlg.wxs(10) : error LGHT0102 : The localization variable !(loc.WixUIOK) is unknown.  Please ensure the variable is defined.
    C:\agent\_work\66\s\src\ext\UIExtension\wixlib\BrowseDlg.wxs(14) : error LGHT0102 : The localization variable !(loc.WixUICancel) is unknown.  Please ensure the variable is defined.
    C:\agent\_work\66\s\src\ext\UIExtension\wixlib\BrowseDlg.wxs(18) : error LGHT0102 : The localization variable !(loc.BrowseDlgComboLabel) is unknown.  Please ensure the variable is defined.

It is not clear, why exactly this is happening, I've found it is possible to fix this by using Culture names in primary .wxl files in exactly the same form as in Wix internal .wxl files, like in WixUI_de-de.wxl. I've updated cultures names in all primary .wxl files in the patch.

6. To cover "5." I've updated WinL10nTest adding runs with and without WixUIExtension for all primary cultures.

7. I assume this problem is not directly related to jpackage. I've found that jpackage.exe --verbose output for Japanese and Chinese locale runs is garbled when it is redirected to file:

[03:54:17.120] ?????? MSI ????? ???

I've found that if encoding is specified to std streams in -jpackage laucher here](

jdk/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java

Line 50 in 37d3146

PrintWriter out = new PrintWriter(System.out);

):

If these streams are changed to:

        PrintWriter out = new PrintWriter(System.out, false, StandardCharsets.UTF_8);
        PrintWriter err = new PrintWriter(System.err, false, StandardCharsets.UTF_8);

then redirected output in the file is correct. I am not sure whether this change makes sense (I understand that this is a console output, and console encoding handling will depend on Windows system locale, I used only en-US system locale), so I didn't include it in the patch. Maybe this is something to keep in mind for future with overall Windows move to better UTF-8 support.

[alexeysemenyukoracle]

Thank you for the deep investigation and detailed report!

#2 issue was an oversight in WinResources_*.properties files. Thank you for catching it!

#7 I'm 100% sure not specifying character encoding in PrintWriter ctor was an oversight. The solution with specifying utf8 encoding looks good to me. Please file a separate CR for this issue.

[akashche]

Thanks for the review! I assume this PR still needs a l10n review from @naotoj before integrating it?

[alexeysemenyukoracle]

Agree. It would be good if @naotoj will look at it on more time.

[naotoj]

LGTM. Sorry for the delay.

[akashche]

/integrate

[openjdk]

@akashche
Your change (at version a6e6ee1) is now ready to be sponsored by a Committer.

[alexeysemenyukoracle]

/sponsor

[openjdk]

Going to push as commit 543163a.
Since your change was applied there have been 49 commits pushed to the master branch:

• 68af7c1: 8291734: Return accurate ACC_SUPER access flag for classes
• e4925a3: 8291924: jpackage: l10n for Windows context menu label
• 7b029ea: 8227651: Tests fail with SSLProtocolException: Input record too big
• 35fd5d8: 8292200: Add java/io/File/GetXSpace.java to Windows problem list
• 9825c33: 8291641: Optimize StackTraceElement.toString()
• cb37282: 8291752: AArch64: Remove check_emit_size parameter from trampoline_call
• 37d3146: 8289002: Minimal x86_64 VM build fails with GCC 11: 'this' pointer is null
• ecfa38f: 8281966: Absolute path of symlink is null in JFileChooser
• 83dc2e3: 8292062: misc javax/swing tests failing
• 4913380: 8292007: Do not include vmSymbol.hpp in method.hpp
• ... and 39 more: https://git.openjdk.org/jdk/compare/88c96dd3eb28a273305d07fb2b0a36122484189c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@alexeysemenyukoracle @akashche Pushed as commit 543163a.

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

[akashche]

About the point "7." above, I've experimented (on Windows Server 2016) with cmd.exe and codepages and found that existing behaviour of jpackage.exe is consistent with existing behaviour of java.exe.

For both java.exe and jpackage.exe, when "Active codepage" in cmd.exe is 437 (default en-US):

C:\Users\Administrator>chcp
Active code page: 437

Then if I set default JVM locale to ja-JP using -Duser.language=ja and -Duser.country=JP, then both output in console window and redirected file output will be garbled to question marks ??????.

If I set the Windows system "Language for non-Unicode programs" to Japanese, then, after the reboot, active codepage in cmd.exe will be 932 (note also Japanese file separator):

C:¥projects¥openjdk¥scripts>chcp
Active code page: 932

In this case running both java.exe and jpackage.exe with ja-JP JVM locale will cause Japanese output to be displayed correctly in the cmd.exe. And output redirected to file will also be written correctly in Shift-JIS encoding.

So I assume that existing behaviour is intended one and not filing the Jira issue now. Please let me know if it is still makes sense to file such Jira issue for UTF-8 stdout (for example, for the future).

[alexeysemenyukoracle]

I don't think it is good that the encoding of jpackage output depends on the default jvm encoding. It is convenient to know that the output is always in a specific encoding regardless of the values of user.language and user.country system properties. But this might not be the case of help and error output produced by jpackage.

Let's keep jpackage aligned with other JDK tools. Could you please check the behavior of java? How the output of java --help is encoded?

[naotoj]

That's the expected behavior. Historically, we only support where user's locale and the system' locale match, otherwise such a garbled text would generate. Since Windows 10, they offer UTF-8 as the system encoding (still they claim it is beta), then all the out put is in UTF-8.

[alexeysemenyukoracle]

Thank you for the clarification, @naotoj ! I think the case is closed. @akashche, no need to bother with "7".