On Wed, 10 Aug 2022 13:12:57 GMT, Alex Kasko <aka...@openjdk.org> wrote:

>> This change is a follow-up to [this 
>> comment](https://bugs.openjdk.org/browse/JDK-8290519?focusedCommentId=14512038&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14512038).
>> 
>> Override implementation is based on [this 
>> comment](https://bugs.openjdk.org/browse/JDK-8290519?focusedCommentId=14510886&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14510886).
>> 
>> As suggested in [this 
>> comment](https://github.com/openjdk/jdk/pull/9753#pullrequestreview-1062560789)
>>  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:
>> 
>>  - [x] enhanced `WinL10nTest` adding checks for `-loc` arguments to 
>> `light.exe` and additional `@Parameter` run that overrides one of the 
>> primary `.wxl` files
>>  - [x] ran jtreg:jdk/tools/jpackage/windows and a `BasicTest` in unpack mode
>
> Alex Kasko has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixes for bundling with different cultures

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](https://github.com/openjdk/jdk/blob/ecfa38ffa8620e41854a043497f19863da297947/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WinMsiBundler.java#L567)
 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](https://github.com/jef-n/wix3/blob/6b68e7f3601dfc35c3ee2a96510113585985b960/src/ext/UIExtension/wixlib/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](https://github.com/openjdk/jdk/blob/37d3146cca2c40dd53fcebd9cb78595f018b3489/src/jdk.jpackage/share/classes/jdk/jpackage/main/Main.java#L50):

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.

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

PR: https://git.openjdk.org/jdk/pull/9780

Reply via email to