Hi all,
It seems the spring-boot.run.directories property doesn't work and external JARs in the directory aren't found by the classloader :
Caused by: java.lang.NoClassDefFoundError: gattaca/character/Vincent
at java.base/java.lang.ClassLoader.defineClass1(Native Method) ~[na:na]
at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1012) ~[na:na]
at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150) ~[na:na]
at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862) ~[na:na]
at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760) ~[na:na]
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681) ~[na:na]
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639) ~[na:na]
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) ~[na:na]
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520) ~[na:na]
When running the plugin in debug mode I do see that the directory is added to the classpath but the classloader doesn't seem to search for classes in it.
This was already reported on stackoverflow https://stackoverflow.com/questions/62389508/external-jar-class-not-found-while-run-time-in-spring-boot-maven but I can't used the mentioned workaround. Let me know if you need a reproducer.
Thanks!
Comment From: snicoll
The directory itself is added, that doesn't mean that JAR files are picked up. The documentation for the property states the following:
Additional directories besides the classes directory that should be added to the classpath.
If you put a JAR file in the "classes" directory, it won't be picked up either. Looking at the SO question, it doesn't seem to be related to what you've described. If I've missed something, please share a reproducer with additional details.
Comment From: johnpoth
Thanks @snicoll for the quick response!
So spring-boot.run.directories should point to classes! It's a bit misleading for me since in standard Java you can also point to JARs and the classloader is able to deal with those ...
Comment From: snicoll
What is misleading exactly? A directory and a file are two separate things and the Javadoc, as well as the name of the property is quite explicit about it being the former.
Standard Java requires you to use * if you want to load JAR files from directories.
Comment From: johnpoth
Hi @snicoll ! I can confirm pointing to files instead of directories works:
-Dspring-boot.run.directories=/path/to/jar/file/file.jar
Or as you've mentioned append a * when pointing to directories just like when using java -cp ...
-Dspring-boot.run.directories=/path/to/jar/directory/*
Thanks again and sorry for the noise ...
Comment From: snicoll
Or as you've mentioned append a * when pointing to directories just like when using java -cp ...
For the record, both of these is hacking the original feature. This feature is not meant to add additional jar files to the classpath, but additional classpath locations in the form of directories. Your first example point to a file (not a directory) and the second isn't a directory either.
We may decide to restrict or revisit this feature in a way that none of this works as it was never intended.
Comment From: johnpoth
Ah that's too bad! It's sometimes useful to add libraries to the classpath at runtime (e.g provided scope dependencies) or switch out an implementation of an API etc.. but maybe that could be provided by a new option instead of this one. Thanks!
Comment From: wilkinsona
The directory restriction feels a little arbitrary to me. I think it would be more natural to allow additional directories or jars to be added to the classpath. To be intuitive, that would require a new name for the property. While there's no command line option for it, the setting is analogous to bootRun's classpath to which files and directories can be added. Having some symmetry between Maven and Gradle is appealing.
Comment From: philwebb
We're going to add a new additionalClasspathElements property (similar to https://www.mojohaus.org/exec-maven-plugin/java-mojo.html#additionalClasspathElements) and pass the values directly to -cp without converting to File/URI/URL/File.
Comment From: rafaelrc7
Hello, is this issue available?
Comment From: philwebb
@rafaelrc7 Yes, you're welcome to work on it and submit a pull-request, however, we've not yet branched for 3.2 so we might not be able to merge it immediately.
Comment From: rafaelrc7
Hello, I would like to confirm a detail. It was noted that the idea would be to
pass the values directly to -cp without converting to File/URI/URL/File.
Currently, the values in the directories array (that will become the new additionalClasspathElements property) go through the conversion described above, done by the addUserDefinedDirectories() and addClasspath() methods in the org.springframework.boot.maven.AbstractRunMojo class.
As this conversion is not desired anymore, I was thinking about appending the values directly to the classpath in the addClasspath() method next to the already existing code that uses the getClassPathUrls() URL values. Would that be right?
The mentioned Mojohaus/exec-maven-plugin code (linked here) does some treatment of, for example, relative paths. Is that desirable? Or should the raw additionalClasspathElements contents be appended to the classpath?
Thanks in advance.
Comment From: rafaelrc7
I implemented the changes I described above in the way I understood them, and the diff can be visualised here:
https://github.com/rafaelrc7/spring-boot/compare/593fa1dc42177cc33b076643bd2ba6c66e84a3db..a012da4b6d4a1a3fe3de371155457320c61b7d82
I can then open a PR if it looks good, or make any changes as required.
Comment From: philwebb
It's interesting that the Mojohaus/exec-maven-plugin applies the additional path processing. I think we were hoping that we could get away with adding the string without needing any processing. Please feel free to submit a pull-request with your current approach, but we amend it before we merge it.