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 2020/07/31 17:30:37 UTC

[GitHub] [maven-dependency-plugin] ian-lavallee opened a new pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

ian-lavallee opened a new pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92


   Reintroduces the verbose option for dependency:tree. Still need to add or fix the following issues:
   - Only supports the default txt output no graphml, tgf, or dot
   - Does not appropriately alert the user to cycles but does deal with them
   - Not sure when to display (scope not updated to compile) type messages
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#issuecomment-672992846


   ci: https://builds.apache.org/job/maven-box/job/maven-dependency-plugin/job/MDEP-644/


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] ian-lavallee commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
ian-lavallee commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r466690592



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/OsProperties.java
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+/**
+ * Platform-dependent project properties normalized from ${os.name} and ${os.arch}. Netty uses these to select
+ * system-specific dependencies through the
+ * <a href='https://github.com/trustin/os-maven-plugin'>os-maven-plugin</a>.
+ */
+public class OsProperties

Review comment:
       Yeah it's used in a static initialization block in VerboseDependencyGraphBuilder, I'll change it to package-private




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r467868212



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,332 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Repository;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.DependencyRequest;
+import org.eclipse.aether.resolution.DependencyResult;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.maven.plugins.dependency.tree.verbose.RepositoryUtility.mavenRepositoryFromUrl;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+public class VerboseDependencyGraphBuilder

Review comment:
       Yes, I think we should drop the verbose package. It's better to keep things non-public where you can. 

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/TreeMojo.java
##########
@@ -309,6 +352,64 @@ public void setSkip( boolean skip )
 
     // private methods --------------------------------------------------------
 
+
+    private DependencyNode convertToCustomDependencyNode( org.eclipse.aether.graph.DependencyNode node )
+    {
+        DefaultDependencyNode rootNode = new DefaultDependencyNode( null,
+                convertAetherArtifactToMavenArtifact( node ), null, null, null );
+
+        rootNode.setChildren( new ArrayList<DependencyNode>() );
+
+        for ( org.eclipse.aether.graph.DependencyNode child : node.getChildren() )
+        {
+            rootNode.getChildren().add( buildTreeDfs( rootNode, child ) );
+        }
+
+        return rootNode;
+    }
+
+    private DependencyNode buildTreeDfs( DependencyNode parent, org.eclipse.aether.graph.DependencyNode child )

Review comment:
       buildTreeDepthFirst? why does it matter whether we build tree depth first or breadth first if it's the same tree?

##########
File path: pom.xml
##########
@@ -456,7 +446,7 @@ under the License.
                 <mockRepo>
                   <source>src/it/mrm/repository</source>
                 </mockRepo>
-                <!-- 
+                <!--

Review comment:
       why not just delete the commented element?

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/TreeMojo.java
##########
@@ -267,6 +306,10 @@ public void execute()
         {
             throw new MojoExecutionException( "Cannot serialise project dependency graph", exception );
         }
+        catch ( DependencyResolutionException e )
+        {
+            e.printStackTrace();

Review comment:
       stack trace might not be very helpful since this is an external issue, not a code issue; is there a logger we can report to instead? 

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/CycleBreakerGraphTransformer.java
##########
@@ -0,0 +1,88 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.collection.DependencyGraphTransformationContext;
+import org.eclipse.aether.collection.DependencyGraphTransformer;
+import org.eclipse.aether.graph.DependencyNode;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Transforms a dependency graph so that it does not contain cycles.
+ *
+ * <p>A cycle in a dependency graph is a situation where a path to a node from the root contains the
+ * same node. For example, jaxen 1.1-beta-6 is known to have cycle with dom4j 1.6.1.
+ */
+public final class CycleBreakerGraphTransformer implements DependencyGraphTransformer
+{
+
+    private final Set<DependencyNode> visitedNodes = Collections.newSetFromMap(

Review comment:
       why are we starting with an empty IdentityHashmap map here? I would think a simple HashSet would suffice. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r466396456



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/OsProperties.java
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+/**
+ * Platform-dependent project properties normalized from ${os.name} and ${os.arch}. Netty uses these to select
+ * system-specific dependencies through the
+ * <a href='https://github.com/trustin/os-maven-plugin'>os-maven-plugin</a>.
+ */
+public class OsProperties

Review comment:
       does this need to be public? For that matter, is it used at all? I don't see where it's referenced. 

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/OsProperties.java
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+

Review comment:
       remove extra blank lines




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] ian-lavallee commented on pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
ian-lavallee commented on pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#issuecomment-667244970


   @elharo you can see the licenses rat plugin rejects in CycleBreaker and MavenRepositoryException. I need to add the serializer unit tests and remove the dependency analyzer test from this PR before it's ready for a full review. This is just created for the license issue at this point.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] ian-lavallee commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
ian-lavallee commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r466696381



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,216 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+
+/**
+ * Aether initialization. This uses Maven Resolver 1.4.2 or later. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, but this is the current one.
+ */
+public final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",

Review comment:
       I agree with this. I spent some time using the injected session and repositorySystemSession but couldn't get the output to be correct. I think we can merge this in still because it will still work for most users and I will work on this issue and the other missing features.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] ian-lavallee commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
ian-lavallee commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r467980668



##########
File path: pom.xml
##########
@@ -456,7 +446,7 @@ under the License.
                 <mockRepo>
                   <source>src/it/mrm/repository</source>
                 </mockRepo>
-                <!-- 
+                <!--

Review comment:
       The comment is in the master branch so I just left it in in case there was a reason for why someone else added it.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#issuecomment-671350089


   tests passed: https://builds.apache.org/job/maven-box/job/maven-dependency-plugin/job/MDEP-644/


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] pzygielo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
pzygielo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r466403074



##########
File path: src/test/java/org/apache/maven/plugins/dependency/TestListClassesMojo.java
##########
@@ -58,6 +59,11 @@ protected void setUp()
         setVariableValueToObject( mojo, "session", legacySupport.getSession() );
     }
 
+    public void test()

Review comment:
       :eyes: :confused: 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] bimargulies-google commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
