nifi icon indicating copy to clipboard operation
nifi copied to clipboard

NIFI-10588 Add a new goal that checks dependency duplications in nars

Open SaumyaGurtu opened this issue 3 years ago • 8 comments

Summary

NIFI-10588

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • [ ] Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • [ ] Pull Request based on current revision of the main branch
  • [ ] Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation. locally tested with mvn verify

Build

  • [ ] Build completed using mvn clean install -P contrib-check
    • [ ] JDK 8
    • [ ] JDK 11
    • [ ] JDK 17

Licensing

  • [ ] New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • [ ] New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • [ ] Documentation formatting appears as expected in rendered files

SaumyaGurtu avatar Sep 14 '22 11:09 SaumyaGurtu

@MikeThomsen and @SaumyaGurtu I'm not sure this PR accomplishes the goal described in NIFI-10457. Although the plugin configuration checks for duplicate dependencies, it does not highlight when certain dependencies should be marked as provided. Therefore, I'm not sure this should be merged.

exceptionfactory avatar Sep 15 '22 16:09 exceptionfactory

@MikeThomsen The proposed solution has passed the checks and highlights the duplicate dependencies in the project if any as asked in the ticket. This will increase the discipline in the repository. Please let me know if you have any change in plans.

SaumyaGurtu avatar Sep 19 '22 06:09 SaumyaGurtu

@SaumyaGurtu Although this pull request looks like a useful improvement, it does not accomplish the goals described in the associated Jira issue. The goal described in the Jira issue is specifically related to NAR bundles. Although a NAR bundle may not contain direct duplication of dependencies, the hierarchical nature of NAR dependencies means that a child NAR could have the same library as a parent NAR, making the extra dependency unnecessary in the child NAR. This kind of relationship is not necessarily detected through the maven-enforcer-plugin because NAR bundles are specific to Apache NiFi. For this reason, an alternative approach will be necessary to accomplish the goal described in the Jira issue.

exceptionfactory avatar Sep 30 '22 16:09 exceptionfactory

@SaumyaGurtu, although this PR does not align with the goals of NIFI-10457, it is still a useful improvement and it does appear to implement the features described in NIFI-10588. I updated the title and link to relate to that Jira issue.

With that change, this can be reviewed with the purpose of banning duplicate dependencies. This needs to be verified in a local build to ensure that it works as designed.

exceptionfactory avatar Oct 07 '22 16:10 exceptionfactory

@exceptionfactory I think you probably meant NIFI-10457 not NIFI-10467

dan-s1 avatar Oct 07 '22 16:10 dan-s1

@exceptionfactory I think you probably meant NIFI-10457 not NIFI-10467

Thanks for the correction @dan-s1!

exceptionfactory avatar Oct 07 '22 16:10 exceptionfactory

@exceptionfactory I am not sure how this differs from what I tried for NIFI-10565 but could not get working with the Maven Enforcer plugin. If this really worked it should have caught the issues I corrected in NIFI-10565.

dan-s1 avatar Oct 07 '22 16:10 dan-s1

@dan-s1 Yes, it sounds similar. This may only check for duplicates declared in the pom.xml itself, without reference to dependencies inherited from parent definitions. With your previous testing, that would be a key point to evaluate, and I plan to take a closer look soon. This warrants some reading of the Maven Enforcer Plugin documentation.

exceptionfactory avatar Oct 07 '22 16:10 exceptionfactory

@SaumyaGurtu, are you able to make the adjustments suggested to move the banDuplicatePomDependencyVersions under the current Maven Enforcer configuration?

exceptionfactory avatar Oct 20 '22 21:10 exceptionfactory

@SaumyaGurtu Based on the work you already did, I updated the branch to move the ban rule under the current set of enforcer rules. I also rebased the PR so that we can get a current status of the build with this rule enabled.

exceptionfactory avatar Oct 25 '22 15:10 exceptionfactory