Issue 145: Fix aggregate-add-third-party modifying project dependencies

[srdo]

Fixes #145.

The issue occurs because the plugin modifies the MavenProject dependency lists in DefaultDependencyTool.loadProjectArtifacts. Dependencies may in some cases be removed if they belong to the same reactor as the current project.

I've added a modified version of the reproduction code from https://github.com/jimonthebarn/maven-license-plugin-issue as a test.

The changes are attempting to preserve the current behavior of the plugin, while also not modifying the MavenProject fields.

[srdo]

Hm, Spring doesn't work with Java 7 anymore. I'll try to rewrite using the shade plugin instead.

[ppalaga]

@srdo thanks for the PR. Great that it contains an integration test! Please rebase on top of master and see the comments above.

[srdo]

I think the comments are addressed. Let me know if it looks good, and I'll squash.

[srdo]

I think there's a small bug in DefaultDependenciesTool.loadProjectDependencies, will try to make a test that covers it and update.

[srdo]

Rebased, and fixed another few bugs in loadProjectArtifacts. I wound up replacing the method. The previous implementation would fail to find transitive dependencies for long chains of sibling dependencies in the same reactor, and would use the wrong list of remote repositories for looking up dependencies. Both issues are demonstrated in it/ISSUE-145-2. The latter bug would cause the build to crash.

The issue with wrong transitive dependencies wasn't a big deal, but I think we might as well fix it. When the aggregator mojo finds dependencies for sibling projects, it was possible to be missing some dependencies in the return from loadProjectArtifacts. For instance if you have siblings A, B, C and A depends on B depends on C, then loadProjectArtifacts for A would replace B with C in the direct dependency list. This was wrong, but since the aggregator mojo processes all modules in the reactor, B would still be picked up when the aggregator mojo moved on to processing B.

[srdo]

The Java 7 tests failed, they pass for me locally. Going to retrigger the build.

[ppalaga]

Rebased, and fixed another few bugs in loadProjectArtifacts. I wound up replacing the method. The previous implementation would fail to find transitive dependencies for long chains of sibling dependencies in the same reactor, and would use the wrong list of remote repositories for looking up dependencies. Both issues are demonstrated in it/ISSUE-145-2.

Thanks a lot for the fixes and special thanks for the test!

The issue with wrong transitive dependencies wasn't a big deal, but I think we might as well fix it.

Not sure you imply (a) you fixed it or (b) it needs to be fixed. If (b) is the case, please file a new issue.

The PR looks good to me now. Please squash the commits, and reword to something like Fix #145 aggregate-add-third-party modifying project dependencies and I'll merge afterwards.

[srdo]

Not sure you imply (a) you fixed it or (b) it needs to be fixed. If (b) is the case, please file a new issue.

It is fixed now.

Squashed.

Thanks for reviewing.

[ppalaga]

@srdo good work, thanks!