bimargulies-google commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r468868407



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,223 @@
+package org.apache.maven.plugins.dependency.tree;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.project.DefaultDependencyResolutionRequest;
+import org.apache.maven.project.DependencyResolutionException;
+import org.apache.maven.project.DependencyResolutionRequest;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.project.ProjectDependenciesResolver;
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+class VerboseDependencyGraphBuilder
+{
+    private static final String PRE_MANAGED_SCOPE = "preManagedScope", PRE_MANAGED_VERSION = "preManagedVersion",
+            MANAGED_SCOPE = "managedScope";
+
+    public DependencyNode buildVerboseGraph( MavenProject project, ProjectDependenciesResolver resolver,
+                                                         RepositorySystemSession repositorySystemSession )
+            throws DependencyResolutionException
+    {
+        DefaultRepositorySystemSession session = MavenRepositorySystemUtils.newSession();
+        session.setLocalRepositoryManager( repositorySystemSession.getLocalRepositoryManager() );
+
+        DependencySelector dependencySelector = new AndDependencySelector(
+                // ScopeDependencySelector takes exclusions. 'Provided' scope is not here to avoid
+                // false positive in LinkageChecker.
+                new ScopeDependencySelector(),
+                new ExclusionDependencySelector() );
+
+        session.setDependencySelector( dependencySelector );
+        session.setDependencyGraphTransformer( new ChainedDependencyGraphTransformer(
+                new CycleBreakerGraphTransformer(), // Avoids StackOverflowError
+                new JavaDependencyContextRefiner() ) );
+        session.setDependencyManager( null );
+
+        DependencyResolutionRequest request = new DefaultDependencyResolutionRequest();
+        request.setMavenProject( project );
+        request.setRepositorySession( session );
+        request.setResolutionFilter( null );
+
+        DependencyNode rootNode = resolver.resolve( request ).getDependencyGraph();
+        // Don't want transitive test dependencies included in analysis
+        DependencyNode prunedRoot = pruneTransitiveTestDependencies( rootNode, project );
+        applyDependencyManagement( project, prunedRoot );
+        return prunedRoot;
+    }
+
+    private void applyDependencyManagement( MavenProject project, DependencyNode root )
+    {
+        Map<String, org.apache.maven.model.Dependency> dependencyManagementMap = createDependencyManagementMap(
+                project.getDependencyManagement() );
+
+        for ( DependencyNode child : root.getChildren() )
+        {
+            for ( DependencyNode nonTransitiveDependencyNode : child.getChildren() )
+            {
+                applyDependencyManagementDfs( dependencyManagementMap, nonTransitiveDependencyNode );
+            }
+        }
+    }
+
+    private void applyDependencyManagementDfs( Map<String, org.apache.maven.model.Dependency> dependencyManagementMap,
+                                               DependencyNode node )
+    {
+        if ( dependencyManagementMap.containsKey( getDependencyManagementCoordinate( node.getArtifact() ) ) )
+        {
+            org.apache.maven.model.Dependency manager = dependencyManagementMap.get(
+                    getDependencyManagementCoordinate( node.getArtifact() ) );
+            Map<String, String> artifactProperties = new HashMap<>();
+            for ( Map.Entry<String, String> entry : node.getArtifact().getProperties().entrySet() )
+            {
+                artifactProperties.put( entry.getKey(), entry.getValue() );
+            }
+
+            if ( !manager.getVersion().equals( node.getArtifact().getVersion() ) )
+            {
+                artifactProperties.put( PRE_MANAGED_VERSION, node.getArtifact().getVersion() );
+                node.setArtifact( node.getArtifact().setVersion( manager.getVersion() ) );
+            }
+
+            if ( !manager.getScope().equals( node.getDependency().getScope() ) )
+            {
+                artifactProperties.put( PRE_MANAGED_SCOPE, node.getDependency().getScope() );
+                // be aware this does not actually change the node's scope, it may need to be fixed in the future
+                artifactProperties.put( MANAGED_SCOPE, manager.getScope() );
+            }
+            node.setArtifact( node.getArtifact().setProperties( artifactProperties ) );
+            node.getDependency().setArtifact( node.getDependency().getArtifact().setProperties( artifactProperties ) );
+        }
+        for ( DependencyNode child : node.getChildren() )
+        {
+            applyDependencyManagementDfs( dependencyManagementMap, child );
+        }
+    }
+
+    private static Map<String, org.apache.maven.model.Dependency> createDependencyManagementMap(
+            DependencyManagement dependencyManagement )
+    {
+        Map<String, org.apache.maven.model.Dependency> dependencyManagementMap = new HashMap<>();
+        if ( dependencyManagement == null )
+        {
+            return dependencyManagementMap;
+        }
+        for ( org.apache.maven.model.Dependency dependency : dependencyManagement.getDependencies() )
+        {
+            dependencyManagementMap.put( getDependencyManagementCoordinate( dependency ), dependency );
+        }
+        return dependencyManagementMap;
+    }
+
+    private static String getDependencyManagementCoordinate( org.apache.maven.model.Dependency dependency )
+    {
+        StringBuilder string = new StringBuilder();
+        string.append( dependency.getGroupId() ).append( ":" ).append( dependency.getArtifactId() )
+                .append( ":" ).append( dependency.getType() );
+        if ( dependency.getClassifier() != null && !dependency.getClassifier().equals( "" ) )
+        {
+            string.append( ":" ).append( dependency.getClassifier() );
+        }
+        return string.toString();
+    }
+
+    private static String getDependencyManagementCoordinate( Artifact artifact )
+    {
+        StringBuilder string = new StringBuilder();
+        string.append( artifact.getGroupId() ).append( ":" ).append( artifact.getArtifactId() ).append( ":" )
+                .append( artifact.getExtension() );
+        if ( artifact.getClassifier() != null && !artifact.getClassifier().equals( "" ) )
+        {
+            string.append( ":" ).append( artifact.getClassifier() );
+        }
+        return string.toString();
+    }
+
+    private Dependency getProjectDependency( MavenProject project )
+    {
+        Model model = project.getModel();
+
+        return new Dependency( new DefaultArtifact( model.getGroupId(), model.getArtifactId(), model.getPackaging(),
+                model.getVersion() ), "" );
+    }
+
+    private DependencyNode pruneTransitiveTestDependencies( DependencyNode rootNode, MavenProject project )
+    {
+        Set<DependencyNode> visitedNodes = new HashSet<>();
+        DependencyNode newRoot = new DefaultDependencyNode( getProjectDependency( project ) );
+        newRoot.setChildren( new ArrayList<DependencyNode>() );
+
+        for ( int i = 0; i < rootNode.getChildren().size(); i++ )
+        {
+            DependencyNode childNode = rootNode.getChildren().get( i );
+            newRoot.getChildren().add( childNode );
+
+            pruneTransitiveTestDependenciesDfs( childNode, visitedNodes );
+        }
+
+        return newRoot;
+    }
+
+    private void pruneTransitiveTestDependenciesDfs( DependencyNode node , Set<DependencyNode> visitedNodes )
+    {
+        if ( !visitedNodes.contains( node ) )
+        {
+            visitedNodes.add( node );
+            // iterator needed to avoid concurrentModificationException
+            Iterator<DependencyNode> iterator = node.getChildren().iterator();
+            while ( iterator.hasNext() )
+            {
+                DependencyNode child = iterator.next();
+                if ( child.getDependency().getScope().equals( "test" ) )
+                {
+                    iterator.remove();
+                }
+                else
+                {
+                    pruneTransitiveTestDependenciesDfs( child, visitedNodes );
+                }
+            }
+        }
+    }
+}

Review comment:
       Provide NL.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo merged pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo merged pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] bimargulies-google commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
bimargulies-google commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r465139379



##########
File path: src/it/projects/analyze-testDependencyWithNonTestScope/src/main/java/hello/Hello.java
##########
@@ -0,0 +1,24 @@
+package hello;
+
+/*
+ * 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.
+ */
+
+public class Hello

Review comment:
       Comment why you have a an empty class?

##########
File path: src/it/projects/tree-verbose-small/pom.xml
##########
@@ -0,0 +1,64 @@
+<?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>tree-verbose2</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <name>VerboseTest2</name>
+  <description>
+    Test2 verbose dependency:tree with actual Maven version.

Review comment:
       Can you clarify this comment; what do we mean by 'actual maven version'?

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,221 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+
+/**
+ * Aether initialization. This is based on Apache Maven Resolver 1.4.2 or later. There are many other versions of Aether

Review comment:
       Does 'base on' means 'works with'?

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,329 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Repository;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.DependencyRequest;
+import org.eclipse.aether.resolution.DependencyResult;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.maven.plugins.dependency.tree.verbose.RepositoryUtility.mavenRepositoryFromUrl;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+
+public class VerboseDependencyGraphBuilder
+{
+    private RepositorySystem repositorySystem;
+
+    /**
+     * Maven repositories to use when resolving dependencies.
+     */
+    private final List<RemoteRepository> repositories;
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",
+            "https://repo1.maven.org/maven2/" ).build();
+
+    public VerboseDependencyGraphBuilder()
+    {
+        this( Collections.singletonList( CENTRAL.getUrl() ) );
+    }
+
+    static
+    {
+        for ( Map.Entry<String, String> entry : OsProperties.detectOsProperties().entrySet() )
+        {
+            System.setProperty( entry.getKey(), entry.getValue() );
+        }
+    }
+
+    /**
+     * @param mavenRepositoryUrls remote Maven repositories to search for dependencies
+     * @throws IllegalArgumentException if a URL is malformed or does not have an allowed scheme
+     */
+    public VerboseDependencyGraphBuilder( Iterable<String> mavenRepositoryUrls )
+    {
+        List<RemoteRepository> repositoryList = new ArrayList<RemoteRepository>();
+        for ( String mavenRepositoryUrl : mavenRepositoryUrls )
+        {
+            RemoteRepository repository = mavenRepositoryFromUrl( mavenRepositoryUrl );
+            repositoryList.add( repository );
+        }
+        this.repositories = repositoryList;
+    }
+
+    public VerboseDependencyGraphBuilder(  List<Repository> repositories )
+    {
+        List<RemoteRepository> repositoryList = new ArrayList<RemoteRepository>();
+        for ( Repository repo : repositories )
+        {
+            repositoryList.add( mavenRepositoryFromUrl( repo.getUrl() ) );
+        }
+        this.repositories = repositoryList;
+    }
+
+    public DependencyNode buildVerboseGraphNoManagement( MavenProject project, RepositorySystem system )
+            throws MojoExecutionException
+    {
+        repositorySystem = system;
+        List<org.apache.maven.model.Dependency> dependencies = project.getDependencies();
+
+        DependencyNode rootNode = new DefaultDependencyNode( getProjectDependency( project ) );
+
+
+        for ( org.apache.maven.model.Dependency dependency : dependencies )
+        {
+            rootNode.getChildren().add( buildFullDependencyGraph( Collections.singletonList( dependency ), project ) );
+        }
+
+        // Don't want transitive test dependencies included in analysis
+        DependencyNode prunedRoot = pruneTransitiveTestDependencies( rootNode, project );
+        applyDependencyManagement( project, prunedRoot );
+        return prunedRoot;
+    }
+
+    private void applyDependencyManagement( MavenProject project, DependencyNode root )
+    {
+        Map<String, org.apache.maven.model.Dependency> dependencyManagementMap = createDependencyManagementMap(
+                project.getDependencyManagement() );
+
+        for ( DependencyNode child : root.getChildren() )
+        {
+            for ( DependencyNode nonTransitiveDependencyNode : child.getChildren() )
+            {
+                applyDependencyManagementDfs( dependencyManagementMap, nonTransitiveDependencyNode );
+            }
+        }
+    }
+
+    private void applyDependencyManagementDfs( Map<String, org.apache.maven.model.Dependency> dependencyManagementMap,
+                                               DependencyNode node )
+    {
+        if ( dependencyManagementMap.containsKey( getDependencyManagementCoordinate( node.getArtifact() ) ) )
+        {
+            org.apache.maven.model.Dependency manager = dependencyManagementMap.get(
+                    getDependencyManagementCoordinate( node.getArtifact() ) );
+            Map<String, String> artifactProperties = new HashMap<>();
+            for ( Map.Entry<String, String> entry : node.getArtifact().getProperties().entrySet() )
+            {
+                artifactProperties.put( entry.getKey(), entry.getValue() );
+            }
+
+            if ( !manager.getVersion().equals( node.getArtifact().getVersion() ) )
+            {
+                artifactProperties.put( "preManagedVersion", node.getArtifact().getVersion() );
+                node.setArtifact( node.getArtifact().setVersion( manager.getVersion() ) );
+            }
+
+            if ( !manager.getScope().equals( node.getDependency().getScope() ) )
+            {
+                artifactProperties.put( "preManagedScope", node.getDependency().getScope() );

Review comment:
       Any temptation to constant-ize these strings?

##########
File path: src/it/projects/analyze-testDependencyWithNonTestScope/invoker.properties
##########
@@ -0,0 +1,21 @@
+# 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.
+
+invoker.goals = clean ${project.groupId}:${project.artifactId}:${project.version}:analyze-report
+# don't know why it fails with Maven 2 on some weird java.lang.NoClassDefFoundError: org/apache/commons/logging/LogFactory

Review comment:
       +1

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/MavenRepositoryException.java
##########
@@ -0,0 +1,34 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+/**
+ * Error interacting with a remote or local Maven repository such as artifact not found.
+ */
+public class MavenRepositoryException extends Exception

Review comment:
       Yea, either use an existing one or pick a more descriptive name.

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseGraphSerializer.java
##########
@@ -0,0 +1,323 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.graph.DependencyNode;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Parses dependency graph and outputs in text format for end user to review.
+ */
+public final class VerboseGraphSerializer
+{
+    private static final String LINE_START_LAST_CHILD = "\\- ";
+    private static final String LINE_START_CHILD = "+- ";
+
+    public String serialize( DependencyNode root )
+    {
+        Set<String> coordinateStrings = new HashSet<>();
+        Map<String, String> coordinateVersionMap = new HashMap<>();
+        StringBuilder builder = new StringBuilder();
+
+        // Use BFS to mirror how Maven resolves dependencies and use DFS to print the tree easily
+        Map<DependencyNode, String> nodeErrors = getNodeConflictMessagesBfs( root, coordinateStrings,
+                coordinateVersionMap );
+
+        // deal with root first
+        Artifact rootArtifact = root.getArtifact();
+        builder.append( rootArtifact.getGroupId() ).append( ":" ).append( rootArtifact.getArtifactId() ).append( ":" )
+                .append( rootArtifact.getExtension() ).append( ":" ).append( rootArtifact.getVersion() ).append(
+                        System.lineSeparator() );
+
+        for ( int i = 0; i < root.getChildren().size(); i++ )
+        {
+            if ( i == root.getChildren().size() - 1 )
+            {
+                dfsPrint( root.getChildren().get( i ), LINE_START_LAST_CHILD, false, builder, nodeErrors );
+            }
+            else
+            {
+                dfsPrint( root.getChildren().get( i ), LINE_START_CHILD, false, builder, nodeErrors );
+            }
+        }
+        return builder.toString();
+    }
+
+    private static String getDependencyCoordinate( DependencyNode node )
+    {
+        Artifact artifact = node.getArtifact();
+
+        if ( node.getDependency() == null )
+        {
+            // should only get here if node is root
+            return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension() + ":"
+                    + artifact.getVersion();
+        }
+
+        String scope;
+        if ( artifact.getProperties().containsKey( "managedScope" ) )
+        {
+            scope = artifact.getProperties().get( "managedScope" );
+        }
+        else
+        {
+            scope = node.getDependency().getScope();
+        }
+
+        String coords = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension() + ":"
+                + artifact.getVersion();
+
+        if ( scope != null && !scope.isEmpty() )
+        {
+            coords = coords.concat( ":" + scope );
+        }
+        return coords;
+    }
+
+    private static String getVersionlessScopelessCoordinate( DependencyNode node )
+    {
+        Artifact artifact = node.getArtifact();
+        // scope not included because we check for scope conflicts separately
+        return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension();
+    }
+
+    private static boolean isDuplicateDependencyCoordinate( DependencyNode node, Set<String> coordinateStrings )
+    {
+        return coordinateStrings.contains( getDependencyCoordinate( node ) );
+    }
+
+    private static String versionConflict( DependencyNode node, Map<String, String> coordinateVersionMap )
+    {
+        if ( coordinateVersionMap.containsKey( getVersionlessScopelessCoordinate( node ) ) )
+        {
+            return coordinateVersionMap.get( getVersionlessScopelessCoordinate( node ) );
+        }
+        return null;
+    }
+
+    private static String scopeConflict( DependencyNode node, Set<String> coordinateStrings )
+    {
+        Artifact artifact = node.getArtifact();
+        List<String> scopes = Arrays.asList( "compile", "provided", "runtime", "test", "system" );
+
+        for ( String scope : scopes )
+        {
+            String version;
+            if ( artifact.getProperties().containsKey( "version" ) )
+            {
+                version = artifact.getProperties().get( "version" );
+            }
+            else
+            {
+                version = artifact.getVersion();
+            }
+
+            String coordinate = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension()
+                    + ":" + version + ":" + scope;
+            if ( coordinateStrings.contains( coordinate ) )
+            {
+                return scope;
+            }
+        }
+        return null;
+    }
+
+    private Map<DependencyNode, String> getNodeConflictMessagesBfs( DependencyNode root, Set<String> coordinateStrings
+            , Map<String, String> coordinateVersionMap )
+    {
+        Map<DependencyNode, String> nodeErrors = new HashMap<>();
+        Set<DependencyNode> visitedNodes = new HashSet<>( 512 );
+        Queue<DependencyNode> queue = new LinkedList<>();
+        visitedNodes.add( root );
+        queue.add( root );
+
+        while ( !queue.isEmpty() )
+        {
+            DependencyNode node = queue.poll();
+
+            if ( node == null || node.getArtifact() == null )
+            {
+                // Should never reach hit this condition with a proper graph sent in
+                nodeErrors.put( node, "Null Artifact Node" );

Review comment:
       Is this awful enough to warrant just throwing?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r463783151



##########
File path: src/it/projects/analyze-testDependencyWithNonTestScope/invoker.properties
##########
@@ -0,0 +1,21 @@
+# 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.
+
+invoker.goals = clean ${project.groupId}:${project.artifactId}:${project.version}:analyze-report
+# don't know why it fails with Maven 2 on some weird java.lang.NoClassDefFoundError: org/apache/commons/logging/LogFactory

Review comment:
       you can remove this comment since Maven 2 isn't supported

##########
File path: pom.xml
##########
@@ -424,17 +438,24 @@ under the License.
               <pomExcludes>
                 <pomExclude>purge-local-repository-bad-pom/pom.xml</pomExclude>
                 <!-- verbose was using Maven2 code, removed with MDEP-494, requires MSHARED-339 to be fixed first -->
-                <pomExclude>tree-verbose/pom.xml</pomExclude>
+                <!--<pomExclude>tree-verbose/pom.xml</pomExclude>-->
+                <pomExclude>setup-custom-ear-lifecycle\pom.xml</pomExclude>
               </pomExcludes>
               <pomIncludes>
                 <pomInclude>*/pom.xml</pomInclude>
+                <pomInclude>analyze*/pom.xml</pomInclude>
+                <pomInclude>analyze-testDependencyWithNonTestScope/pom.xml</pomInclude>
                 <pomInclude>purge-local-repository-without-pom</pomInclude>
               </pomIncludes>
               <!-- for mrm -->
               <settingsFile>src/it/mrm/settings.xml</settingsFile>
               <filterProperties>
                 <repository.proxy.url>${repository.proxy.url}</repository.proxy.url>
               </filterProperties>
+
+              <addTestClassPath>true</addTestClassPath>
+              <!--<mavenExecutable>${maven.home}/bin/mvnDebug</mavenExecutable>-->

Review comment:
       this should probably be removed, not commented out

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,329 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Repository;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.DependencyRequest;
+import org.eclipse.aether.resolution.DependencyResult;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.maven.plugins.dependency.tree.verbose.RepositoryUtility.mavenRepositoryFromUrl;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+

Review comment:
       no blank line

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/MavenRepositoryException.java
##########
@@ -0,0 +1,34 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+/**
+ * Error interacting with a remote or local Maven repository such as artifact not found.
+ */
+public class MavenRepositoryException extends Exception

Review comment:
       There's probably an existing exception type we could use here. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r466705394



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,207 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+
+/**
+ * Aether initialization. This uses Maven Resolver 1.4.2 or later. There are many other versions of Aether

Review comment:
       This comment seems no longer correct. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] ian-lavallee commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
ian-lavallee commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r467268283



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseGraphSerializer.java
##########
@@ -0,0 +1,311 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.graph.DependencyNode;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Parses dependency graph and outputs in text format for end user to review.
+ */
+public final class VerboseGraphSerializer

Review comment:
       Because of the verbose package its in, yes. It only has the one public method so I can either leave it or get rid of the verbose package and put it in org.apache.maven.plugins.dependency.tree instead of org.apache.maven.plugins.dependency.tree.verbose since the verbose package only has 3 classes now

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,332 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Repository;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.DependencyRequest;
+import org.eclipse.aether.resolution.DependencyResult;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.maven.plugins.dependency.tree.verbose.RepositoryUtility.mavenRepositoryFromUrl;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+public class VerboseDependencyGraphBuilder

Review comment:
       Because of the verbose package its in, yes. It only has the one public method so I can either leave it or get rid of the verbose package and put it in org.apache.maven.plugins.dependency.tree instead of org.apache.maven.plugins.dependency.tree.verbose since the verbose package only has 3 classes now




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r467126945



##########
File path: src/it/projects/tree-verbose/verify.bsh
##########
@@ -27,11 +27,10 @@ String expected = FileUtils.fileRead( new File( basedir, "expected.txt" ) );
 actual = actual.replaceAll( "[\n\r]+", "\n" );
 expected = expected.replaceAll( "[\n\r]+", "\n" );
 
-System.out.println( "Checking dependency tree..." );
-
 if ( !actual.equals( expected ) )
 {
-    throw new Exception( "Unexpected dependency tree" );
+    throw new Exception( "Unexpected dependency tree." + System.lineSeperator() + "Expected:" + System.lineSeperator()

Review comment:
       separator

##########
File path: pom.xml
##########
@@ -423,12 +415,10 @@ under the License.
               <projectsDirectory>src/it/projects</projectsDirectory>
               <pomExcludes>
                 <pomExclude>purge-local-repository-bad-pom/pom.xml</pomExclude>
-                <!-- verbose was using Maven2 code, removed with MDEP-494, requires MSHARED-339 to be fixed first -->
-                <pomExclude>tree-verbose/pom.xml</pomExclude>
               </pomExcludes>
               <pomIncludes>
                 <pomInclude>*/pom.xml</pomInclude>
-                <pomInclude>purge-local-repository-without-pom</pomInclude>
+                <!--<pomInclude>purge-local-repository-without-pom</pomInclude>-->

Review comment:
       What's up with this test? It should either be uncommented or removed. 

##########
File path: src/it/projects/unpack-custom-ear/verify.groovy
##########
@@ -17,5 +17,11 @@
  * under the License.
  */
 def buildLog = new File( basedir, 'build.log' )
+String fileContents = buildLog.text
 assert buildLog.exists()
-assert buildLog.text.contains( "[DEBUG] Found unArchiver by type: " )
+assert buildLog.length() != 0
+
+println "BuildLog Length:"

Review comment:
       we should probably take these two out now that debugging is done. Assertions can stay

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,208 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+
+/**
+ * Aether initialization. This uses org.eclipse.aether:aether-api:0.9.0.M2. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, eventually you may want to change it over to
+ * org.apache.maven.resolver:maven-resolver-api.
+ */
+final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",
+            "https://repo1.maven.org/maven2/" ).build();
+
+    private RepositoryUtility()
+    {
+    }
+
+    private static DefaultRepositorySystemSession createDefaultRepositorySystemSession( RepositorySystem system )
+    {
+        DefaultRepositorySystemSession session = MavenRepositorySystemUtils.newSession();
+        LocalRepository localRepository = new LocalRepository( findLocalRepository() );
+        session.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepository ) );
+        return session;
+    }
+
+    /**
+     * Opens a new Maven repository session that looks for the local repository in the customary ~/.m2 directory. If not
+     * found, it creates an initially empty repository in a temporary location.
+     */
+    private static DefaultRepositorySystemSession newSession( RepositorySystem system )
+    {
+        DefaultRepositorySystemSession session = createDefaultRepositorySystemSession( system );
+        return session;
+    }
+
+    /**
+     * Open a new Maven repository session for full dependency graph resolution.
+     *
+     * @see {@link VerboseDependencyGraphBuilder}
+     */
+    static DefaultRepositorySystemSession newSessionForFullDependency( RepositorySystem system )
+    {
+        // This combination of DependencySelector comes from the default specified in
+        // `MavenRepositorySystemUtils.newSession`.
+        // LinkageChecker needs to include 'provided'-scope and optional dependencies.
+        DependencySelector dependencySelector = new AndDependencySelector(
+                // ScopeDependencySelector takes exclusions. 'Provided' scope is not here to avoid
+                // false positive in LinkageChecker.
+                new ScopeDependencySelector(), // removed "test" parameter
+                new ExclusionDependencySelector() );
+
+        return newSession( system, dependencySelector );
+    }
+
+    private static DefaultRepositorySystemSession newSession( RepositorySystem system,
+                                                              DependencySelector dependencySelector )
+    {
+        DefaultRepositorySystemSession session = createDefaultRepositorySystemSession( system );
+        session.setDependencySelector( dependencySelector );
+
+        // By default, Maven's MavenRepositorySystemUtils.newSession() returns a session with
+        // ChainedDependencyGraphTransformer(ConflictResolver(...), JavaDependencyContextRefiner()).
+        // Because the full dependency graph does not resolve conflicts of versions, this session does
+        // not use ConflictResolver.
+        session.setDependencyGraphTransformer(
+                new ChainedDependencyGraphTransformer( new CycleBreakerGraphTransformer(), // Avoids StackOverflowError
+                        new JavaDependencyContextRefiner() ) );
+
+        // No dependency management in the full dependency graph
+        session.setDependencyManager( null );
+
+        return session;
+    }
+
+    private static String findLocalRepository()
+    {
+        // TODO is there Maven code for this?

Review comment:
       we should resolve this todo. 

##########
File path: src/it/projects/tree-verbose-small/verify.bsh
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+
+import java.io.*;
+import org.codehaus.plexus.util.*;
+
+String actual = FileUtils.fileRead( new File( basedir, "target/tree.txt" ) );
+String expected = FileUtils.fileRead( new File( basedir, "expected.txt" ) );
+
+actual = actual.replaceAll( "[\n\r]+", "\n" );
+expected = expected.replaceAll( "[\n\r]+", "\n" );
+
+if ( !actual.equals( expected ) )
+{
+    throw new Exception( "Unexpected dependency tree." + System.lineSeperator() + "Expected:" + System.lineSeperator()

Review comment:
       I think this and below should be `System.lineSeparator​`
   i.e. seperator --> separator
   
   It's concerning that nothing caught this.
   
   
   

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,208 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+
+/**
+ * Aether initialization. This uses org.eclipse.aether:aether-api:0.9.0.M2. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, eventually you may want to change it over to
+ * org.apache.maven.resolver:maven-resolver-api.
+ */
+final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",
+            "https://repo1.maven.org/maven2/" ).build();
+
+    private RepositoryUtility()
+    {
+    }
+
+    private static DefaultRepositorySystemSession createDefaultRepositorySystemSession( RepositorySystem system )
+    {
+        DefaultRepositorySystemSession session = MavenRepositorySystemUtils.newSession();
+        LocalRepository localRepository = new LocalRepository( findLocalRepository() );
+        session.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepository ) );
+        return session;
+    }
+
+    /**
+     * Opens a new Maven repository session that looks for the local repository in the customary ~/.m2 directory. If not
+     * found, it creates an initially empty repository in a temporary location.
+     */
+    private static DefaultRepositorySystemSession newSession( RepositorySystem system )
+    {
+        DefaultRepositorySystemSession session = createDefaultRepositorySystemSession( system );
+        return session;
+    }
+
+    /**
+     * Open a new Maven repository session for full dependency graph resolution.
+     *
+     * @see {@link VerboseDependencyGraphBuilder}
+     */
+    static DefaultRepositorySystemSession newSessionForFullDependency( RepositorySystem system )
+    {
+        // This combination of DependencySelector comes from the default specified in
+        // `MavenRepositorySystemUtils.newSession`.
+        // LinkageChecker needs to include 'provided'-scope and optional dependencies.
+        DependencySelector dependencySelector = new AndDependencySelector(
+                // ScopeDependencySelector takes exclusions. 'Provided' scope is not here to avoid
+                // false positive in LinkageChecker.
+                new ScopeDependencySelector(), // removed "test" parameter
+                new ExclusionDependencySelector() );
+
+        return newSession( system, dependencySelector );
+    }
+
+    private static DefaultRepositorySystemSession newSession( RepositorySystem system,
+                                                              DependencySelector dependencySelector )
+    {
+        DefaultRepositorySystemSession session = createDefaultRepositorySystemSession( system );
+        session.setDependencySelector( dependencySelector );
+
+        // By default, Maven's MavenRepositorySystemUtils.newSession() returns a session with
+        // ChainedDependencyGraphTransformer(ConflictResolver(...), JavaDependencyContextRefiner()).
+        // Because the full dependency graph does not resolve conflicts of versions, this session does
+        // not use ConflictResolver.
+        session.setDependencyGraphTransformer(
+                new ChainedDependencyGraphTransformer( new CycleBreakerGraphTransformer(), // Avoids StackOverflowError
+                        new JavaDependencyContextRefiner() ) );
+
+        // No dependency management in the full dependency graph
+        session.setDependencyManager( null );
+
+        return session;
+    }
+
+    private static String findLocalRepository()
+    {
+        // TODO is there Maven code for this?
+        Path home = Paths.get( System.getProperty( "user.home" ) );
+        Path localRepo = home.resolve( ".m2" ).resolve( "repository" );
+        if ( Files.isDirectory( localRepo ) )
+        {
+            return localRepo.toAbsolutePath().toString();
+        }
+        else
+        {
+            return makeTemporaryLocalRepository();

Review comment:
       This one concerns me. It might not be appropriate in the context of a maven plugin. Unlike cloud-opensource-java, there really should be a local repo already. 

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/OsProperties.java
##########
@@ -0,0 +1,89 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+/**
+ * Platform-dependent project properties normalized from ${os.name} and ${os.arch}. Netty uses these to select
+ * system-specific dependencies through the
+ * <a href='https://github.com/trustin/os-maven-plugin'>os-maven-plugin</a>.
+ */
+final class OsProperties
+{
+
+    public static Map<String, String> detectOsProperties()

Review comment:
       non-public

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,208 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+
+/**
+ * Aether initialization. This uses org.eclipse.aether:aether-api:0.9.0.M2. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, eventually you may want to change it over to
+ * org.apache.maven.resolver:maven-resolver-api.
+ */
+final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",

Review comment:
       non-public

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,332 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Repository;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.DependencyRequest;
+import org.eclipse.aether.resolution.DependencyResult;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.maven.plugins.dependency.tree.verbose.RepositoryUtility.mavenRepositoryFromUrl;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+public class VerboseDependencyGraphBuilder

Review comment:
       does this need to be public?

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseGraphSerializer.java
##########
@@ -0,0 +1,311 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.graph.DependencyNode;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Parses dependency graph and outputs in text format for end user to review.
+ */
+public final class VerboseGraphSerializer

Review comment:
       does this need to be public?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#issuecomment-668760436


   CI running here: https://builds.apache.org/job/maven-box/job/maven-dependency-plugin/job/te/


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] elharo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r465241721



##########
File path: pom.xml
##########
@@ -320,6 +311,27 @@ under the License.
         </exclusion>
       </exclusions>
     </dependency>
+    <!-- VerboseGraphBuilder & VerboseSerializer -->

Review comment:
       I wouldn't include this comment since these are not the artifacts where those classes are found. 

##########
File path: src/it/projects/tree-verbose-small/verify.bsh
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+import java.io.*;
+
+import org.codehaus.plexus.util.*;
+
+System.out.println( "Basedir is " + basedir );
+
+String actual = FileUtils.fileRead( new File( basedir, "target/tree.txt" ) );
+String expected = FileUtils.fileRead( new File( basedir, "expected.txt" ) );
+
+actual = actual.replaceAll( "[\n\r]+", "\n" );
+expected = expected.replaceAll( "[\n\r]+", "\n" );
+
+System.out.println( "Checking dependency tree..." );
+
+if ( !actual.equals( expected ) )
+{
+    throw new Exception( "Unexpected dependency tree" );

Review comment:
       It helps if a test failure can indicate why it failed; in this case what was expected and what did it actually get. Or if those two are large, then what's the difference?
   
   Might not be worth doing here unless there's a good utility method for this we can call to calculate the diff. 

##########
File path: src/it/projects/tree-verbose-small/verify.bsh
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+import java.io.*;
+
+import org.codehaus.plexus.util.*;
+
+System.out.println( "Basedir is " + basedir );

Review comment:
       This might be something Maven commonly does in integration tests, but the general principle is that a passing test generates no output. 

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,216 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+
+/**
+ * Aether initialization. This uses Maven Resolver 1.4.2 or later. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, but this is the current one.
+ */
+public final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",
+            "https://repo1.maven.org/maven2/" ).build();
+
+    // DefaultTransporterProvider.newTransporter checks these transporters
+    private static final Set<String> ALLOWED_REPOSITORY_URL_SCHEMES = new HashSet<String>(
+            Arrays.asList( "file", "http", "https" ) );
+
+    private RepositoryUtility()
+    {
+    }
+
+    //@VisibleForTesting
+    static DefaultRepositorySystemSession createDefaultRepositorySystemSession( RepositorySystem system )
+    {
+        DefaultRepositorySystemSession session = MavenRepositorySystemUtils.newSession();
+        LocalRepository localRepository = new LocalRepository( findLocalRepository() );
+        session.setLocalRepositoryManager( system.newLocalRepositoryManager( session, localRepository ) );
+        return session;
+    }
+
+    /**
+     * Opens a new Maven repository session that looks for the local repository in the customary ~/.m2 directory. If not
+     * found, it creates an initially empty repository in a temporary location.
+     */
+    public static DefaultRepositorySystemSession newSession( RepositorySystem system )
+    {
+        DefaultRepositorySystemSession session = createDefaultRepositorySystemSession( system );
+        return session;
+    }
+
+    /**
+     * Open a new Maven repository session for full dependency graph resolution.
+     *
+     * @see {@link VerboseDependencyGraphBuilder}
+     */
+    static DefaultRepositorySystemSession newSessionForFullDependency( RepositorySystem system )
+    {
+        // This combination of DependencySelector comes from the default specified in
+        // `MavenRepositorySystemUtils.newSession`.
+        // LinkageChecker needs to include 'provided'-scope and optional dependencies.
+        DependencySelector dependencySelector = new AndDependencySelector(
+                // ScopeDependencySelector takes exclusions. 'Provided' scope is not here to avoid
+                // false positive in LinkageChecker.
+                new ScopeDependencySelector(), // removed "test" parameter
+                new ExclusionDependencySelector() );
+
+        return newSession( system, dependencySelector );
+    }
+
+    private static DefaultRepositorySystemSession newSession( RepositorySystem system,
+                                                              DependencySelector dependencySelector )
+    {
+        DefaultRepositorySystemSession session = createDefaultRepositorySystemSession( system );
+        session.setDependencySelector( dependencySelector );
+
+        // By default, Maven's MavenRepositorySystemUtils.newSession() returns a session with
+        // ChainedDependencyGraphTransformer(ConflictResolver(...), JavaDependencyContextRefiner()).
+        // Because the full dependency graph does not resolve conflicts of versions, this session does
+        // not use ConflictResolver.
+        session.setDependencyGraphTransformer(
+                new ChainedDependencyGraphTransformer( new CycleBreakerGraphTransformer(), // Avoids StackOverflowError
+                        new JavaDependencyContextRefiner() ) );
+
+        // No dependency management in the full dependency graph
+        session.setDependencyManager( null );
+
+        return session;
+    }
+
+    private static String findLocalRepository()
+    {
+        // TODO is there Maven code for this?
+        Path home = Paths.get( System.getProperty( "user.home" ) );
+        Path localRepo = home.resolve( ".m2" ).resolve( "repository" );
+        if ( Files.isDirectory( localRepo ) )
+        {
+            return localRepo.toAbsolutePath().toString();
+        }
+        else
+        {
+            return makeTemporaryLocalRepository();
+        }
+    }
+
+    private static String makeTemporaryLocalRepository()
+    {
+        try
+        {
+            File temporaryDirectory = Files.createTempDirectory( "m2" ).toFile();
+            temporaryDirectory.deleteOnExit();
+            return temporaryDirectory.getAbsolutePath();
+        }
+        catch ( IOException ex )
+        {
+            return null;
+        }
+    }
+
+    /**
+     * Returns Maven repository specified as {@code mavenRepositoryUrl}, after validating the syntax of the URL.
+     *
+     * @throws IllegalArgumentException if the URL is malformed for a Maven repository
+     */
+    public static RemoteRepository mavenRepositoryFromUrl( String mavenRepositoryUrl )
+    {
+        try
+        {
+            // Because the protocol is not an empty string (checked below), this URI is absolute.
+            new URI( mavenRepositoryUrl );
+        }
+        catch ( URISyntaxException ex )
+        {
+            throw new IllegalArgumentException( "Invalid URL syntax: " + mavenRepositoryUrl );
+        }
+
+        RemoteRepository repository = new RemoteRepository.Builder( null, "default", mavenRepositoryUrl ).build();
+
+        return repository;
+    }
+
+    private static VersionRangeResult findVersionRange( RepositorySystem repositorySystem,
+                                                        RepositorySystemSession session,
+                                                        String groupId, String artifactId )
+            throws VersionRangeResolutionException
+    {
+
+        Artifact artifactWithVersionRange = new DefaultArtifact( groupId, artifactId, null, "(0,]" );
+        VersionRangeRequest request = new VersionRangeRequest( artifactWithVersionRange,
+                Arrays.asList( RepositoryUtility.CENTRAL ), null );
+
+
+        return repositorySystem.resolveVersionRange( session, request );
+
+    }
+
+    /**
+     * Returns the highest version for {@code groupId:artifactId} in {@code repositorySystem}.
+     */
+    static Version findHighestVersion( RepositorySystem repositorySystem, RepositorySystemSession session,
+                                       String groupId, String artifactId ) throws VersionRangeResolutionException
+    {
+        return findVersionRange( repositorySystem, session, groupId, artifactId ).getHighestVersion();
+    }
+
+    /**
+     * Returns the latest Maven coordinates for {@code groupId:artifactId} in {@code repositorySystem}.
+     */
+    public static String findLatestCoordinates( RepositorySystem repositorySystem, String groupId, String artifactId )

Review comment:
       please verify that we need all public methods in this class

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseDependencyGraphBuilder.java
##########
@@ -0,0 +1,332 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DependencyManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Repository;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.DependencyRequest;
+import org.eclipse.aether.resolution.DependencyResult;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.maven.plugins.dependency.tree.verbose.RepositoryUtility.mavenRepositoryFromUrl;
+
+/**
+ * Builds the VerboseDependencyGraph
+ */
+public class VerboseDependencyGraphBuilder
+{
+    private RepositorySystem repositorySystem;
+
+    /**
+     * Maven repositories to use when resolving dependencies.
+     */
+    private final List<RemoteRepository> repositories;
+
+    private static final String PRE_MANAGED_SCOPE = "preManagedScope", PRE_MANAGED_VERSION = "preManagedVersion",
+        MANAGED_SCOPE = "managedScope";
+
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",

Review comment:
       There should be a constant for this somewhere in Maven core or utils

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,216 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+
+/**
+ * Aether initialization. This uses Maven Resolver 1.4.2 or later. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, but this is the current one.
+ */
+public final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",

Review comment:
       I'm a little worried this class is not how we should be doing things in a Maven plugin. Generally we'd inject the session. 

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,216 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+
+/**
+ * Aether initialization. This uses Maven Resolver 1.4.2 or later. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, but this is the current one.
+ */
+public final class RepositoryUtility
+{
+
+    public static final RemoteRepository CENTRAL = new RemoteRepository.Builder( "central", "default",
+            "https://repo1.maven.org/maven2/" ).build();
+
+    // DefaultTransporterProvider.newTransporter checks these transporters
+    private static final Set<String> ALLOWED_REPOSITORY_URL_SCHEMES = new HashSet<String>(
+            Arrays.asList( "file", "http", "https" ) );
+
+    private RepositoryUtility()
+    {
+    }
+
+    //@VisibleForTesting

Review comment:
        @VisibleForTesting --> visible for testing
   since it's no longer an annotation

##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/RepositoryUtility.java
##########
@@ -0,0 +1,216 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+
+import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.collection.DependencySelector;
+import org.eclipse.aether.repository.LocalRepository;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.VersionRangeRequest;
+import org.eclipse.aether.resolution.VersionRangeResolutionException;
+import org.eclipse.aether.resolution.VersionRangeResult;
+import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
+import org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.util.graph.transformer.JavaDependencyContextRefiner;
+import org.eclipse.aether.version.Version;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+
+/**
+ * Aether initialization. This uses Maven Resolver 1.4.2 or later. There are many other versions of Aether
+ * from Sonatype and the Eclipse Project, but this is the current one.
+ */
+public final class RepositoryUtility

Review comment:
       can this be non-public?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] ian-lavallee commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
ian-lavallee commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r465194646



##########
File path: src/main/java/org/apache/maven/plugins/dependency/tree/verbose/VerboseGraphSerializer.java
##########
@@ -0,0 +1,323 @@
+package org.apache.maven.plugins.dependency.tree.verbose;
+
+/*
+ * 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.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.graph.DependencyNode;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * Parses dependency graph and outputs in text format for end user to review.
+ */
+public final class VerboseGraphSerializer
+{
+    private static final String LINE_START_LAST_CHILD = "\\- ";
+    private static final String LINE_START_CHILD = "+- ";
+
+    public String serialize( DependencyNode root )
+    {
+        Set<String> coordinateStrings = new HashSet<>();
+        Map<String, String> coordinateVersionMap = new HashMap<>();
+        StringBuilder builder = new StringBuilder();
+
+        // Use BFS to mirror how Maven resolves dependencies and use DFS to print the tree easily
+        Map<DependencyNode, String> nodeErrors = getNodeConflictMessagesBfs( root, coordinateStrings,
+                coordinateVersionMap );
+
+        // deal with root first
+        Artifact rootArtifact = root.getArtifact();
+        builder.append( rootArtifact.getGroupId() ).append( ":" ).append( rootArtifact.getArtifactId() ).append( ":" )
+                .append( rootArtifact.getExtension() ).append( ":" ).append( rootArtifact.getVersion() ).append(
+                        System.lineSeparator() );
+
+        for ( int i = 0; i < root.getChildren().size(); i++ )
+        {
+            if ( i == root.getChildren().size() - 1 )
+            {
+                dfsPrint( root.getChildren().get( i ), LINE_START_LAST_CHILD, false, builder, nodeErrors );
+            }
+            else
+            {
+                dfsPrint( root.getChildren().get( i ), LINE_START_CHILD, false, builder, nodeErrors );
+            }
+        }
+        return builder.toString();
+    }
+
+    private static String getDependencyCoordinate( DependencyNode node )
+    {
+        Artifact artifact = node.getArtifact();
+
+        if ( node.getDependency() == null )
+        {
+            // should only get here if node is root
+            return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension() + ":"
+                    + artifact.getVersion();
+        }
+
+        String scope;
+        if ( artifact.getProperties().containsKey( "managedScope" ) )
+        {
+            scope = artifact.getProperties().get( "managedScope" );
+        }
+        else
+        {
+            scope = node.getDependency().getScope();
+        }
+
+        String coords = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension() + ":"
+                + artifact.getVersion();
+
+        if ( scope != null && !scope.isEmpty() )
+        {
+            coords = coords.concat( ":" + scope );
+        }
+        return coords;
+    }
+
+    private static String getVersionlessScopelessCoordinate( DependencyNode node )
+    {
+        Artifact artifact = node.getArtifact();
+        // scope not included because we check for scope conflicts separately
+        return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension();
+    }
+
+    private static boolean isDuplicateDependencyCoordinate( DependencyNode node, Set<String> coordinateStrings )
+    {
+        return coordinateStrings.contains( getDependencyCoordinate( node ) );
+    }
+
+    private static String versionConflict( DependencyNode node, Map<String, String> coordinateVersionMap )
+    {
+        if ( coordinateVersionMap.containsKey( getVersionlessScopelessCoordinate( node ) ) )
+        {
+            return coordinateVersionMap.get( getVersionlessScopelessCoordinate( node ) );
+        }
+        return null;
+    }
+
+    private static String scopeConflict( DependencyNode node, Set<String> coordinateStrings )
+    {
+        Artifact artifact = node.getArtifact();
+        List<String> scopes = Arrays.asList( "compile", "provided", "runtime", "test", "system" );
+
+        for ( String scope : scopes )
+        {
+            String version;
+            if ( artifact.getProperties().containsKey( "version" ) )
+            {
+                version = artifact.getProperties().get( "version" );
+            }
+            else
+            {
+                version = artifact.getVersion();
+            }
+
+            String coordinate = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getExtension()
+                    + ":" + version + ":" + scope;
+            if ( coordinateStrings.contains( coordinate ) )
+            {
+                return scope;
+            }
+        }
+        return null;
+    }
+
+    private Map<DependencyNode, String> getNodeConflictMessagesBfs( DependencyNode root, Set<String> coordinateStrings
+            , Map<String, String> coordinateVersionMap )
+    {
+        Map<DependencyNode, String> nodeErrors = new HashMap<>();
+        Set<DependencyNode> visitedNodes = new HashSet<>( 512 );
+        Queue<DependencyNode> queue = new LinkedList<>();
+        visitedNodes.add( root );
+        queue.add( root );
+
+        while ( !queue.isEmpty() )
+        {
+            DependencyNode node = queue.poll();
+
+            if ( node == null || node.getArtifact() == null )
+            {
+                // Should never reach hit this condition with a proper graph sent in
+                nodeErrors.put( node, "Null Artifact Node" );

Review comment:
       My rationale for not throwing an exception is since this is the verbose option if a user is running this then something has probably gone wrong with their dependency resolution and we should show them if the graph has an invalid node somewhere so they can look into it. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-dependency-plugin] pzygielo commented on a change in pull request #92: [MDEP-644] Reintroduce the verbose option for dependency:tree

Posted by GitBox <gi...@apache.org>.
pzygielo commented on a change in pull request #92:
URL: https://github.com/apache/maven-dependency-plugin/pull/92#discussion_r466876792



##########
File path: src/test/java/org/apache/maven/plugins/dependency/TestListClassesMojo.java
##########
@@ -24,6 +24,7 @@
 import org.apache.maven.plugin.testing.stubs.MavenProjectStub;
 import org.apache.maven.settings.Server;
 import org.apache.maven.settings.Settings;
+import org.junit.Assert;

Review comment:
       Perhaps this unused import could be removed now.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org