[MDEP-791] Non-test scoped and transitive dependencies in compile scope

[jira-importer]

Slawomir Jaranowski opened MDEP-791 and commented

When we use some dependency in test classes which is not used in production code but is required as transitive dependency for other used in production code - such dependency should not be included in {}Non-test scoped{}.

Example:

• test code use ObjectCodec from jackson-core
• production code use only ObjectMapper from jackson-databind
• production code don't use any classes from jackson-core

jackson-core is needed by jackson-databind and must by in compile scope so should not be reported as Non-test scoped

 

---

Affects: 3.2.0, 3.3.0

Attachments:

• MDEP-791.zip (2.84 kB)

Issue Links:

• MDEP-753 Non-test dependency reported as Non-test scoped test only dependency
  ("split from")
• MNG-6058 Test dependencies should override application dependencies only during testing
  ("is caused by")
• MNG-6058 Test dependencies should override application dependencies only during testing

15 votes, 26 watchers

[jira-importer]

Elliotte Rusty Harold commented

Thanks for the clear description. This is working as intended and designed. In the case cited jackson-databind should declare a dependency on jackson-core. Your pom.xml should only declare a test scoped dependency. That is, only declare the direct dependencies in each scope. Do not declare transitive dependencies.

[jira-importer]

Slawomir Jaranowski commented

Test project in attachment [^MDEP-791.zip]

Step to reproduce

jackson-core no on direct dependency

mvn clean verify 

INFO] --- maven-dependency-plugin:3.3.0-SNAPSHOT:analyze-only (check-dependencies) @ tstp ---
[WARNING] Used undeclared dependencies found:
[WARNING]    com.fasterxml.jackson.core:jackson-core:jar:2.13.1:compile
[WARNING] Non-test scoped test only dependencies found:
[WARNING]    com.fasterxml.jackson.core:jackson-core:jar:2.13.1:compile
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

 

jackson-core with compile scope

mvn clean verify -P core-compile

INFO] --- maven-dependency-plugin:3.3.0-SNAPSHOT:analyze-only (check-dependencies) @ tstp ---
[WARNING] Non-test scoped test only dependencies found:
[WARNING]    com.fasterxml.jackson.core:jackson-core:jar:2.13.1:compile
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

jackson-core with test scope

mvn clean verify -P core-test

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] .../tj/src/main/java/tstp/MDEP753.java:[9,48] cannot access com.fasterxml.jackson.core.Versioned
  class file for com.fasterxml.jackson.core.Versioned not found
[INFO] 1 error

How to make build pass without exclusion artifacts from analysis?
Maybe I do something in wrong way ...

[jira-importer]

Slawomir Jaranowski commented

Elliotte Rusty Harold please try my steps ...

[jira-importer]

Thorsten Glaser commented

Elliotte Rusty Harold there was much discussion about this in https://issues.apache.org/jira/browse/MDEP-753

The problem here is:

•  jackson-databind does declare a dependency on jackson-core
• src/main/ directly uses classes from jackson-databind, but none from jackson-core
  • these classes derive from classes in jackson-core
• src/test/ directly uses classes from jackson-core

With direct dependencies on jackson-databind and jackson-core, MDEP says test-only dependency jackson-core (because src/main/ does not use it). But there’s no fix that can be used with the current model because:

• adding <scope>test</scope> to jackson-core removes it from the transitive compile dependencies, making compile fail
• removing jackson-core from direct dependencies (rightfully) gives an MDEP warning because src/test/ directly uses it

[jira-importer]

Elliotte Rusty Harold commented

Thanks for the explanation. Based on that I agree there's a bug, but I do not yet agree that it is in the maven-dependency-plugin. The bug is that "adding <scope>test</scope> to jackson-core removes it from the transitive compile dependencies, making compile fail".

This should not happen. Whatever part of Maven is doing this (the compiler plugin?) has a serious issue, and that is what we need to fix. Saying that the top-level project depends on an artifact with test scope does not imply anything about what transitive dependencies it may depend on in what scopes.

[jira-importer]

Thorsten Glaser commented

I don’t really disagree, but my understanding of the current Maven model means I cannot fully agree either.

You say “Saying that the top-level project depends on an artifact with test scope”, but this is not what we say (at least not in {}<dependencyManagement>{}). Rather, we say “the artefact FOO is available in the following scopes: test”.

I actually rely on the ability to override things from external dependencies quite a lot. Hmm, perhaps <exclude> would suffice… but, hm, no. I can mentally construct something where I would want an implicit (or explicit) dependency available at test time but not at compile or runtime; for example, consider you use some kind of framework that normally pulls in its own logging, and that’s okay to test my (jar) module, but otherwise (at least at runtime) I provide the necessary things on my own (or don’t even need to because it uses reflection so just removing the default dependency will change the features but still work).

Perhaps we need a way to specify scopes as either “add to” or “set to”. (A more flexible scope specification would also make it possible, for example, to set something to compile+test but not runtime, or the other way around. Currently we’re limited to a set of choices that set the module-in-scope bitmask to certain values, which is less flexible.)

Additionally, I’d argue (and I’m sure people like chonton would do that even more vegemently) that changing anything in the way Maven resolution works is something for a 4.x and you should try to work within the 3.x model in updates of this plugin, especially considering the last working version of MDEP does not work with contemporary supported JDK versions and so people direly need an updated working version.

[jira-importer]

Stanislav Spiridonov commented

I agree with Elliotte Rusty Harold - simple use case - my project uses the slf4j only in test classes, so I add it to the test scope and... Maven will remove it from compile scope? Even if other dependencies are using it?

[jira-importer]

Slawomir Jaranowski commented

I see that we start discuss how dependency algorithm should / can work in the future, next major version of Maven ...

But we should agree that this issue should be resolved to proper work with Maven 3.x.

So I'm looking for solution, one I think is exclude artifacts from "non-test scoped" list if it is present on transitive dependency for another used artifact in compile scope.

[jira-importer]

Elliotte Rusty Harold commented

I am frankly astonished that Maven 3.x works like this, and frankly so surprised that I'm going to have to verify that for myself before I really believe it. I will be even more astonished if this proves to be a deliberate, documented choice of how the Maven resolution should work. Unless that can be demonstrated (and I really hope it can't) then I think this is a serious bug that needs to be fixed in Maven 3.x, not something we should live with. As a much less important side issue, I do not yet see any bug in the dependency plugin and analyzer and don't think we should spend time here to bless this broken behavior.

It is of course possible that there's more information here I don't have yet, and that I'm misunderstanding the real concern. Maybe there's something especially weird about Jackson that trips over issues most projects don't have, perhaps involving reflection. That would be a much less surprising fact to discover. It certainly wouldn't be the first time Jackson's gone off the deep end.

However I am not willing to accept that the description given here of the Maven resolution algorithm makes sense or was intentional without a lot stronger evidence.

[jira-importer]

Slawomir Jaranowski commented

Dependency tree for test project:

mvn dependency:tree -Dverbose -P core-test

[INFO] --- maven-dependency-plugin:3.3.0-SNAPSHOT:tree (default-cli) @ tstp ---
[INFO] mdep753:tstp:jar:0.1-SNAPSHOT
[INFO] +- com.fasterxml.jackson.core:jackson-databind:jar:2.13.1:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.13.1:compile
[INFO] |  \- (com.fasterxml.jackson.core:jackson-core:jar:2.13.1:compile - omitted for duplicate)
[INFO] +- org.junit.jupiter:junit-jupiter-api:jar:5.8.2:test
[INFO] |  +- org.opentest4j:opentest4j:jar:1.2.0:test
[INFO] |  +- org.junit.platform:junit-platform-commons:jar:1.8.2:test
[INFO] |  |  \- (org.apiguardian:apiguardian-api:jar:1.1.2:test - omitted for duplicate)
[INFO] |  \- org.apiguardian:apiguardian-api:jar:1.1.2:test
[INFO] \- com.fasterxml.jackson.core:jackson-core:jar:2.13.1:test (scope not updated to compile)

is it also wrong?

[jira-importer]

Slawomir Jaranowski commented

Elliotte Rusty Harold

I think this is a serious bug that needs to be fixed in Maven 3.x, not something we should live with.

so ... Is there any reported issue for Maven about it?

It is not only jackson problem, similar was shown in MDEP-753 with spring.

I also tested 3.3.0-SNAPSHOT with a big project in my work and with the same result.

[jira-importer]

Tamas Cservenak commented

Maven always makes "closer dep declaration win" (closer to project being built), and "explicit dep declaration overrides implicit" (so if you explicitly declare something in POM it affects transitive deps).

In short, you have a project P that depends on D1 compile time (as P extends/uses classes from D1), where in turn D1 depends on D2 (as D1 extends/uses classes from D2). Hence, right thing to do is:

In P declare direct compile dependency on D1.

What wrong is, that m-dep-p reports this as a problem (it clearly ignores the fact that by P extending classes from D1 it implicitly "uses" classes from D2 as well, otherwise compilation of P would fail, would be impossible.

All the other changes are wrong:

If P declare direct test dependency on D2 , this breaks compilation of P, as Java compiler must have access to classes in D2 that classes in D1 extends. Also, as P -> D2 is closer to P than indirect dependency P -> D1 -> D2, former wins.

IF P declare direct compile dependency on D2, while formally not incorrect, is superfluous IMHO.

 

In short, IMHO direct dependency on D2 should NOT be directly declared (in any scope), and m-dep-p bug should be fixed in regarding of this false report. It clearly considers only "first level" deps, but is unaware the fact that to compile P you must have not only D1 on compile classpath but also D2. The fact D2 classes are referenced directly in tests only (this one detects m-dep-p) should be overridden by the fact that D2 is needed as compile dependency for D1 in main code.

[jira-importer]

Tamas Cservenak commented

Should be easy to reproduce (maybe create IT):

D2: contains class D2Class

D1: contains class D1Class extends D2Class

P: contains class PClass extends D1Class.

For this to compile, D1 and D2 should be present on classpath.

If P tests contains PTestClass extends D2Class you have similar setup as OP with jackson.

[jira-importer]

Stanislav Spiridonov commented

Tamas Cservenak - "All the other changes are wrong" - is not correct, at least the first point.

Takes your example but imagines that D1 does not depend on D2 at the moment when you start the P. So P must declare the D2 as a test dependency. After that author of D1 make a fix and D1 now depends on D2. And it breaks your project P.

Another reason why P has to declare direct test dependency on D2 - you may decide to replace the D1 dependency with C1 (make the same things as D1 but has another implementation). And again without test dependency on D2, the project P will be broken.

I am pretty sure that any changes in the project dependencies, that do not break the contact/interface shouldn't influence your project setup.

[jira-importer]

Tamas Cservenak commented

Sorry, am talking about release dependencies used in reproducible way (no ranges), as then

• "D1 does not depend on D2 at the moment" – Fine, releases are immutable.
• "after that D1 make a fix and D1 now depends on D2" – I believe this D1 and D1 above have different versions then? How does it affects P? This will become issue once P upgrades to new D1 version, but the P can also adapt and remove D2 from test scope.
• "replace D1 with C1" – this is again something I don't understand: we are here in this situation exactly as D1 declares dependency on D2, How come now C1 come into picture, along with the fact that you still need D2? Also, if C1 depens on C2, why would your test be written against D2 and not C2? Finally, this again means P can adapt (as it changes dep D1 to C1), and return the D2 into test scope, no?

Could you maybe point me at real life examples?