Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v7]
On Mon, 11 Nov 2024 09:51:35 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains eight additional > commits since the last revision: > > - Merge branch 'openjdk:master' into unused > - Neater warning silencer in proc_md.h > - Revert _WIN32 workaround in log_messages.c > - Copyright in VersionInfo.cpp > - Remove neutralLangId in VersionInfo.cpp > - Remove global memHandle, would've liked to keep it as a comment :( > - Merge branch 'master' into unused > - 8342682 jpackage changes looks good. - Marked as reviewed by almatvee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21616#pullrequestreview-2430719530
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v5]
On Fri, 8 Nov 2024 13:39:08 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Remove neutralLangId in VersionInfo.cpp src/jdk.jpackage/windows/native/libjpackage/VersionInfo.cpp line 1: > 1: /* Copyright year needs to be updated. "2020," -> "2020, 2024,". - PR Review Comment: https://git.openjdk.org/jdk/pull/21616#discussion_r1835031208
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v5]
On Fri, 16 Dec 2022 03:38:54 GMT, Damon Nguyen wrote: >> Open l10n drop >> All tests passed > > Damon Nguyen has updated the pull request incrementally with one additional > commit since the last revision: > > Revert old translation. Fix lang codes jpackage changes looks good. - Marked as reviewed by almatvee (Reviewer). PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]
On Fri, 16 Dec 2022 00:24:26 GMT, Damon Nguyen wrote: >> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_ja.properties >> line 37: >> >>> 35: >>> resource.post-app-image-script=\u30A2\u30D7\u30EA\u30B1\u30FC\u30B7\u30E7\u30F3\u30FB\u30A4\u30E1\u30FC\u30B8\u3092\u79FB\u5165\u3057\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8 >>> 36: >>> resource.post-msi-script=exe\u30A4\u30F3\u30B9\u30C8\u30FC\u30E9\u306Emsi\u30D5\u30A1\u30A4\u30EB\u304C\u4F5C\u6210\u3055\u308C\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8 >>> 37: resource.wxl-file-name=MsiInstallerStrings_en.wxl >> >> Why this was changed? > > Also not sure as it is what I received from the latest translation drop. If > this is incorrect, I can manually correct this here and for the other locales > for this file. Yes it needs to be correct. It was changed to _ja, etc with [commit](https://github.com/openjdk/jdk20/commit/543163a03b5f1af7a7e7af317a26eb8c5aa81c38#diff-afb8ea68d8b3e9be4bba1c417c82d2162eea9333a3157f05ef57c026af94eb3b). Which was done on Aug 10, 2022, so it seems message drop is based on old code. - PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]
On Fri, 16 Dec 2022 00:15:51 GMT, Damon Nguyen wrote:
>> src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties
>> line 82:
>>
>>> 80:
>>> 81: error.foreign-app-image=Fehler: .jpackage.xml-Datei fehlt in
>>> app-image-Verzeichnis ({0})
>>> 82: error.invalid-app-image=Fehler: app-image-Verzeichnis ({0}) wurde von
>>> einer anderen jpackage-Version generiert, oder .jpackage.xml ist nicht
>>> wohlgeformt
>>
>> error.invalid-app-image looks like old translation and missing "{1}". Also,
>> why () is used instead of ""?
>
> I'm not sure for the decisions of change in punctuation and for the missing
> {1} as I didn't do the translation myself. Would this be better to be
> reverted?
Looks like translation is done on files before JDK-8293462 was integrated
(before Sep 26). See
[commit](https://github.com/openjdk/jdk20/commit/1e222bccd3807c1be0d1d824e0ff9745751d8375#diff-8bfeb61c827c2bc073c65cbf4137dbf8f7baa5f4f88e5d59785e985d9510577c)
when I changed these two strings. For now it is better to revert them to
original English version or figure out why translation is old.
-
PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]
On Thu, 15 Dec 2022 20:43:42 GMT, Damon Nguyen wrote:
>> Open l10n drop
>> All tests passed
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix https and changed URL back
src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties
line 82:
> 80:
> 81: error.foreign-app-image=Fehler: .jpackage.xml-Datei fehlt in
> app-image-Verzeichnis ({0})
> 82: error.invalid-app-image=Fehler: app-image-Verzeichnis ({0}) wurde von
> einer anderen jpackage-Version generiert, oder .jpackage.xml ist nicht
> wohlgeformt
error.invalid-app-image looks like old translation and missing "{1}". Also, why
() is used instead of ""?
src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties
line 82:
> 80:
> 81: error.foreign-app-image=\u30A8\u30E9\u30FC:
> app-image\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA({0})\u306B.jpackage.xml\u30D5\u30A1\u30A4\u30EB\u304C\u3042\u308A\u307E\u305B\u3093
> 82: error.invalid-app-image=\u30A8\u30E9\u30FC:
> app-image\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA({0})\u306F\u5225\u306Ejpackage\u30D0\u30FC\u30B8\u30E7\u30F3\u307E\u305F\u306F\u4E0D\u6B63\u306A.jpackage.xml\u3067\u751F\u6210\u3055\u308C\u307E\u3057\u305F
Same as "de" translation.
src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties
line 82:
> 80:
> 81: error.foreign-app-image=\u9519\u8BEF\uFF1Aapp-image \u76EE\u5F55 ({0})
> \u4E2D\u7F3A\u5C11 .jpackage.xml \u6587\u4EF6
> 82: error.invalid-app-image=\u9519\u8BEF\uFF1A\u53E6\u4E00\u4E2A jpackage
> \u7248\u672C\u6216\u683C\u5F0F\u9519\u8BEF\u7684 .jpackage.xml
> \u751F\u6210\u4E86 app-image \u76EE\u5F55 ({0})
Same as "de" translation.
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/MsiInstallerStrings_de.wxl
line 2:
> 1:
> 2: xmlns="http://schemas.microsoft.com/wix/2006/localization"; Codepage="1252">
Why "de-de" was changed to "de"?
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/MsiInstallerStrings_ja.wxl
line 2:
> 1:
> 2: xmlns="http://schemas.microsoft.com/wix/2006/localization"; Codepage="932">
Why "ja-jp" to "ja"?
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_de.properties
line 37:
> 35: resource.post-app-image-script=Auszuf\u00FChrendes Skript nach dem
> Auff\u00FCllen des Anwendungsimages
> 36: resource.post-msi-script=Auszuf\u00FChrendes Skript nach dem Erstellen
> der MSI-Datei f\u00FCr das EXE-Installationsprogramm
> 37: resource.wxl-file-name=MsiInstallerStrings_en.wxl
Why this was changed?
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_ja.properties
line 37:
> 35:
> resource.post-app-image-script=\u30A2\u30D7\u30EA\u30B1\u30FC\u30B7\u30E7\u30F3\u30FB\u30A4\u30E1\u30FC\u30B8\u3092\u79FB\u5165\u3057\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8
> 36:
> resource.post-msi-script=exe\u30A4\u30F3\u30B9\u30C8\u30FC\u30E9\u306Emsi\u30D5\u30A1\u30A4\u30EB\u304C\u4F5C\u6210\u3055\u308C\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8
> 37: resource.wxl-file-name=MsiInstallerStrings_en.wxl
Why this was changed?
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_zh_CN.properties
line 37:
> 35:
> resource.post-app-image-script=\u8981\u5728\u586B\u5145\u5E94\u7528\u7A0B\u5E8F\u6620\u50CF\u4E4B\u540E\u8FD0\u884C\u7684\u811A\u672C
> 36: resource.post-msi-script=\u5728\u4E3A exe
> \u5B89\u88C5\u7A0B\u5E8F\u521B\u5EFA msi
> \u6587\u4EF6\u4E4B\u540E\u8981\u8FD0\u884C\u7684\u811A\u672C
> 37: resource.wxl-file-name=MsiInstallerStrings_en.wxl
Same as above.
-
PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v4]
On Thu, 15 Dec 2022 23:01:35 GMT, Damon Nguyen wrote:
>> Open l10n drop
>> All tests passed
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Revert double quote as well
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/MsiInstallerStrings_ja.wxl
line 15:
> 13:Id="ShortcutPromptDlgStartMenuShortcutControlLabel">スタート・メニューのショートカットの作成
> 14: [ProductName]セットアップ
> 15: Open with [ProductName]
Why it was removed in all translations, except English?
src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_de.properties
line 51:
> 49: error.msi-product-version-minor-out-of-range=Nebenversion muss im Bereich
> [0, 255] liegen
> 50: error.version-swap=Versionsinformationen f\u00FCr {0} konnten nicht
> aktualisiert werden
> 51: error.icon-swap=Failed to update icon for {0}
Why this was removed?
-
PR: https://git.openjdk.org/jdk20/pull/35
