Don't delegate to parent CL for "leaking" dependencies from build system #4905
Conversation
This solution is pretty bad and should be temporary until a proper solution like quarkusio#770 is implemented
|
As discussed at https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/gradle/near/179211143 my hunch is that this is better fixed by actually using an isolated classloader rather than use our own custom one hooked up to gradle. Gradle's worker api allows you to control it - see https://guides.gradle.org/using-the-worker-api/#changing_the_isolation_mode culprit is at https://github.com/quarkusio/quarkus/blob/master/core/creator/src/main/java/io/quarkus/creator/phase/augment/AugmentTask.java#L162 and that code looks (on first look) ripe for using Worker API to separate our quarkus work from gradle work. |
|
@maxandersen makes sense. However I am not sure we can get that done in the short term |
| // The solution is pretty bad and should be views as temporary until a proper solution | ||
| // (like the one proposed here: https://github.com/quarkusio/quarkus/issues/770) | ||
| if (lookup) { | ||
| Class<?> loadedClass = findLoadedClass(name); |
There was a problem hiding this comment.
I think this might be problematic. What is Gradle has already loaded some classes from guava? findLoadedClass will return the one loaded by the base class loader, but any classes that have not been loaded yet will be loaded by this class loader, breaking any package protected access and loading different versions.
There was a problem hiding this comment.
I am assuming that would only happen if some plugin used in the build depended on Guava and was unshaded. It could probably happen, but I am assuming it won't be very likely.
There was a problem hiding this comment.
Why would it be in the build classpath if it is not being used?
There was a problem hiding this comment.
Guava sneaks in (or at least used to sneak in) because it was a dependency of the maven resolver, or some other library we use in boostrap.
|
I am going to try and find the time to come up with a better solution for this today. |
|
I think this approach might be better for now: #5013 |
|
So class loader isolation won't really help here, as the underlying issue is that gradle does not seem to differentiate between user dependencies and plugin dependencies (at least as far as I can tell, there may be an API to do this that I am not aware of, alternatively it may be related to how we structure our build files). Class loader isolation does not help when we can't determine which artifacts are actual runtime artifacts that the user wants, vs artifacts brought in by plugins. |
|
I'll keep this around in draft for just reference sake and close it when we have a proper classloader system in place. |
|
Closing since we probably won't do this |
|
@gsmet why did you mark this as invalid ? |
|
reopening it since at it is worth exploring if we can't get stop gradle plugins from mingling with user dependencies or at least verify that this and related issues are all gone now. |
|
gah- stupid github, wrong PR/issue. closing ;) |
Fixes: #4767
Would be great to have #4579 in so some new integration tests could be added with known problematic cases.
The problem is caused because the parent CL (defined here) contains all the classes of the build system.
This solution is pretty bad and should be temporary until a proper solution
like #770 is implemented.