Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NullPointerException when using plugin dependencies in version 1.6.0 #77

Conversation

jonasrutishauser
Copy link
Contributor

Fixes #76

A regression was introduced with the following change:
exec mojo can run executableDependency instead of executable (7684096)

@masu1402
Copy link

When is a new version of the API with this fix to be expected?

@rfscholte
Copy link
Member

There are conflicts with this PR, so you need to update your patch first. Be aware that we don't like protected fields. Also, this might already be solved with #122 maybe you can verify that first.

@jonasrutishauser jonasrutishauser force-pushed the bugfix/NPE-with-includePluginDependencies branch from 43d3164 to 320bafd Compare February 22, 2019 20:52
@jonasrutishauser
Copy link
Contributor Author

I've updated the patch as it is still needed.
The protected field is now replaced with a protected getter and an IllegalArgumentException introduced with #122 is fixed too.

@mobogojo-zz
Copy link

@jonasrutishauser @rfscholte - I just verified this change locally in a separate project, the change is required because the pluginDependencies field is duplicated. If we reference pluginDependencies in AbstractExecMojo it will always be null and the direct casts to hashmaps create null pointers. Leaving the field private means we need a get method in AbstractExecMojo otherwise we can't get the pluginDependencies in ExecJavaMojo. This alone gets you further but the filter can't be null and that is the last change. It would be interesting to understand why this wasn't reproduced in a sample project/test, I didn't investigate that far.

@mobogojo-zz
Copy link

mobogojo-zz commented Sep 18, 2019

I've found an additional edge case and bug: in the case where this is used in a multi module project

This returns a different result:
MavenProject executableProject =
this.projectBuilder.build(executablePomArtifact, buildingRequest).getProject()

so if you execute from the sub pom it works, if you execute from the root pom your executable will be missing and it will fail all the time.

this method: determineRelevantPluginDependencies

relevantDependencies = this.resolveExecutableDependencies(executableArtifact);
+relevantDependencies.add(executableArtifact);

@starksm64
Copy link

Three changes from this PR allowed me to build the plugin locally and use with the includePluginDependencies setting.

diff --git a/src/main/java/org/codehaus/mojo/exec/AbstractExecMojo.java b/src/main/java/org/codehaus/mojo/exec/AbstractExecMojo.java
index bd926e1..fdbe0ae 100644
--- a/src/main/java/org/codehaus/mojo/exec/AbstractExecMojo.java
+++ b/src/main/java/org/codehaus/mojo/exec/AbstractExecMojo.java
@@ -262,6 +262,11 @@ public abstract class AbstractExecMojo
         return session;
     }
 
+    protected final List<Artifact> getPluginDependencies()
+    {
+        return pluginDependencies;
+    }
+
     /**
      * Examine the plugin dependencies to find the executable artifact.
      * 
diff --git a/src/main/java/org/codehaus/mojo/exec/ExecJavaMojo.java b/src/main/java/org/codehaus/mojo/exec/ExecJavaMojo.java
index 7b47684..b6ace87 100644
--- a/src/main/java/org/codehaus/mojo/exec/ExecJavaMojo.java
+++ b/src/main/java/org/codehaus/mojo/exec/ExecJavaMojo.java
@@ -28,6 +28,8 @@ import org.apache.maven.plugins.annotations.ResolutionScope;
 import org.apache.maven.project.MavenProject;
 import org.apache.maven.project.ProjectBuilder;
 import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.shared.artifact.filter.resolve.AndFilter;
+import org.apache.maven.shared.artifact.filter.resolve.TransformableFilter;
 import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResult;
 import org.apache.maven.shared.transfer.dependencies.DefaultDependableCoordinate;
 import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolver;
@@ -52,12 +54,6 @@ public class ExecJavaMojo
     @Component
     private ProjectBuilder projectBuilder;
 
-    /**
-     * @since 1.1-beta-1
-     */
-    @Parameter( readonly = true, defaultValue = "${plugin.artifacts}" )
-    private List<Artifact> pluginDependencies;
-
     /**
      * The main class to execute.<br>
      * With Java 9 and above you can prefix it with the modulename, e.g. <code>com.greetings/com.greetings.Main</code>
@@ -625,7 +621,7 @@ public class ExecJavaMojo
             if ( this.executableDependency == null )
             {
                 getLog().debug( "All Plugin Dependencies will be included." );
-                relevantDependencies = new HashSet<Artifact>( this.pluginDependencies );
+                relevantDependencies = new HashSet<Artifact>( getPluginDependencies() );
             }
             else
             {
@@ -661,7 +657,9 @@ public class ExecJavaMojo
             MavenProject executableProject =
                 this.projectBuilder.build( executablePomArtifact, buildingRequest ).getProject();
 
-            for ( ArtifactResult artifactResult : dependencyResolver.resolveDependencies( buildingRequest, executableProject.getModel(), null ) )
+            for ( ArtifactResult artifactResult : dependencyResolver.resolveDependencies( buildingRequest,
+                                                                                          executableProject.getModel(),
+                                                                                          new AndFilter(Collections.<TransformableFilter>emptyList() ) ) )
             {
                 executableDependencies.add( artifactResult.getArtifact() );
             }

1.6.0

Fixes mojohaus#76
A regression was introduced with the following change:
exec mojo can run executableDependency instead of executable (7684096)
@slawekjaranowski slawekjaranowski force-pushed the bugfix/NPE-with-includePluginDependencies branch from 320bafd to dc9bdc6 Compare July 12, 2022 21:36
@slawekjaranowski slawekjaranowski merged commit b595f69 into mojohaus:master Jul 12, 2022
@slawekjaranowski
Copy link
Member

Thanks for PR and for patience 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullPointerException with inlcudePluginDependecies=true
6 participants