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

[Erik Joelsson]

On Fri, 27 Oct 2023 11:35:54 GMT, Jan Lahoda <jlahoda 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.

> The changes are indeed complex. I'm trying to tease out all the implications. I have a few questions/comments.
> 
> 2. The definition from spec.gmk says:
> 
> ```
> BUILD_JAVA_FLAGS := @BOOTCYCLE_JVM_ARGS_BIG@
> BUILD_JAVA=@FIXPATH@ $(BUILD_JDK)/bin/java $(BUILD_JAVA_FLAGS)
> ```
> 
> Thus it seems that BUILD_JAVA is using the "big" java flags (though I admit I did not follow to check exactly what BOOTCYCLE_JVM_ARGS_BIG means) . But the old code used JAVA_SMALL.
> 
> Is this an oversight, an assumption that it does not matter, or a measurement-founded decision that it does not matter? Maybe we should add a BUILD_JAVA_SMALL; or maybe it is not worth it. I cannot really say which, though I lend towards the former.

If we had a BUILD_JAVA_SMALL I would probably have used it. In this case I probably couldn't be bothered as it's just one java invocation, so won't make much of a difference. However, it was a while since I wrote this, so I don't actually remember. OTOH, for every single invocations we add, the number goes up and will eventually make a significant impact.
> 
> 3. The old code did `-add-exports java.base/jdk.internal.javac=java.compiler.interim,jdk.compiler.interim`. I can't say I understand what the meaning of it was, but I don't understand why it is removed now, either. I'd appreciate some explanation about this.

The java.base module has this in module-info.java:

exports jdk.internal.javac to
        java.compiler,
        jdk.compiler,
        jdk.incubator.vector,
        jdk.jshell;

Before this patch we used interim langtools to run this buildtool. The interim JDK N tools are built into module names with the `.interim` suffix so not to clash with the JDK N-1 versions of these modules in the bootJDK. This means that when running the interim langtools, `java.base` isn't exporting jdk.internal.javac to the interim modules. In most other cases where we use the interim langtools modules, we add `$(INTERIMA_LANGTOOLS_ADD_EXPORTS)` to work around this. This tool instead defined its own smaller set of add-exports. 

In this patch, we are instead using the BUILD_JDK to run the tool, which is a real JDK N with langtools modules included. When using that, the exports in java.base are enough.

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

PR Comment: https://git.openjdk.org/jdk/pull/16400#issuecomment-1802562841