This is becoming a bug versus feature discussion. Up until now you've made changes which might change the resolution because you've marked them as a bug which must be fixed. However, what is 'the truth': the documentation or the code? The answer is: in the end it is the code. And if you want to have them in sync, you sometimes need to adjust the documentation instead of the code, because the code has a behavior one is used to. Since we're talking about changes in resolution, I also expose this topic. To me it is not a bug nor a feature, but it is a design flaw. And the issue is often not exposed, because the dependencies used for testing are not conflicting the compile dependencies. As long as the compilations works and all the tests run, users often don't look in detail at the dependency. The fact right now is that if I add/change a test-scoped dependency, it could happen that the project won't run due to a missing transitive dependency.
We are very, very lucky this doesn't happen that often.

Robert

On Sun, 25 Dec 2016 19:32:53 +0100, Christian Schulte <c...@schulte.it> wrote:

From what I can tell, the resolver now behaves exactly as the API
Javadoc states it should. I've also written various test cases for the
resolver testing the documented behaviour. From the resolver point of
view, MRESOLVER-8 and MRESOLVER-9 and MRESOLVER-10 are really "just"
bugfixes. How that manifests in Maven is another story. Looking at all
this (again and again and again) is confusing. Mostly because things you
are used to know how they behave change in a way no-one is used to. The
resolver currently really does what it's supposed to do.

Am 25.12.2016 um 18:01 schrieb Christian Schulte:
Am 12/25/16 um 11:57 schrieb Robert Scholte:
In Sun, 25 Dec 2016 05:23:14 +0100, Christian Schulte <c...@schulte.it>
wrote:

Am 12/24/16 um 18:40 schrieb Guillaume Boué:
Why would the PMD plugin care about what Doxia require transitively?
Christian, can you explain a bit more why those changes are needed?

Classpath consistency. Running the unit tests with a completely
different classpath than what the plugin is using at runtime makes those
unit tests meaningless, so to say.


We're talking about *unittests*, they should test one unit. In those cases
there should be no issue which such dependencies, because if that unit
requires a certain dependency, it should already have been defined as
direct dependency.

Exactly. In the "widest" scope required. In this case, that's not the
test scope but the compile scope. It's not the test scope overriding the
compile scope here. It's the nearest declaration overriding a transitive
dependency in an incompatible way. What if those two dependencies
(compile and test) would require different major versions of that
dependency? That's the "one tree per project" issue I have been talking
about.

If a test framework requires a newer version of some dependency also
required at compile time, it is weird to be forced to upgrade this
dependency just for testing (and what if this dependency is compiled with
a newer version of Java compared to the max of the project).

Still means you are not testing the runtime classpath. You know things
are working in test scope. There are different artifacts in the runtime
scope you never have tested. The ITs are different. You would need to
duplicate all unit tests in the ITs. That's not possible most of the
time (you are executing goals in the ITs - you cannot instantiate a
class and perform various tests with that from the ITs).

Maybe it is also caused by the current implementation of test tools. JUnit is using its own classpath for everything. I've been talking with them if
it would make sense to give the JUnit engine, the test classes and the
main classes all their own classloader. That would make it possible to
have better separation.

I understand the statement of classpath consistency, I used to think like that in the past too, but for *unit*testing this shouldn't matter. That's
why integration tests are just as important.

All I did is make an implementation behave as is documented in it's
Javadoc. Please take a look at this class.

<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>

Read the Javadoc of all constructors. Concentrate on direct vs.
transitive here. Read the Javadoc of the getDependencies and
addDependency methods as well. Again, concentrate on direct vs.
transitive here as well.

Now read the Javadoc of this class.

<https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/selector/ScopeDependencySelector.java;h=92db1417e8affadd976e2c51a38b33500161ce45;hb=HEAD>

/**
 * A dependency selector that filters transitive dependencies based on
their scope. Direct dependencies are always
 * included regardless of their scope. <em>Note:</em> This filter does
not assume any relationships between the scopes.
 * In particular, the filter is not aware of scopes that logically
include other scopes.
 *
 * @see Dependency#getScope()
 */
public final class ScopeDependencySelector
    implements DependencySelector

The class calling into this is the DefaultDependencyCollector.

<https://git-wip-us.apache.org/repos/asf?p=maven-resolver.git;a=blob;f=maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDependencyCollector.java;h=aaf5865f3fe74060e60be35f1d2bd9b13c69d206;hb=HEAD>

Do you notice there is an inconsistency in that 'CollectRequest' class
depending on the way it gets constructed? The 'getDependencies' method
clearly states:

@return The direct dependencies, never {@code null}.

Now compare if that statement is correct (direct vs. transitive)
depending on how that class has been constructed and tell me why only
the ScopeDependencySelector above has had logic deciding if those
dependencies are transitive or direct when all other
selector,traverser,manager implementations did not.

There is this resolver only a few people seem to have taken a closer
look at. I am not telling anyone how things should behave and what has
to change etc. There is an API Maven is calling into no-one has ever
verified is doing what it is stating in it's Javadoc. Now compare the
Javadoc of those constructors of that CollectRequest class above to the
Javadoc of the getDependencies and addDependency methods of that same
class. Only one class (ScopeDependencySelector) handled things
differently (direct vs. transitive). There are plenty of other classes
not handling anything differently. Either that ScopeDependencySelector
got it right and all other classes need to be updated or it's the other
way around. I tried both. That has led to MNG-6135. What we now have on
master (resolver and core) is consistent to what the Javadoc of those
getDependencies and addDependency methods state. And still Hervé made a
very valid point during this. Why is there such a difference at all?

Regards,



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

Reply via email to