On Tue, 13 May 2025 01:55:19 GMT, Alexander Matveev <almat...@openjdk.org> wrote:
>> Alexey Semenyuk has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 512 commits: >> >> - Add ConfigFilesStasher that allows to save contents of jpackage build >> directories in external directory; Add clean_stashed_files.sh >> - Merge branch 'master' into JDK-8333664 >> - Remove redundant StandardBundlerParam.createResource() >> - Adapt JDK-8352480 fix >> - Merge branch 'master' into JDK-8333664 >> - Fix javadoc >> - Merge branch 'master' into JDK-8333664 >> - 8333568: Test that jpackage doesn't modify R/O files/directories >> >> Reviewed-by: almatvee >> - 8356562: SigningAppImageTwoStepsTest test fails >> >> Reviewed-by: almatvee >> - Remove clean_stashed_files.sh >> - ... and 502 more: https://git.openjdk.org/jdk/compare/43696030...011eb710 > > src/jdk.jpackage/linux/classes/jdk/jpackage/internal/DesktopIntegration.java > line 477: > >> 475: return Optional.of(fa) >> 476: .flatMap(FileAssociation::icon) >> 477: .map(DesktopIntegration::getSquareSizeOfImage) > > `getSquareSizeOfImage` will return 0, if exception is thrown for invalid > image. Should it return -1 or should we only accept icons if `getIconSize() > > 0`? `getIconSize()` returns the value returned by `getSquareSizeOfImage()` call or -1 if icon is not specified. This way the return value of `getIconSize()` falls into three categories: - <0 - failed to get dimensions of the given icon; - 0 - no icon file specified; - \>0 - icon size; If we change `getSquareSizeOfImage()` as you suggest, the return value of `getIconSize()` will fall into two categories: - <0 - failed to get dimensions of the given icon, or the icon was not specified; - \>0 - icon size; This change will make it impossible to tell if the icon was given or if an error occurred reading it from the return value of `getIconSize()`. The caller code accepts an icon for the file association only if its size >0. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19668#discussion_r2093568135
