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

[MDEP-317] - add mojo to analyze invalid exclusions #362

Merged
merged 9 commits into from Apr 14, 2024

Conversation

vbreivik
Copy link
Contributor

@vbreivik vbreivik commented Jan 1, 2024

This mojo reports if exclusions are defined on a dependency, but that dependency does not pull in said artifacts.

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MDEP-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace MDEP-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its clean verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

Logs out a warning, alternatively fail when failOnError flag is set, when a dependency defines an exclusion that is not valid.

Some notes
The failOnWarning property is the same property as for analyze, keep it that way or make its own property?

I placed the logic in its own package and made it its own execution, it can be moved to be a part of analyze if wanted. Having it as its own will make upgrading not change current behavior.

The exclusion glob pattern logic is copied from ExclusionArtifactFilter added in maven-core in jira MNG-7843.

@vbreivik vbreivik force-pushed the MDEP-317/invalid_exclude branch 2 times, most recently from 8d2de4c to 6410931 Compare January 14, 2024 20:59
This mojo reports if exclusions are defined on a dependency, but that dependency does not pull in said artifacts.
@vbreivik vbreivik marked this pull request as ready for review March 3, 2024 11:54
@slawekjaranowski
Copy link
Member

@vbreivik thanks for idea and PR ... I will try to review in a few days

@slawekjaranowski slawekjaranowski self-assigned this Mar 21, 2024
@vbreivik
Copy link
Contributor Author

@vbreivik thanks for idea and PR ... I will try to review in a few days

I cannot take credit for the idea. I just saw it in Jira and was a bit bored. :)

Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first some of simple comments after reading a change

Bumped dependency versions used in invoker test
Renamed test from snake_case to camelCase
Removed usage of Guava
@vbreivik
Copy link
Contributor Author

@slawekjaranowski I have updated the test that started failing after c9e488b, can you trigger the build again?

 - remove fork additional executions
 - remove clean from test execution
@slawekjaranowski
Copy link
Member

I think about extend test with scenario, for multimodule project:

root pom - dependencyManagement with exclusion

  • child module with using dependency

Add testcase for multimodule project with invalid dependency managed exclusion where one of the child modules uses this dependency

Add project name in output when violations are found.
@vbreivik
Copy link
Contributor Author

I think about extend test with scenario, for multimodule project:

root pom - dependencyManagement with exclusion

  • child module with using dependency

I added a test case with this scenario. I ended up putting the module name in the warning to make the test more clear.

Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, lgtm

@slawekjaranowski slawekjaranowski merged commit 4907f4a into apache:master Apr 14, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants