Added support for including excluded dependencies when generating POM file. #453

Merged
merged 2 commits into from May 5, 2014

3 participants

@jdpgrailsdev
grails member

Added logic to the MavenPomGenerator to honor the Ivy exclusions. This change matches up the exclusion filters for a given dependency with its transitive dependencies in order to generate valid Maven XML. I also added a new Spock test to validate the change.

Jonathan Pearlin Added support for including excluded dependencies when generating POM…
… file.

Added null check to PluginBuildSettings when no plugin dependencies are
present.
30dd36a
@graemerocher
grails member

This change is problematic, because it introduces a dependency on Ivy to the MavenPomGenerator meaning that MavenPomGenerator will not work when Aether is configured

@jdpgrailsdev
grails member

Agreed. I wanted to support both Ivy and Aether, but grails-bootstrap is already a dependency of grails-aether, so it would have caused a cycle. I can make a fix that only does the exclusion resolution if "ivy" is the selected resolver. The other option would be to move the Aether-based resolver or the MavenPomGenerator to a more common location so that it could be used by both grails-bootstrap and grails-aether. Please let me know which way you would like to proceed.

@jdpgrailsdev
grails member

Another thought is to refactor out the single dependency resolution (for its transitive dependencies) to the DependencyManager interface (and its implementations) so that the MavenPomGenerator can look these up regardless of which dependency resolution scheme is chosen. I will take a crack at that, as I think it will address the issue.

Jonathan Pearlin Added single dependency resolution logic to DependencyManager to supp…
…ort exclusion functionality when creating a POM file.
889c9a7
@lhotari lhotari closed this Apr 14, 2014
@jdpgrailsdev
grails member

As this was closed without any additional comments/thoughts, any advice on how to proceed with an acceptable solution? I believe that the second commit listed above (jdpgrailsdev@889c9a7) is a step in the right direction, though it is definitely not the only way to solve this issue.

@lhotari lhotari reopened this Apr 15, 2014
@graemerocher graemerocher merged commit b8d88a1 into grails:2.3.x May 5, 2014
@graemerocher
grails member

@jdpgrailsdev The pull request was closed by accident by Lari, it was reopened, I only just saw you made the change. Don't hesitate to nag the dev list or send me an email if you get delayed again. Sorry about that.

@jdpgrailsdev
grails member

@graemerocher No worries. Thanks for the update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment