RFR: 8321224: ct.sym for JDK 22 contains references to internal modules [v2]

Jan Lahoda jlahoda at openjdk.org
Mon Dec 4 21:27:59 UTC 2023


On Mon, 4 Dec 2023 11:39:04 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reflecting review feedback - keeping CT_TRANSITIVE_MODULES separate from CT_MODULES.
>
> make/modules/jdk.compiler/Gendata.gmk line 41:
> 
>> 39: CT_MODULES := $(filter-out $(MODULES_FILTER), $(DOCS_MODULES))
>> 40: CT_EXTRA_MODULES := jdk.unsupported
>> 41: CT_TRANSITIVE_MODULES := $(call FindTransitiveIndirectDepsForModules, $(CT_MODULES)) $(CT_MODULES)
> 
> Now the CT_TRANSITIVE_MODULES will include all CT_MODULES. I am pretty sure the intention was to keep them separated, for clarity, but to act on both CT_MODULES and CT_TRANSITIVE_MODULES. See e.g. the foreach on line 43. (This change will also make that foreach process CT_MODULES twice.)
> 
> Also, just for code readability, can you move the hard-coded list of extra modules to after the programmatic definitions of CT_MODULES and transitive modules, perhaps even with an empty line in between?
> 
> I think a better solution is to change your new line to:
> 
> $(ECHO) $(CT_MODULES)  $(CT_TRANSITIVE_MODULES)  $(CT_EXTRA_MODULES) >$(SUPPORT_OUTPUTDIR)/symbols/included-modules

Thanks for the comment! I tried to act on it here:
https://github.com/openjdk/jdk/pull/16942/commits/879277891708533b0b56dbe36b20760c62708bbf

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16942#discussion_r1414515956


More information about the build-dev mailing list