Verify that Java language level isn't higher than runtime version #18340
Conversation
fmeum
force-pushed
the
17281-warnings
branch
12 times, most recently
from
dea1f3c to
26b39ce
Compare
The value of `--(tool_)java_language_version` results in a matching `-target` argument passed to javac and thus in the generation of class files that only run on runtimes of the same or higher versions. This commit introduces a check that fails if the language version is strictly higher than the version of the current runtime. In the exec configuration, this covers two situations at once: * Compilation of a Java tool, where the runtime version is determined by `--tool_java_runtime_version`. * Compilation with a `java_plugin`, where the runtime version is determined by the `java_runtime_version_alias` target specified as the Java toolchain's `java_runtime`. The latter situation led to especially confusing error messages when the `--tool_java_language_level` was higher than the version of the runtime used for Java compilation.
fmeum
changed the title
|
@cushon Could you take a look? This implements your suggestion from #17281 (comment). I wonder whether having a new |
github-actions
bot
added
awaiting-review
There was a problem hiding this comment.
Thanks for picking this up, it would be nice to make the interaction between these flags less bug-prone
I wonder if this is going to have compatibility impact for existing builds that are getting away with configurations that are now rejected. @hvadehra WDYT? Maybe we could get a sense by running it through the downstream projects pipeline?
| runtimeVersion = Integer.parseUnsignedInt(rawRuntimeVersionLastPart); | ||
| } catch (NumberFormatException ignored) { | ||
| // Could be e.g. the "jdk" part of "local_jdk" without a version number, which we can't | ||
| // check at this point. |
There was a problem hiding this comment.
It seems slightly dicey to figure out the Java features version from the string name of the runtime version. That convention doesn't hold for all of the Java runtimes we define at google, for example.
Is there somewhere we could do this validation when we have the JavaRuntimeInfo, in order to check the java_runtime.version instead of the value of --java_runtime_version=?
| rawLanguageVersion, rawRuntimeVersion)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If we go with the logic in this class, it would be nice to have some unit test coverage for the parsing logic in JavaConfigurationTest
|
Parsing the runtime name isn't the greatest, I agree. It is also possible for users to manually specify a Maybe it would suffice to perform this check in the exec configuration? The situation where an annotation processor isn't compatible with the Java runtime running javac is definitely the most confusing situation improved by this PR. It could also be sufficient to catch an exception in the right place in |
I think that's a possibility, but would that still be parsing the runtime name? WDYT about the other option I mentioned of doing the validation later during analysis when we have the
That also seems like a possibility. I was thinking about doing the validation in JavaBuilder, but it would be nice to avoid extra logic that inspected the bootclasspath before starting compilation. Just adding additional context to the error that would be reported anyways addresses that. |
|
Superseded by changes to JavaBuilder. |
github-actions
bot
removed
the
awaiting-review
The value of
--(tool_)java_language_versionresults in a matching-targetargument passed to javac and thus in the generation of classfiles that only run on runtimes of the same or higher versions.
This commit introduces a check that fails if the language version is
strictly higher than the version of the current runtime. In the exec
configuration, this covers two situations at once:
by
--tool_java_runtime_version.java_plugin, where the runtime version isdetermined by the
java_runtime_version_aliastarget specified as theJava toolchain's
java_runtime.The latter situation led to especially confusing error messages when
the
--tool_java_language_levelwas higher than the version of theruntime used for Java compilation.
Work towards #17281