-
Notifications
You must be signed in to change notification settings - Fork 88
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
ArtifactResolver's getAllDependecies method returns a list of dependencies unfiltered by maven resolver's standard version conflict resolution process #190
Comments
Hi @Hillrunner2008 can you provide an example of the issue ? I think I understand the issue is that if we have several times the same dependency will likely to be part of the deployment and class loader will try to load classes from all of them in certain order (not clear which one). I think it is a valid bug and good catch... do you mind to open a jira in https://issues.redhat.com/projects/JBPM/issues/JBPM-9450?filter=allopenissues (feel free to asign to em). |
@elguardian A simple example would be a single kjar with a dependency tree including any conflict Example) The maven way to resolve the dependencies will correctly prune 18.0.0 as nearest neighbor is preferred when a conflict in the graph is found. My observation is that because this method (https://github.com/kiegroup/kie-soup/blob/master/kie-soup-maven-utils/kie-soup-maven-integration/src/main/java/org/appformer/maven/integration/ArtifactResolver.java#L155) does not resolve conflicts there are cases (like in kie-server deployments) where the inability to resolve the dependency graph "superset" prior to the conflict resolution will cause a deployment to fail. To put a finer point on it, if the goal is to prepackage all necessary dependencies for a kjar to avoid runtime resolution from a remote maven repository as you likely would in the case of a containerized kie-server to truly have something immutable you will find that the kiecontainer deployment will fail when you don't package the unnecessary conflicted libraries (e.g. both C 18.0.0 and 21.0.0). It was in this context I stumbled into the issue and was curious about the root cause. I doubt there is a good use case for this method to be returning "all" dependencies without conflict resolution filtering, and believe the fix is actually as simple as removing this custom code for calculating the dependencies as its already available elsewhere. |
Created https://issues.redhat.com/browse/JBPM-9860, but I don't have permission to change the assignment. |
Proposed fix #223 |
closing this one as there is one jira already |
I'm unclear on the reason for the getAllDependencies method.
https://github.com/kiegroup/kie-soup/blob/master/kie-soup-maven-utils/kie-soup-maven-integration/src/main/java/org/appformer/maven/integration/ArtifactResolver.java#L155
This method aggregates together all dependencies both direct and transitive into a Set; however, this Set is only constrained by duplicates and does not attempt to resolve version conflicts in the standard way maven would in the same case. (see https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Transitive_Dependencies).
The drools project leverages this method (see https://github.com/kiegroup/drools/blob/master/kie-ci/src/main/java/org/kie/scanner/KieModuleMetaDataImpl.java#L173) and a side effect of this turns out to be many transitive artifacts are resolved (i.e. downloaded to local disk repo). Additionally, this project goes on to attempt to load all classes from all versions of a dependency into its KieContainer instance's classloader.
Luckily this turns out to fail since that classloader was not backed by the same set of resolved artifacts, but its easy to imagine a worse outcome and the chaos this would create.
A side effect of this "bug" that impacts the kie-server project in the drools ecosystem is that deployments of kjars to the kie-server application require a maven repository either contain or actively resolve these unnecessary dependencies. In the context of containerizing a kjar backed application this presents an interesting challenge as approaches such as this (see https://github.com/kiegroup/droolsjbpm-integration/blob/master/kie-maven-plugin/src/main/java/org/kie/maven/plugin/PackageKjarDependenciesMojo.java) seem destined to reliably fail in all cases where there is a conflict in the dependency graph. To be clear, when I say fail I only mean the expectation that an artifact can be resolved is baked into the product as it does not seem to have any awareness that some of these dependencies really should not be coming back from the ArtifactResolver's getAllDependecies method at all.
I do not have any context on if this method has value in some other way, but I believe its likely a bug and at best is misleading.
The text was updated successfully, but these errors were encountered: