HHH-13682 Generate Java 11/13/14 bytecode for tests when building with JDK11/13/14 #3329
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The build requires JDK8+, so we're alwways Java 8 compatible.
It's no longer necessary since we upgraded to byte-buddy 1.10.2, and it causes bytecode to be converted from Java 14 to Java 12 in some cases (I don't know why).
…lizedAssociationTest
This test accesses a field of an entity directly and expects it to be
automatically initialized; this cannot work without extended bytecode
enhancement.
This used to work with Java 8 bytecode, but only by chance. It seems
that Java 8 bytecode relies on "synthetic", static access methods
inserted by the compiler to access the fields of entities in this test:
any access to the field is done through this access method instead of
through a direct field access. Since we apply bytecode enhancement to
all methods of entities, this means that access to fields triggers
initialization, without any bytecode enhancement in the caller class.
I believe this is specific to nested classes, but couldn't find a
source. For reference, the bytecode of access methods looks like this:
static int access$002(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, int);
Code:
0: aload_0
1: iload_1
2: dup_x1
3: putfield hibernate#3 // Field id:I
6: ireturn
static org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId access$102(org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$AnEntity, org.hibernate.test.bytecode.enhancement.lazy.NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId);
Code:
0: aload_0
1: aload_1
2: dup_x1
3: putfield hibernate#2 // Field entityImmutableNaturalId:Lorg/hibernate/test/bytecode/enhancement/lazy/NaturalIdInUninitializedAssociationTest$EntityImmutableNaturalId;
6: areturn
With Java 11, however, access to fields of entities is done directly,
even for nested classes. So the access methods no longer exist, and we
don't get automatic initialization upon field access. We need extended
bytecode enhancement, like we would in any other case of field access
(in particular accessing fields of non-nested classes).
... just in case we need that for some cutting-edge JDK, for example 15, that would not be supported by Gradle yet.
So that we can feed it Java 13/14 bytecode
yrodiere
changed the title
Sanne
reviewed
Apr 14, 2020
| int majorJVMVersionInt = Integer.valueOf(JavaVersion.current().toString()); | ||
| //Set the -Dnet.bytebuddy.experimental=true property only when we need it: | ||
| if (majorJVMVersionInt >= 12) { | ||
| systemProperty 'net.bytebuddy.experimental', true |
There was a problem hiding this comment.
We no longer need this? Perhaps the check should be bumped to >= 15 ? It's kinda a pattern for ByteBuddy to only support the "work in progress" JDKs when this flag is on. I didn't check if the strategy changed.
Sanne
approved these changes
Apr 14, 2020
There was a problem hiding this comment.
Changes look good, but I have a conceptual question:
are we still able to check that Hibernate ORM - as built with JDK8 - runs fine in JDK11 ?
Because that's what matters, since we release with JDK8. Clearly we could have tests (usercode) compiled with JDK11.
We never had that ability in the first place. We were always building with JDK11 when testing in JDK11. It's just that we were setting the target to Java 8, so we were generating Java 8 bytecode... through JDK11. Also, this patch does not change the bytecode generated for ORM in any way. It's just about the tests. To sum up:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://hibernate.atlassian.net/browse/HHH-13682
This PR is based on #3328 (HHH-13925 - upgrade to Gradle 6.3) . #3328 should be merged first. This PR includes commits from #3328, so it should be rebased after #3328 has been merged.=> DoneThe goal here is simply to compile tests to Java 11/13/14 when we build with JDK 11/13/14. That way, not only do we check that Hibernate ORM can be run within JDK 11/13/14, but we also check that it will correctly interact with Java 11/13/14 bytecode, which is especially important for bytecode enhancement.
Hopefully this will allow us to avoid the kind of problems we've had before, where bytebuddy needed an update and we didn't notice it because we were only ever manipulating Java 8 bytecode in our tests.
Build against JDK13: https://ci.hibernate.org/view/Personal%20jobs/job/hibernate-orm-personal-yoann-jdk13/8/
Build against JDK14: https://ci.hibernate.org/view/Personal%20jobs/job/hibernate-orm-personal-yoann-jdk14/4/
How to test that Java 11/13/14 is actually generated for tests, and not for the main code: