-
Notifications
You must be signed in to change notification settings - Fork 72
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
Weaver doesn't consider transitive dependencies #69
Comments
Here's a workaround I've created... if there's an easier way, please let me know:
|
Hi @marcopelegrini, <configuration>
<weaveDependencies>
<weaveDependency>
<groupId>org.agroup</groupId>
<artifactId>to-weave</artifactId>
</weaveDependency>
<weaveDependency>
<groupId>org.anothergroup</groupId>
<artifactId>gen</artifactId>
</weaveDependency>
</weaveDependencies>
</configuration> If so, no, transitive dependencies are not supported yet. PRs are very welcome, though. :) |
Hi @bmarwell that's exactly right... so looks like it's not an issue but a feature request :) |
Sounds good to me! I'd make it not default, because I think the dependent transitive jars should already contain the woven classes, or am I missing something? |
I would be very careful to implement this. If at all, it should be opt-in, like Ben said, because it would break backward compatibility. It is just not how Ajc works. You usually do not want to weave the world, but be explicit about what you want woven. Granted, a Maven plugin is an abstraction layer and not necessarily a 1-to-1 mapping to underlying tool parameters, so I can imagine this as a convenience feature. But spontaneously, I am skeptical and would rather not implement it. But that is, of course, up to the team of plugin maintainers. We should stop for a moment and think about how other plugins handle transitive dependencies and what would be the most probable user expectation. Please also consider the test matrix that would ensue for such a feature. Last but not least, think about what it would mean for aspect weaving: It would become dependent on how some upstream maintainer of a third-party component changes her POM. The results could be quite unpredictable. OK, opt-in means "use at your own risk", but AspectJ Maven already being "copy & paste programming" in large parts, based on my StackOverflow experience, I am expecting lots of newbies to use it because it sounds cool and promises to save typing and doing POM maintenance work. The result would be lots of support questions. |
Alex, thanks for your insights!
I am considering and valuing your input as well!
That is what I fear most. Even with opt-in, I think builds would break randomly because some of your project's dependencies introduced some breaking change. @marcopelegrini can you please give us a use case? |
Yes, for sure... if you look at my workaround you kinda grasp what I'm trying to do.. I have a multi-module project, layered out like this:
So if I configure the plugin in the In my case the ideal configuration supported would be some sort of pattern based configuration, because I prefer not to specify all transitive artifactIds (they might change over time), but I want to weave every dependency that's part of my own groupId.
|
That is something completely different and much simpler than transitive dependency inclusion. You simply want to use patterns for artifact IDs or maybe also groupIDs. That would rather help for the case of multiple direct dependencies from the same group ID. I would not expect that this also applies recursively. Recursion into the dependency tree via something like <configuration>
<includeTransitiveWeaveDependencies>true</includeTransitiveWeaveDependencies>
<weaveDependencies>
<weaveDependency>
<groupId>my.groupId</groupId>
<artifactId>A</artifactId>
</weaveDependency>
</weaveDependencies>
</configuration> or, more specific, <configuration>
<weaveDependencies>
<weaveDependency>
<groupId>my.groupId</groupId>
<artifactId>A</artifactId>
<includeTransitive>true</includeTransitive>
</weaveDependency>
</weaveDependencies>
</configuration> really would be a separate feature, which - if active - ideally would be usable in combination with name filtering. Something like <configuration>
<weaveDependencies>
<weaveDependency>
<groupId>my.groupId</groupId>
<artifactId>A</artifactId>
<transitive>
<includes>my.groupId:*,org.apache.*:*</includes>
<excludes>org.apache.*:ant*</excludes>
<transitive>
</weaveDependency>
</weaveDependencies>
</configuration> Because it would be the next logical thing users would come up with: to demand that the feature supports situations like "I want to include transitive dependencies, but not all of them". But then we are starting to rebuild logic like in Maven Shade. |
For sure, that's a different solution.
Yes, so this would ignore where the dependency is coming from, and rather weave them based on that pattern:
|
If you include a dependency to be weaved that has a transitive dependency that you also wants to weave, this later is not considered unless explicitly specified.
Is there a workaround for that?
The text was updated successfully, but these errors were encountered: