You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/09/14 11:44:18 UTC
[GitHub] [nifi] SaumyaGurtu opened a new pull request, #6418: NIFI-10457: Add a new goal that checks dependency duplications in nars
SaumyaGurtu opened a new pull request, #6418:
URL: https://github.com/apache/nifi/pull/6418
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-10457](https://issues.apache.org/jira/browse/NIFI-10457)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### 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
### 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](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] SaumyaGurtu commented on pull request #6418: NIFI-10457: Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
SaumyaGurtu commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1250623247
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1290716055
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10457: Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1248350659
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1271807996
> @exceptionfactory I think you probably meant NIFI-10457 not NIFI-10467
Thanks for the correction @dan-s1!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1271805223
@exceptionfactory I think you probably meant NIFI-10457 not NIFI-10467
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10457: Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1263793254
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory closed pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
URL: https://github.com/apache/nifi/pull/6418
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1271818520
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1271792979
@SaumyaGurtu, although this PR does not align with the goals of NIFI-10467, 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1286162643
@SaumyaGurtu, are you able to make the adjustments suggested to move the `banDuplicatePomDependencyVersions` under the current Maven Enforcer configuration?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] MikeThomsen commented on a diff in pull request #6418: NIFI-10457: Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on code in PR #6418:
URL: https://github.com/apache/nifi/pull/6418#discussion_r972208509
##########
pom.xml:
##########
@@ -823,6 +823,17 @@
<artifactId>maven-enforcer-plugin</artifactId>
<version>3.1.0</version>
<executions>
+ <execution>
+ <id>no-duplicate-declared-dependencies</id>
Review Comment:
Makes me wonder if we shouldn't make plans to ban dependency convergence to encourage more discipline on dependency management.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6418:
URL: https://github.com/apache/nifi/pull/6418#discussion_r991459627
##########
pom.xml:
##########
@@ -823,6 +823,17 @@
<artifactId>maven-enforcer-plugin</artifactId>
<version>3.1.0</version>
<executions>
+ <execution>
+ <id>no-duplicate-declared-dependencies</id>
+ <goals>
+ <goal>enforce</goal>
+ </goals>
+ <configuration>
+ <rules>
+ <banDuplicatePomDependencyVersions/>
Review Comment:
Instead of adding a new `execution` block, the `banDuplicatePomDependencyVersions` rule should be added to the existing `execution` section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6418: NIFI-10588 Add a new goal that checks dependency duplications in nars
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6418:
URL: https://github.com/apache/nifi/pull/6418#issuecomment-1271813112
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org