Am 12/19/16 um 18:26 schrieb Stephen Connolly: > We need to clarify. > > There is stuff that is "wrong" but has become expected behaviour because we > never "fixed" it => we cannot "fix" now because it will break too many users > > There is stuff that is "wrong" because we broke it, some users have > legitimate bugs and other users have hacks around the "wrong" => we will > break something no matter what we do, so maybe we can "fix" > > There is stuff that is "wrong" because it was never specified and we'd now > like it to work one way because that way feels "right" > > Etc > > What class of problem is this? > > To my mind, depMgmt should never alter the scope of a dependency... if you > specify a scope you are saying apply the rest when it is added with this > scope. > > But that is my "how I would like it to work"... > > Christian, you need to identify which kind of "wrong" behaviour is going on > here... then we can reframe the debate and find a path forward
That's by far not easy. I maybe should explain where most of this comes from. Working on an issue, I read a lot from the 'DefaultDependencyCollector' and involved classes from the resolver. That's like walking in a minefield. Also for the original author. During reading, I found some inconsistencies between various 'DependencySelector' implementations which happened to be the cause for the issue I was working on. I took the one implementation I understood was working as intendend and made two others match that logic (decide if a depencency is transitive or direct). Then wrote test cases for all of this to test they all behave the same. What influences the transitive vs. direct logic is how the 'CollectRequest' has been populated. For project resolution, the 'setRootArtifact(POM)' method should be used, for dependency resolution the 'setRoot(Dependency)' method should be used. <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-api/src/main/java/org/eclipse/aether/collection/CollectRequest.java;h=d9c252733d0bdcef3c3b6fcb3313a0b21f5b6253;hb=HEAD> This has a direct impact on the way the following methods need to decide transitive vs. direct. 'deriveChildSelector' here: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-api/src/main/java/org/eclipse/aether/collection/DependencySelector.java;h=b257ffa434695689922a6d1eee7d9f7f55497416;hb=HEAD> 'deriveChildManager' here: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-api/src/main/java/org/eclipse/aether/collection/DependencyManager.java;h=993e388b158ae1ec0b327c70d700c4d77c28243d;hb=HEAD> 'deriveChildTraverser' here: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-api/src/main/java/org/eclipse/aether/collection/DependencyTraverser.java;h=be1887bfdd67b35e427c491aefc00f7539a71f16;hb=HEAD> 'deriveChildFilter' here: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-api/src/main/java/org/eclipse/aether/collection/VersionFilter.java;h=fb36747f3ef8c875a46445ced2d3f37c69cc999d;hb=HEAD> Implementations in use are here: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=tree;f=maven-resolver-util/src/main/java/org/eclipse/aether/util/graph;h=1a02167e9815aed57cfc660f2f9abf155eb2ccf1;hb=HEAD> Sometime back in time, this difference between POM resolution and dependency resolution must have been introduced without all those 'deriveXyz' implementations having been updated to reflect that difference. That's what I did. They now all behave the same and correctly decide transitive vs. direct based on what the 'CollectRequest' has been setup to. The core use-cases are the following: 1. Collect project ------------------ CollectRequest.setRootArtifact(Artifact): The artifact passed to this method is a project. This will become the root node of the result tree. CollectRequest.addDependency(Dependency): The dependencies passed to this method are the *direct* dependencies of that POM the way the core model builder has build/managed them. CollectRequest.addManagedDependency(Dependency): The management from the POM to be applied to *transitive* dependencies. The direct dependencies already have been managed by the core model builder. Whereas the model builder only manages elements not declared directly, the resolver overrides all elements (it cannot know if a mandatory element has been declared directly or has been managed by the core model builder because the elements are just there.) 2. Collect dependency --------------------- CollectRequest.setRoot(Dependency): The dependency passed to this method is some *direct* dependency you got from somewhere. Management is considered to already be applied. This will become the root node of the result tree. CollectRequest.addDependency(Dependency): The dependencies passed to this method are considered to be *transitive* dependencies of the root dependency. The resolver will apply management to them. See above. That is different. Here they are transitive, whereas above they are direct. This is what was not handled consistently accross the various selectors. CollectRequest.addDependencyManagement(Dependency): The management to be applied to *transitive* dependencies. Plugins and core extensions were collected the 2nd way. Transitive 'test' and 'provided' dependencies were not collected, but transitive 'optional' dependencies were collected due to the inconsistent selector implementations. When the selectors started behaving consistently the transitive optional dependencies also were not collected. There was an IT testing this I updated for 3.4. Igor then stressed that this is incorrect and plugins should be resolved the same way as projects. That's what we now have on master. The update to the IT could be reverted. It really are "just" bugfixes. The bugfix in question is part of this commit: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=commit;h=1ee92862c67ec98564c4d8be1207355960f1dd5d> This diff: <https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blobdiff;f=aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDependencyCollector.java;h=62a8ff2a0ab8fd99576777446266ca588bc0fe40;hp=f1fcbf258de04b698e0ce23ca91e6827b5e6a47d;hb=1ee92862c67ec98564c4d8be1207355960f1dd5d;hpb=11a061b66fd15b8c3584f48afa69955d9392861e> Passing the managed dependency to the selectors instead of the unmanaged dependency is correct. I have no doubt about it. It just happens that the managed dependency may not get selected although the unmanaged would. The issues we are seeing is a direct result of this. It all were issues with the POM, not with the behaviour so far. What we now have on master is just consistent. This again has led to someone (Hervé) raising another issue. There should not be a difference in collecting a dependency or a project. Not collecting transitive 'test', 'provided' and 'optional' dependencies really seems incorrect following Hervé's point. Those selectors really should never remove any nodes no matter what and the resolver should then decide what to resolve after collection. Discarding a transitive test dependency during collection may make another dependency appear which would not have been there otherwise. Not setting the 'ScopeDependencySelector' and 'OptionalDependencySelector' on the 'RepositorySystemSession' and solely leaving the 'ExclusionDependencySelector' in place already should do that. That's what Robert is requesting as well, when I got that correct. Removing a dependency from the collect result should only be done by applying exclusions. Doing that it would change the whole collection result whenever there are conflicts between transitive and non-transitive dependencies resolved differently when the non-transitive dependency is part of what is passed to the conflic resolver or not. Those nodes currently are cut off during collection and are never passed to the conflict resolver. That cut off is what you are now seeing with the dependency management bugfix. That same cut off is happening without management applied as well and maybe that is what is to question here. It's hard to understand the original intention of all of that with this inconsistencies in place. The ones I noticed are fixed now. You see consistent behaviour and now we have something to talk about. Consider the management is behaving exactly the same way as if a transitive dependency had been declared that way in the model directly. It may uncover behaviour we are all relying on for years which was hidden somewhere no-one noticed. Just keep that in mind. The management is overriding everything transitive to what is in the management and the behaviour you are seeing with this is exactly the same as if the dependency would have been declared the same way as it is managed. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
