On Fri, 27 Mar 2026 16:06:36 GMT, Alan Bateman <[email protected]> wrote:

>> David Beaumont has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 12 commits:
>> 
>>  - Merge branch 'master' into jimage_preview_mode
>>  - rename jimage_exists to jimage_is_open
>>  - Feedback tweaks
>>  - Feedback tweaks
>>  - More feedback tweaks
>>  - Updated copyright
>>  - Feedback changes
>>  - Merge branch 'master' into jimage_preview_mode
>>  - Merge branch 'master' into jimage_preview_mode
>>  - undo exploded image changes for now
>>  - ... and 2 more: https://git.openjdk.org/jdk/compare/7695b1f9...0e802079
>
> src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java line 132:
> 
>> 130:      *     is present.
>> 131:      * @return flags for the ATTRIBUTE_PREVIEW_FLAGS attribute.
>> 132:      */
> 
> getPreviewFlags looks very strange. Is this motivated by tests? (I wouldn't 
> expect anything other than the jlink image writer to be concerned with this).

It's where it is because it's using the encapsulated FLAG_XXX constants. It's 
split out as this separate util function because the existing code has 2, 
completely separate locations, at which it needs the flags calculated. The docs 
for the method explain it:


     * <p>Since preview flags are calculated separately for resource nodes and
     * directory nodes (in two quite different places) it's useful to have a
     * common helper.


If the existing code were re-worked to avoid processing resource and directory 
nodes in completely difference classes (`ImageResourcesTree` and 
`ImageFileCreator`), this could probably be neatened up. Any other solution 
would involve code duplication and/or breaking encapsulation.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3008361537

Reply via email to