Fix Tooling API compatibility #14428
Conversation
|
|
||
| @Unroll | ||
| @Requires(adhoc = { TestPrecondition.JDK11_OR_LATER.fulfilled && !GradleContextualExecuter.embedded }) | ||
| def "Java #compilerJdkVersion.majorVersion client can launch task on Java #clientJdkVersion.majorVersion with Gradle #gradleVersion on Java #gradleDaemonJdkVersion.majorVersion"(JavaVersion compilerJdkVersion, JavaVersion clientJdkVersion, JavaVersion gradleDaemonJdkVersion, String gradleVersion) { |
There was a problem hiding this comment.
This is close to what I was thinking we could do, but I think we should avoid running the compiler ourselves. I also think we should run this test regardless if the test is running on JDK8, JDK11 or beyond.
Take a look at this reworking of this test:
89e981a
Instead of compiling the TAPI client by hand, everything is wrapped into a Gradle build that can do a few things for us:
- Resolve the TAPI client dependencies (vs relying on the classpath of the test to have it)
- Find a JDK8 toolchain so we can build 6-8 compatible classes
- Target the right compatibility level
I think this makes it a little easier to also compile a custom action since everything is contained in the 'client-runner' project.
PTAL and see if this is helpful.
There was a problem hiding this comment.
It is absolutely helpful, thanks Sterling, for making the modeling much nicer.
donat
force-pushed
the
donat/tapi-javaversion-experiment
branch
from
5573f17
to
b6a16c7
Compare
donat
force-pushed
the
donat/tapi-javaversion-experiment
branch
from
465636c
to
39412be
Compare
donat
force-pushed
the
donat/tapi-javaversion-experiment
branch
from
39412be
to
181caa3
Compare
- Instead of compiling a class from resources via the command-line, use a Gradle build with different JDK targets - We do not need to test the current release against itself, we have other TAPI tests to cover that - We do not need to test that a new JDK can compile backwards to older bytecode
donat
force-pushed
the
donat/tapi-javaversion-experiment
branch
from
181caa3
to
19a4648
Compare
|
I was not able to properly rebase the PR without including a bunch of unrelated files. It seems like a bug in GH. I've moved the PR to #14444. |
When a Tooling API client connects to Gradle, it loads classes from the target version and uses those to interact with the daemon. Unfortunately, the
JavaVersionclass is also involved in this process. The old versions of that class are not prepared for the new Java version number schemes. This means when a TAPI client (i.e. an IDE process) is running on a newer JDK it effectively cannot connect to older Gradle versions.Fortunately, the
JavaVersionclass is isolated: it doesn't have any dependency on other Gradle classes. Consequently, we can easily load a variant different from the one provided by the target Gradle version. This PR does just that: it forces the tooling implementation loader to load the class from the Tooling API jar, which contains the latest version.Our CI infrastructure missed this regression as all integration tests use the same Java version for the Tooling API client and for the Gradle daemon. This PR also implements this missing test coverage.