RFR: 8318913: The module-infos for --release data do not contain pre-set versions

[Magnus Ihse Bursie]

On Wed, 8 Nov 2023 16:22:31 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> Consider a simple module, like:
>> 
>> module test {}
>> 
>> 
>> And compile it with JDK 22 and JDK 21 using:
>> javac --release 21
>> 
>> The results of the compilations will differ: when compiling with JDK 21, the mandated java.base dependency will get a version, possibly like "21-internal". When compiling with JDK 22, the version of the java.base dependency will be empty.
>> 
>> This is a) because `module-info.class`es in `ct.sym` do not have any module version set; b) for JDK N, `--release N` is not using `ct.sym`, but rather `lib/modules`, which may contain a range of version specifiers.
>> 
>> This patch does two changes:
>> a) tweaks the `module-info.class`es in `ct.sym`, so that they contain a simple version. For `--release N`, the version is `N`.
>> b) tweaks the whole build so that `ct.sym` is used always for `--release`, a `lib/modules` is never used. I.e. the appropriate classfiles are copied into `ct.sym`. This not only allows for a general approach to module versions, but simplifies the `--release` handling in javac, and should enable future improvements. This is, however, a relatively big change.
>> 
>> The use of `lib/modules` for `--release <current>` was made to improve build performance, but the build has been updated since this has been introduced, so the slowdown caused by rebuilding `ct.sym` should be much lower now.
>> 
>> With these changes, compiling with `--release N` should record the same dependency versions in `module-info` on JDK N and JDK N + 1.
>
> make/modules/jdk.compiler/Gendata.gmk line 75:
> 
>> 73:     $(wildcard $(MODULE_SRC)/share/data/symbols/*) \
>> 74:     $(MODULE_INFOS) \
>> 75:     $(foreach m, $(CT_MODULES) $(call FindTransitiveIndirectDepsForModules, $(CT_MODULES)), \
> 
> This dependency line is hard to read. Maybe extract it to a variable and depend on the contents of the variable instead?
> 
> Also, this is the only place where CT_MODULES is used. I'd recommend also creating an intermediate variable like CT_TRANSITIVE_MODULES for $(call FindTransitiveIndirectDepsForModules, $(CT_MODULES)), and grouping all these declarations in the same place (either in the top of the file as now, or closer to the usage here).

In fact, it took me some time to realize these were dependencies, and not lines in the rule. Maybe if we also make a variable for the symbols/* dependencies, we can fit all dependencies in a single line? That'd definitely help with readability. 

Alternatively, do not separate each dependency on a single line, but start them directly after the colon, and just break lines when they no longer fit due to line length.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16400#discussion_r1386893537