You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/03 21:05:54 UTC

[GitHub] [maven] cstamas opened a new pull request, #900: [MNG-7619] Reverse Dependency Tree

cstamas opened a new pull request, #900:
URL: https://github.com/apache/maven/pull/900

   Adds Maven feature that is able to explain why an artifact is present in local repository.
   
   Usable for diagnosing resolution issues.
   
   ---
   
   https://issues.apache.org/jira/browse/MNG-7619


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] michael-o commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336265250

   Can we leave out these empty parens: `org.apache.maven.plugins:maven-dependency-plugin:jar:3.3.0 () (plugin)`?  From a user PoV it looks like a bug.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] cstamas commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336258963

   Excercise: build this PR (will produce Maven 3.9.0-SNAPSHOT distro), then use same distro to build itself (or just maven.3.9.x, does not matter) using these:
   ```
   mvn clean install -Dmaven.repo.local=local -Dmaven.repo.local.recordReverseTree -Drat.skip
   ```
   (rat is dumb to inspect "local" where local repository is and fail, so we skip it).
   
   Q1: why is maven-core 2.2.1 in local repo?
   ```
   [cstamas@infinity maven (maven-3.9.x +%)]$ tree -L 1 local/org/apache/maven/maven-core/2.2.1/.tracking/
   local/org/apache/maven/maven-core/2.2.1/.tracking/
   ├── org.apache.maven.plugins_maven-remote-resources-plugin_jar_1.7.0
   ├── org.codehaus.mojo_build-helper-maven-plugin_jar_1.12
   └── org.codehaus.mojo_buildnumber-maven-plugin_jar_1.4
   
   0 directories, 3 files
   [cstamas@infinity maven (maven-3.9.x +%)]$ cat local/org/apache/maven/maven-core/2.2.1/.tracking/org.codehaus.mojo_build-helper-maven-plugin_jar_1.12 
   org.apache.maven:maven-core:jar:2.2.1 (compile) (plugin)
     org.codehaus.mojo:build-helper-maven-plugin:jar:1.12 () (plugin)
   [cstamas@infinity maven (maven-3.9.x +%)]$
   ```
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] cstamas commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336366201

   @laeubi while I agree with ideas you wrote, I am still for this PR to go in, here is why: 
   This PR is (nearly) trivial, but gives you powerful tool (for investigation, answering several questions), is registered early (earliest possible), and does not prevent anything of those you wrote (a plugin). Moreover, by being earliest present, and present during whole session of a real build, it covers _everything_ (even some plugin that may dynamically resolve). Not easily done with something like build extension or even plugin (they are all too late to cover everything).
   
   Also, none of these prevents anything of those things you wrote, this is totally orthogonal. I personally see this like `-X`, a handy switch seldom used by users, but when one needs it, is very very handy.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] pzygielo commented on a diff in pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
pzygielo commented on code in PR #900:
URL: https://github.com/apache/maven/pull/900#discussion_r1038965386


