-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
HHH-14082 Hibernate cannot determine its core version in modular configuration #10726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
final String version = Version.class.getPackage().getImplementationVersion(); | ||
return version != null ? version : "[WORKING]"; | ||
ModuleDescriptor moduleDescriptor = Version.class.getModule().getDescriptor() ; | ||
return moduleDescriptor != null ? (moduleDescriptor.version().isPresent() ? moduleDescriptor.version().toString() : "[WORKING]") : "[WORKING]"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a fallback to getPackage().getImplementationVersion()
somewhere here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to suggest that maybe we could do something similar to this version injection into bytecode as it's done for Search e.g. here:
But then I notice that there already seems to be an injection gradle plugin:
Line 22 in 9361a1b
id "org.hibernate.build.version-injection" version "2.0.0" apply false |
Maybe we should just make sure that the plugin is used in this case?
(cc7c7d7 this commit might be related as it seems it "removes" the injection part?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a fallback to
getPackage().getImplementationVersion()
somewhere here?
Hello Gavin,
Ok, I will look through. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a fallback to
getPackage().getImplementationVersion()
somewhere here?
Hello @gavinking,
Is it better to check getPackage() and getImplementationVersion() or to inject "org.hibernate.build.version-injection" in gradle? Which one do you recommend?
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mehmetali2003 I don't know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(cc7c7d7 this commit might be related as it seems it "removes" the injection part?)
It's been a while, but I don't think the intent was to remove the bytecode injection part. I think the intent was to make sure the constant would be initialized as a "real constant" when running the native-image compiler.
So I suspect we simply never noticed that the bytecode injection had stopped working, as Version.class.getPackage().getImplementationVersion()
has been effective enough (until today..)
+1 to restore the bytecode injection. Perhaps we should do it in such a way to actually cause a hard-failure when it fails, so to notice such defects earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm so it seems the plugin is just disabled and not configured at the moment 😖
We'd need at minimum:
versionInjection {
into( 'org.hibernate.Version', 'initVersion' )
}
as an injection plugin config, then
id "org.hibernate.build.version-injection" apply true
actually enable the plugin for the core module (it is disabled in one of the more common build files)
aaaand we'd also need a new release of the plugin... with the current version it is failing with the serialisation exception (+ it won't find the method as it's private so another tiny change to the plugin is needed). I have a fix locally, but looking at the plugin repository, we'd need to update its release process first 🙂 ,
with all that in place, it renders the following version class:
public final class Version {
private static final String VERSION = initVersion();
private static String initVersion() {
return "7.1.1-SNAPSHOT";
}
private Version() {
}
public static String getVersionString() {
return VERSION;
}
public static void logVersion() {
((CoreMessageLogger)Logger.getMessageLogger(MethodHandles.lookup(), CoreMessageLogger.class, Version.class.getName())).version(getVersionString());
}
@AllowSysOut
public static void main(String[] args) {
System.out.println("Hibernate ORM core version " + getVersionString());
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marko-bekhta But is all this actually better than just using the features that are built into Java?
HHH-14082 Hibernate cannot determine it's core version in modular configuration
Currently the Version gets the information via: Version.class.getPackage().getImplementationVersion()
But in modular mode it should call this instead: Version.class.getModule().getDescriptor().version()
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-14082