[GitHub] [maven-deploy-plugin] fvanhovell-ing commented on a change in pull request #1: fix DeployAtEnd failures

[GitBox <gi...@apache.org>]

fvanhovell-ing commented on a change in pull request #1:
URL: https://github.com/apache/maven-deploy-plugin/pull/1#discussion_r556436305



##########
File path: src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java
##########
@@ -152,42 +141,93 @@ public void execute()
             // @formatter:on
             // CHECKSTYLE_ON: LineLength
 
-            ArtifactRepository repo = getDeploymentRepository( pdr );
-
             if ( !deployAtEnd )
             {
-                deployProject( getSession().getProjectBuildingRequest(), pdr, repo );
+                queueDeployment( null );
+                deployProject( pdr );
             }
             else
             {
-                DEPLOYREQUESTS.add( pdr );
-                addedDeployRequest = true;
+                queueDeployment( pdr );
             }
         }
+        deployQueuedRequests();
+    }
+
+    /**
+     * Queue a deployment request.  In a reactor, some deployments may be delayed or skipped while others are not
+     * @param pdr The deployment request.  Null indicates that the deployment was not delayed
+     * @throws MojoExecutionException
+     * @throws MojoFailureException
+     */
+    private void queueDeployment( ProjectDeployerRequest pdr )
+        throws MojoExecutionException, MojoFailureException
+    {
+        List<ProjectDeployerRequest> deployRequests = getDeployRequests();
 
-        boolean projectsReady = READYPROJECTSCOUNTER.incrementAndGet() == reactorProjects.size();
-        if ( projectsReady )
+        // build may be parallel, protect against multiple threads accessing
+        synchronized ( deployRequests )
         {
-            synchronized ( DEPLOYREQUESTS )
+
+            deployRequests.add( pdr );
+            if ( pdr != null && deployRequests.size() != reactorProjects.size() )
             {
-                while ( !DEPLOYREQUESTS.isEmpty() )
-                {
-                    ArtifactRepository repo = getDeploymentRepository( DEPLOYREQUESTS.get( 0 ) );
+                getLog().info( "Deploying " + project.getGroupId() + ":" + project.getArtifactId() + ":"
+                  + project.getVersion() + " at end" );
+            }
+        }
+    }
 
-                    deployProject( getSession().getProjectBuildingRequest(), DEPLOYREQUESTS.remove( 0 ), repo );
+    public void deployQueuedRequests() throws MojoExecutionException, MojoFailureException
+    {
+        List deployRequests = getDeployRequests();
+        // build may be parallel, protect against multiple threads accessing
+        synchronized ( deployRequests )
+        {
+            if ( deployRequests.size() != reactorProjects.size() )
+            {
+                return;
+            }
+            for ( Object dr : deployRequests )
+            {
+                if ( dr != null )
+                {
+                    /*
+                     * Cast the instance to a ProjectDeployerRequest.  This specialized casting would
+                     * not be necessary if ProjectDeployerRequest were in core.
+                     */
+                    deployProject( CastHelper.castToSameClassLoader( ProjectDeployerRequest.class, dr ) );
                 }
             }
         }
-        else if ( addedDeployRequest )
+    }
+
+    private List<ProjectDeployerRequest> getDeployRequests()
+    {
+        Properties projectProperties = getSession().getUserProperties();
+
+        // Plugin instances may be in different Classworlds if they are loaded in different modules
+        // containing different extensions.  The plugin cannot rely on static variables, only injected
+        // or session shared variables
+        synchronized ( projectProperties )
         {
-            getLog().info( "Deploying " + project.getGroupId() + ":" + project.getArtifactId() + ":"
-                + project.getVersion() + " at end" );
+            String propertyKey = getClass().getCanonicalName();
+            List<ProjectDeployerRequest> reqs = (List<ProjectDeployerRequest>) projectProperties.get( propertyKey );
+            if ( reqs == null )
+            {
+                reqs = new ArrayList<ProjectDeployerRequest>( reactorProjects.size() );
+                projectProperties.put( propertyKey, reqs );

Review comment:
       How about serializing the value `reqs` here (and deserialize on line 215)




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