On Mon, 1 Dec 2025 15:17:13 GMT, Erik Joelsson <[email protected]> wrote:

>> Christoph Langer has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Cleanup comment
>>  - Fix pdb filtering for JMODs build
>
> make/CreateJmods.gmk line 72:
> 
>> 70:         DEST := $(LIBS_DIR_FILTERED), \
>> 71:         FILES := $(FILES_LIBS), \
>> 72:         NAME_MACRO := rename_stripped \
> 
> These trailing commas were deliberate and are part of the [Code Conventions 
> for the Build 
> System](https://openjdk.org/groups/build/doc/code-conventions.html) (17, 
> though it's not very clearly stated).

ok, fixed.

> make/Images.gmk line 313:
> 
>> 311:   $(call SetupCopyStrippedDebuginfo,JDK)
>> 312:   $(call SetupCopyStrippedDebuginfo,JRE)
>> 313: endif
> 
> I might be missing something, but why is this needed? Shouldn't the public 
> pdbs already be part of the jmods in this case and so would get linked into 
> the image by jlink?

You are right. And this seems to work with linkable runtime as well. So I 
removed that part.

> make/Images.gmk line 322:
> 
>> 320:       $(eval DBGFILES := $(if $(filter true+public,$(call 
>> isTargetOs,windows)+$(SHIP_DEBUG_SYMBOLS)), \
>> 321:         $(filter-out %.stripped.pdb,$(DBGFILES)),$(DBGFILES)) \
>> 322:       ) \
> 
> I think I would use lower case for `DBGFILES` to signify a local variable and 
> reduce risk of clashing with any other variable in the current context.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24012#discussion_r2584191011
PR Review Comment: https://git.openjdk.org/jdk/pull/24012#discussion_r2584193678
PR Review Comment: https://git.openjdk.org/jdk/pull/24012#discussion_r2584195087

Reply via email to