##########
maven-core/src/main/java/org/apache/maven/internal/aether/ReverseTreeRepositoryListener.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+package org.apache.maven.internal.aether;
+
+import static java.util.Objects.requireNonNull;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.ListIterator;
+import java.util.Objects;
+import org.eclipse.aether.AbstractRepositoryListener;
+import org.eclipse.aether.RepositoryEvent;
+import org.eclipse.aether.RequestTrace;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.collection.CollectStepData;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * A class building reverse tree using {@link CollectStepData} trace data provided in {@link RepositoryEvent}
+ * events fired during collection.
+ *
+ * @since 3.9.0
+ */
+class ReverseTreeRepositoryListener extends AbstractRepositoryListener {
+    @Override
+    public void artifactResolved(RepositoryEvent event) {
+        requireNonNull(event, "event cannot be null");
+
+        if (!event.getArtifact()
+                .getFile()
+                .getPath()
+                .startsWith(event.getSession().getLocalRepository().getBasedir().getPath())) {
+            return; // reactor artifact
+        }
+        RequestTrace trace = event.getTrace();
+        CollectStepData collectStepTrace = null;

Review Comment:
   Class method (static) would need no instance for test. Like it doesn't to do its job.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] laeubi commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
laeubi commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336328111

   Not sure if this was already mentioned, but instead of "bake this in" would it not be better to have a way for (core-)plugins to hook in a `RepositoryListener` or even on the project level as we already to for `AbstractMavenLifeCycleListeners` / `WorkspaceReaders`? The this can become part of m-dependency-p for example.
   
   I also wonder if it is even required to record the data at all, should it not be sufficient to first load the pom(s), then generate a list of used plugins and then resolve their dependencies and then do something similar to `dependency:tree` (maybe plugin-dependency-tree)? To make it more convenient then one might pass a `-DdependencyReverse=<GAV>` to have a reverse output rooted at the given GAV when build finished.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] cstamas merged pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
cstamas merged PR #900:
URL: https://github.com/apache/maven/pull/900


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] pzygielo commented on a diff in pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
pzygielo commented on code in PR #900:
URL: https://github.com/apache/maven/pull/900#discussion_r1038946491


##########
maven-core/src/main/java/org/apache/maven/internal/aether/ReverseTreeRepositoryListener.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+package org.apache.maven.internal.aether;
+
+import static java.util.Objects.requireNonNull;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.ListIterator;
+import java.util.Objects;
+import org.eclipse.aether.AbstractRepositoryListener;
+import org.eclipse.aether.RepositoryEvent;
+import org.eclipse.aether.RequestTrace;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.collection.CollectStepData;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * A class building reverse tree using {@link CollectStepData} trace data provided in {@link RepositoryEvent}
+ * events fired during collection.
+ *
+ * @since 3.9.0
+ */
+class ReverseTreeRepositoryListener extends AbstractRepositoryListener {
+    @Override
+    public void artifactResolved(RepositoryEvent event) {
+        requireNonNull(event, "event cannot be null");
+
+        if (!event.getArtifact()
+                .getFile()
+                .getPath()
+                .startsWith(event.getSession().getLocalRepository().getBasedir().getPath())) {
+            return; // reactor artifact

Review Comment:
   Could this condition be extracted to method IDK, `isReactorArtifact(event)`?
   The comment then removed?
   The method tested?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] michael-o commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336266571

   > Thats org.eclipse.aether.graph.Dependency#toString, we could fix it, but given this is "advanced" feature, I don't think is something would block this?
   
   Agree, we need to spin off the issue there. Here, no change is required.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] cstamas commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336263333

   Note: this is based on by @grgrzybek proposal and reuses some code he provided. See https://github.com/apache/maven-resolver/pull/182


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] pzygielo commented on a diff in pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
pzygielo commented on code in PR #900:
URL: https://github.com/apache/maven/pull/900#discussion_r1038946944


##########
maven-core/src/main/java/org/apache/maven/internal/aether/ReverseTreeRepositoryListener.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+package org.apache.maven.internal.aether;
+
+import static java.util.Objects.requireNonNull;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.ListIterator;
+import java.util.Objects;
+import org.eclipse.aether.AbstractRepositoryListener;
+import org.eclipse.aether.RepositoryEvent;
+import org.eclipse.aether.RequestTrace;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.collection.CollectStepData;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * A class building reverse tree using {@link CollectStepData} trace data provided in {@link RepositoryEvent}
+ * events fired during collection.
+ *
+ * @since 3.9.0
+ */
+class ReverseTreeRepositoryListener extends AbstractRepositoryListener {
+    @Override
+    public void artifactResolved(RepositoryEvent event) {
+        requireNonNull(event, "event cannot be null");
+
+        if (!event.getArtifact()
+                .getFile()
+                .getPath()
+                .startsWith(event.getSession().getLocalRepository().getBasedir().getPath())) {
+            return; // reactor artifact
+        }
+        RequestTrace trace = event.getTrace();
+        CollectStepData collectStepTrace = null;

Review Comment:
   Could this variable be moved one line up, and the loop then extracted to (tested) class method?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] cstamas commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336402839

   Um, unsure why GH says "removed request" from @pzygielo , I actually asked both @michael-o and @pzygielo for repeated review...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] michael-o commented on a diff in pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #900:
URL: https://github.com/apache/maven/pull/900#discussion_r1038874052


##########
maven-core/src/main/java/org/apache/maven/internal/aether/ReverseTreeRepositoryListener.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+package org.apache.maven.internal.aether;
+
+import static java.util.Objects.requireNonNull;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ListIterator;
+import java.util.Objects;
+import org.eclipse.aether.AbstractRepositoryListener;
+import org.eclipse.aether.RepositoryEvent;
+import org.eclipse.aether.RequestTrace;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.collection.CollectStepData;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * A class building reverse tree using {@link CollectStepData} trace data provided in {@link RepositoryEvent}
+ * events fired during collection.
+ *
+ * @since 3.9.0
+ */
+class ReverseTreeRepositoryListener extends AbstractRepositoryListener {
+    private static final String EOL = System.lineSeparator();

Review Comment:
   LS, not EOL



##########
maven-core/src/main/java/org/apache/maven/internal/aether/ReverseTreeRepositoryListener.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+package org.apache.maven.internal.aether;
+
+import static java.util.Objects.requireNonNull;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ListIterator;
+import java.util.Objects;
+import org.eclipse.aether.AbstractRepositoryListener;
+import org.eclipse.aether.RepositoryEvent;
+import org.eclipse.aether.RequestTrace;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.collection.CollectStepData;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * A class building reverse tree using {@link CollectStepData} trace data provided in {@link RepositoryEvent}
+ * events fired during collection.
+ *
+ * @since 3.9.0
+ */
+class ReverseTreeRepositoryListener extends AbstractRepositoryListener {
+    private static final String EOL = System.lineSeparator();
+
+    @Override
+    public void artifactResolved(RepositoryEvent event) {
+        requireNonNull(event, "event cannot be null");
+
+        if (!event.getArtifact()
+                .getFile()
+                .getPath()
+                .startsWith(event.getSession().getLocalRepository().getBasedir().getPath())) {
+            return; // reactor artifact
+        }
+        RequestTrace trace = event.getTrace();
+        CollectStepData collectStepTrace = null;
+        while (trace != null) {
+            if (trace.getData() instanceof CollectStepData) {
+                collectStepTrace = (CollectStepData) trace.getData();
+                break;
+            }
+            trace = trace.getParent();
+        }
+
+        if (collectStepTrace == null) {
+            return;
+        }
+
+        Artifact resolvedArtifact = event.getArtifact();
+        Artifact nodeArtifact = collectStepTrace.getNode().getArtifact();
+
+        if (isInScope(resolvedArtifact, nodeArtifact)) {
+            Dependency node = collectStepTrace.getNode();
+            String trackingData = node + " (" + collectStepTrace.getContext() + ")" + EOL;
+            String indent = "";
+            ListIterator<DependencyNode> iter = collectStepTrace
+                    .getPath()
+                    .listIterator(collectStepTrace.getPath().size());
+            while (iter.hasPrevious()) {
+                DependencyNode curr = iter.previous();
+                indent += "  ";
+                trackingData += indent + curr + " (" + collectStepTrace.getContext() + ")" + EOL;
+            }

Review Comment:
   For cases with a loop a string builder is advised.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven] cstamas commented on pull request #900: [MNG-7619] Reverse Dependency Tree

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #900:
URL: https://github.com/apache/maven/pull/900#issuecomment-1336266075

   Thats org.eclipse.aether.graph.Dependency#toString, we could fix it, but given this is "advanced" feature, I don't think is something would block this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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