Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for transitive dependency with local test scope #70

Closed
wants to merge 1 commit into from

Conversation

electrum
Copy link
Contributor

@electrum electrum commented Feb 4, 2024

This example project has a direct dependencies on Guice in compile scope and Guava in test scope. Guice depends on Guava, so the runtime classpath should include Guava, as it is required at runtime.

Note that this test currently fails on all versions of Maven, even though ProvisioningMojo requests compile+runtime.

This example project has a direct dependencies on Guice in compile
scope and Guava in test scope. Guice depends on Guava, so the runtime
classpath should include Guava, as it is required at runtime.
@electrum
Copy link
Contributor Author

electrum commented Feb 4, 2024

cc @cstamas

@cstamas
Copy link
Collaborator

cstamas commented Feb 4, 2024

Will take a peek on EU morning

@cstamas
Copy link
Collaborator

cstamas commented Feb 5, 2024

Seems we finally found a bug, a bug in Maven Core bug old 14 years 😄
https://issues.apache.org/jira/browse/MNG-8041

This whole topic got "inflamed" as I documented "how should things work" (simple to prove with MIMA or even with Maven + circumvention):
https://maven.apache.org/resolver-archives/resolver-2.0.0-alpha-7/common-misconceptions.html

But Maven violates this, and this bug leads to issues like:
https://issues.apache.org/jira/browse/MRESOLVER-391
https://issues.apache.org/jira/browse/MASSEMBLY-1008
etc

@cstamas
Copy link
Collaborator

cstamas commented Feb 5, 2024

This Maven PR apache/maven#1398 makes this reproducer work as expected (guava ends up in lib). And even more, all the Maven ITs did pass, which also explains how could this edge case lurk in there.

cstamas added a commit to cstamas/provisio that referenced this pull request Feb 6, 2024
Similar fix to m-assembly-p: do not rely on Maven Core to provide
"runtime" resolution scope as it have issues, see:
https://issues.apache.org/jira/browse/MNG-8041

Instead, use Resolver APIs directly to get what is needed.
Contains several minor code improvements across Provisio
maven-plugin module.

Contains IT from jvanzyl#70
Fixes jvanzyl#71
cstamas added a commit to cstamas/provisio that referenced this pull request Feb 9, 2024
Similar fix to m-assembly-p: do not rely on Maven Core to provide
"runtime" resolution scope as it have issues, see:
https://issues.apache.org/jira/browse/MNG-8041

Instead, use Resolver APIs directly to get what is needed.
Contains several minor code improvements across Provisio
maven-plugin module.

Other changes in this PR:
* added support for -X to dump proviso graph
* marked Mojos as thread safe, removed now unneeded ask for resolution
  (as they use Resolver APIs)

Contains IT from jvanzyl#70
Fixes jvanzyl#71
wendigo pushed a commit that referenced this pull request Feb 9, 2024
Similar fix to m-assembly-p: do not rely on Maven Core to provide
"runtime" resolution scope as it have issues, see:
https://issues.apache.org/jira/browse/MNG-8041

Instead, use Resolver APIs directly to get what is needed.
Contains several minor code improvements across Provisio
maven-plugin module.

Other changes in this PR:
* added support for -X to dump proviso graph
* marked Mojos as thread safe, removed now unneeded ask for resolution
  (as they use Resolver APIs)

Contains IT from #70
Fixes #71
@wendigo
Copy link
Collaborator

wendigo commented Feb 9, 2024

Closing. Part of the fix: 1f17f55

@wendigo wendigo closed this Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants