You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/05/20 15:43:12 UTC
[GitHub] [maven-dependency-plugin] chadwick00 opened a new pull request, #218: Feature/mdep 808 restrict dependency analysis by group
chadwick00 opened a new pull request, #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218
Full write up of the use case on MDEP-808. Short version is that I've added a new {{includeDependencies}} parameter to allow the scope of the warnings from the analyze goal to be limited to the include dependencies.
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MDEP) 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.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] 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.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] 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](https://www.apache.org/licenses/icla.pdf) 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](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [x] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1342916650
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
+ *
+ * The filter syntax is:
+ *
+ * <pre>
+ * [groupId]:[artifactId]:[type]:[version]
+ * </pre>
+ *
+ * where each pattern segment is optional and supports full and partial <code>*</code> wildcards. An empty pattern
+ * segment is treated as an implicit wildcard. *
+ * <p>
+ * For example, <code>org.apache.*</code> will match all artifacts whose group id starts with
+ * <code>org.apache.</code>, and <code>:maven-dependency-plugin</code> will result in the analysis being applied
Review Comment:
Done by copying the example Javadoc above. Decided the purpose of this section of the Javadoc is to explain the Maven co-ordinate and wildcard combination rather than the specific behaviour of the parameter.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] chadwick00 commented on a diff in pull request #218: [MDEP-808] restrict dependency analysis by group
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1331584239
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -215,7 +216,7 @@
* @since 2.10
*/
@Parameter
- private String[] ignoredUsedUndeclaredDependencies = new String[0];
+ private final String[] ignoredUsedUndeclaredDependencies = new String[0];
Review Comment:
Although looking back through the code again, I probably did this to remind myself that these aren't mutated by the code. Presumably only a mad-man would write code that would change the contents of a Mojo parameter, even if the compiler allowed them to.
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -215,7 +216,7 @@
* @since 2.10
*/
@Parameter
- private String[] ignoredUsedUndeclaredDependencies = new String[0];
+ private final String[] ignoredUsedUndeclaredDependencies = new String[0];
Review Comment:
Although looking back through the code again, I probably did this to remind myself that these aren't mutated by the code. Presumably only a mad-man would write code that would change the contents of a Mojo parameter, even if the compiler allowed them to?
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1342659340
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -638,4 +720,35 @@ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] exc
return result;
}
+
+
+ /**
+ * Filter for artifacts that match the <code>include</code> criteria.
+ * No filtering is applied if the criteria is empty.
+ *
+ * @param artifacts filtered for elements that do match the criteria
+ * @param includes the filter to be applied
+ * @return the list of artifacts that didn't match the criteria
+ */
+ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] includes )
+ {
+ List<Artifact> result = new ArrayList<>();
+
+ if ( includes.length > 0 )
+ {
+ ArtifactFilter filter = new StrictPatternIncludesArtifactFilter( Arrays.asList( includes ) );
+
+ for ( Iterator<Artifact> it = artifacts.iterator(); it.hasNext(); )
+ {
+ Artifact artifact = it.next();
+ if ( !filter.include( artifact ) )
+ {
+ it.remove();
Review Comment:
The original `filterDependencies` method both mutates the `artifacts` argument and indicates what is has removed in the `result`. I didn't want to change the stateful nature of this methods because that would be a pretty serious overhaul to the code. So the remove call is needed to remove `artifacts` that don't match the include argument.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] elharo commented on a diff in pull request #218: [MDEP-808] restrict dependency analysis by group
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1331499957
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
+ *
+ * The filter syntax is:
+ *
+ * <pre>
+ * [groupId]:[artifactId]:[type]:[version]
+ * </pre>
+ *
+ * where each pattern segment is optional and supports full and partial <code>*</code> wildcards. An empty pattern
+ * segment is treated as an implicit wildcard. *
+ * <p>
+ * For example, <code>org.apache.*</code> will match all artifacts whose group id starts with
+ * <code>org.apache.</code>, and <code>:maven-dependency-plugin</code> will result in the analysis being applied
Review Comment:
This is confusing. Try using an example of an unrelated library instead like JodaTime.
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -621,7 +695,15 @@ private void writeScriptableOutput( Set<Artifact> artifacts )
}
}
- private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] excludes )
+
+ /**
+ * Filter for artifacts that don't match the <code>exclude</code> criteria.
+ *
+ * @param artifacts filtered by elements that don't match the <code>excludes</code> argument
Review Comment:
exclude or excludes? You have both.
##########
src/it/projects/analyze-include-dependency/pom.xml:
##########
@@ -0,0 +1,81 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.its.dependency</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0-SNAPSHOT</version>
+
+ <name>Test</name>
+ <description>
+ Test dependency:analyze with includeDependencies
+ </description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-project</artifactId>
Review Comment:
why so old?
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -438,6 +478,40 @@ private boolean checkDependencies()
warning = true;
}
+ // log dependencies that weren't included
+ if ( verbose && !notIncludedUsedDeclared.isEmpty() )
Review Comment:
why are these logged? I thought the point of this PR was to avoid reporting on these.
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -638,4 +720,35 @@ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] exc
return result;
}
+
+
+ /**
+ * Filter for artifacts that match the <code>include</code> criteria.
+ * No filtering is applied if the criteria is empty.
+ *
+ * @param artifacts filtered for elements that do match the criteria
+ * @param includes the filter to be applied
+ * @return the list of artifacts that didn't match the criteria
+ */
+ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] includes )
+ {
+ List<Artifact> result = new ArrayList<>();
+
+ if ( includes.length > 0 )
+ {
+ ArtifactFilter filter = new StrictPatternIncludesArtifactFilter( Arrays.asList( includes ) );
+
+ for ( Iterator<Artifact> it = artifacts.iterator(); it.hasNext(); )
+ {
+ Artifact artifact = it.next();
+ if ( !filter.include( artifact ) )
+ {
+ it.remove();
Review Comment:
why the remove call? Doesn't seem needed.
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
Review Comment:
This isn't clear that everything is included by default when this isn't set.
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -638,4 +720,35 @@ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] exc
return result;
}
+
+
+ /**
+ * Filter for artifacts that match the <code>include</code> criteria.
+ * No filtering is applied if the criteria is empty.
+ *
+ * @param artifacts filtered for elements that do match the criteria
+ * @param includes the filter to be applied
+ * @return the list of artifacts that didn't match the criteria
+ */
+ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] includes )
+ {
+ List<Artifact> result = new ArrayList<>();
+
+ if ( includes.length > 0 )
Review Comment:
!isEmpty()
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
+ *
+ * The filter syntax is:
+ *
+ * <pre>
+ * [groupId]:[artifactId]:[type]:[version]
+ * </pre>
+ *
+ * where each pattern segment is optional and supports full and partial <code>*</code> wildcards. An empty pattern
+ * segment is treated as an implicit wildcard. *
+ * <p>
+ * For example, <code>org.apache.*</code> will match all artifacts whose group id starts with
Review Comment:
will match --> matches (tech writing lives in the eternal present)
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1343212367
##########
src/it/projects/analyze-include-dependency/pom.xml:
##########
@@ -0,0 +1,81 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.its.dependency</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0-SNAPSHOT</version>
+
+ <name>Test</name>
+ <description>
+ Test dependency:analyze with includeDependencies
+ </description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-project</artifactId>
Review Comment:
I've tried to update the version of the all the dependencies @mavenVersion@, but at least one of the dependencies doesn't like it. I can try harder by choosing alternative dependencies, but I suppose the benefit of keeping these at an old version is that the referenced classes within dependency won't deprecated, as [has been done elsewhere](https://github.com/apache/maven-dependency-plugin/blob/master/src/it/projects/analyze-ignore-unused-declared-dependency/pom.xml#L43).
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1343216254
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
Review Comment:
Decided that building sub-graphs was way to complicated, so sticking with the original changes, which is consistent with the standard Maven filters.... [include then exclude](https://github.com/apache/maven-common-artifact-filters/blob/9d692d49b35014a2d308c78894ee502bccd0361e/src/main/java/org/apache/maven/shared/artifact/filter/collection/AbstractArtifactFeatureFilter.java#L63).
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
Review Comment:
Decided that building sub-graphs was way too complicated, so sticking with the original changes, which is consistent with the standard Maven filters.... [include then exclude](https://github.com/apache/maven-common-artifact-filters/blob/9d692d49b35014a2d308c78894ee502bccd0361e/src/main/java/org/apache/maven/shared/artifact/filter/collection/AbstractArtifactFeatureFilter.java#L63).
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1345651208
##########
src/site/apt/examples/control-dependencies-under-dependency-analysis.apt.vm:
##########
@@ -57,6 +62,11 @@ Exclude dependencies from dependency analysis
</goals>
<configuration>
<failOnWarning>true</failOnWarning>
+
+ <!-- limit warnings to dependencies for group com.google.code.findbugs -->
+ <includeDependencies>
+ <includeDependency>com.google.code.findbugs</includeDependency>
Review Comment:
includeDependency --> groupId
Instead of a pattern here,= separate groupId and artifactId elements are simpler.
Maybe includeDependencies --> analyze or check. I'm not sure.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] chadwick00 commented on a diff in pull request #218: [MDEP-808] restrict dependency analysis by group
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1331603841
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
Review Comment:
Yes, I've been musing on this point. The default behaviour prior to this change is that everything is included. That can't change without requiring a pom changes downstream. So this is strictly:
"_an override to the ignore_"
That's a bit mind-bending if you don't have the context. I think the best we can do is to provide a recipe in the Javadoc describing an example use case, such as the one we're going to use it for. Broadly, everything _ignored_, but this one Maven co-ordinate _in_.
It doesn't feel like if we were designing this anew we'd start from here, but without a brain-wave on how to prevent a backward incompatible change, this will be the approach.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1342636364
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -638,4 +720,35 @@ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] exc
return result;
}
+
+
+ /**
+ * Filter for artifacts that match the <code>include</code> criteria.
+ * No filtering is applied if the criteria is empty.
+ *
+ * @param artifacts filtered for elements that do match the criteria
+ * @param includes the filter to be applied
+ * @return the list of artifacts that didn't match the criteria
+ */
+ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] includes )
+ {
+ List<Artifact> result = new ArrayList<>();
+
+ if ( includes.length > 0 )
Review Comment:
Not a collection. Happy to call Arrays.asList (includes) earlier if you prefer.
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -638,4 +720,35 @@ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] exc
return result;
}
+
+
+ /**
+ * Filter for artifacts that match the <code>include</code> criteria.
+ * No filtering is applied if the criteria is empty.
+ *
+ * @param artifacts filtered for elements that do match the criteria
+ * @param includes the filter to be applied
+ * @return the list of artifacts that didn't match the criteria
+ */
+ private List<Artifact> filterDependencies( Set<Artifact> artifacts, String[] includes )
+ {
+ List<Artifact> result = new ArrayList<>();
+
+ if ( includes.length > 0 )
Review Comment:
Not a Collection. Happy to call Arrays.asList (includes) earlier if you prefer.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] slawekjaranowski commented on a diff in pull request #218: Feature/mdep 808 restrict dependency analysis by group
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r956565980
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -215,7 +216,7 @@
* @since 2.10
*/
@Parameter
- private String[] ignoredUsedUndeclaredDependencies = new String[0];
+ private final String[] ignoredUsedUndeclaredDependencies = new String[0];
Review Comment:
Please don't use `final` for Mojo parameters. It can be inline by compiler ...
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1342895950
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -438,6 +478,40 @@ private boolean checkDependencies()
warning = true;
}
+ // log dependencies that weren't included
+ if ( verbose && !notIncludedUsedDeclared.isEmpty() )
Review Comment:
These are logged in the verbose section only. Mirroring the [existing verbose section](https://github.com/apache/maven-dependency-plugin/blob/master/src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java#L403) for the explicitly ignored dependencies, I think you'd also want to understand what was ignored implicitly by not being covered within the includes.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] chadwick00 commented on a diff in pull request #218: [MDEP-808] restrict dependency analysis by group
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1331533415
##########
src/it/projects/analyze-include-dependency/pom.xml:
##########
@@ -0,0 +1,81 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.its.dependency</groupId>
+ <artifactId>test</artifactId>
+ <version>1.0-SNAPSHOT</version>
+
+ <name>Test</name>
+ <description>
+ Test dependency:analyze with includeDependencies
+ </description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-project</artifactId>
Review Comment:
I think it was just that I wanted to valid project and didn't particularly care about the versions within the project. I can search for a substitution variable to keep in current if having this always lag behind is an eye-sore come review time.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1343216254
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
Review Comment:
Decided that building sub-graphs was way too complicated, so sticking with the original approach and updating the Javadoc as requested, which is consistent with the standard Maven filters.... [include then exclude](https://github.com/apache/maven-common-artifact-filters/blob/9d692d49b35014a2d308c78894ee502bccd0361e/src/main/java/org/apache/maven/shared/artifact/filter/collection/AbstractArtifactFeatureFilter.java#L63).
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1342895950
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -438,6 +478,40 @@ private boolean checkDependencies()
warning = true;
}
+ // log dependencies that weren't included
+ if ( verbose && !notIncludedUsedDeclared.isEmpty() )
Review Comment:
These are logged in the verbose section only. In the same way that the [existing verbose section](https://github.com/apache/maven-dependency-plugin/blob/master/src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java#L403) for the explicitly ignored dependencies, I think you'd also want to understand what was been ignore implicitly by not being covered within the include block.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MDEP-808] restrict dependency analysis by group [maven-dependency-plugin]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1345651208
##########
src/site/apt/examples/control-dependencies-under-dependency-analysis.apt.vm:
##########
@@ -57,6 +62,11 @@ Exclude dependencies from dependency analysis
</goals>
<configuration>
<failOnWarning>true</failOnWarning>
+
+ <!-- limit warnings to dependencies for group com.google.code.findbugs -->
+ <includeDependencies>
+ <includeDependency>com.google.code.findbugs</includeDependency>
Review Comment:
includeDependency --> groupId
##########
src/site/apt/examples/control-dependencies-under-dependency-analysis.apt.vm:
##########
@@ -37,6 +37,11 @@ Exclude dependencies from dependency analysis
The plugin can then be configured to ignore dependencies that are
"declared but unused", "undeclared but used", and "non-test scoped"
in selected list or in all simultaneously.
+
+ Another case is when mature projects have accumulated superfluous dependencies or
+ relied upon transitivity for their dependencies. Where fixing all the dependency
+ problems up-front is not viable the plugin can be configured to only raise
Review Comment:
comma after viable
##########
src/site/apt/examples/control-dependencies-under-dependency-analysis.apt.vm:
##########
@@ -37,6 +37,11 @@ Exclude dependencies from dependency analysis
The plugin can then be configured to ignore dependencies that are
"declared but unused", "undeclared but used", and "non-test scoped"
in selected list or in all simultaneously.
+
+ Another case is when mature projects have accumulated superfluous dependencies or
+ relied upon transitivity for their dependencies. Where fixing all the dependency
+ problems up-front is not viable the plugin can be configured to only raise
+ warnings for dependencies meeting "include" criteria.
Review Comment:
This needs to be expanded. Based solely on this text — that is, without reading the code or the JIRA — I do not know which dependencies are and are not checked.
##########
src/site/apt/examples/control-dependencies-under-dependency-analysis.apt.vm:
##########
@@ -37,6 +37,11 @@ Exclude dependencies from dependency analysis
The plugin can then be configured to ignore dependencies that are
"declared but unused", "undeclared but used", and "non-test scoped"
in selected list or in all simultaneously.
+
+ Another case is when mature projects have accumulated superfluous dependencies or
+ relied upon transitivity for their dependencies. Where fixing all the dependency
+ problems up-front is not viable the plugin can be configured to only raise
+ warnings for dependencies meeting "include" criteria.
Review Comment:
This needs to be expanded. Based solely on this text — that is, without reading the code or the JIRA — I do not know which dependencies are and are not checked.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] chadwick00 commented on a diff in pull request #218: [MDEP-808] restrict dependency analysis by group
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1331645202
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -266,7 +267,30 @@ public abstract class AbstractAnalyzeMojo
// defaultValue value on @Parameter - not work with Maven 3.2.5
// When is set defaultValue always win, and there is no possibility to override by plugin configuration.
@Parameter
- private List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+ private final List<String> ignoredPackagings = Arrays.asList( "pom", "ear" );
+
+
+ /**
+ * List of dependencies to be included. The result of this list is then applied to the ignore parameters.
Review Comment:
Thinking through some of the use-cases further and wanting more fine-grained control, we're essentially building sub-graphs in the dependency graph. I think that is expressible using Maven co-ordinates, but would be good to back these thoughts with some tests.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-dependency-plugin] chadwick00 commented on a diff in pull request #218: [MDEP-808] restrict dependency analysis by group
Posted by "chadwick00 (via GitHub)" <gi...@apache.org>.
chadwick00 commented on code in PR #218:
URL: https://github.com/apache/maven-dependency-plugin/pull/218#discussion_r1331539792
##########
src/main/java/org/apache/maven/plugins/dependency/analyze/AbstractAnalyzeMojo.java:
##########
@@ -215,7 +216,7 @@
* @since 2.10
*/
@Parameter
- private String[] ignoredUsedUndeclaredDependencies = new String[0];
+ private final String[] ignoredUsedUndeclaredDependencies = new String[0];
Review Comment:
Done.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org