From 06d10f9f0a5041e0dc667320b032e3963df91717 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 09:21:44 +0200 Subject: [PATCH 01/11] Install At End feature (no extension) This PR makes installAtEnd work as expected even if maven-install-plugin is not used as extension. How it works: it uses mojo Context to store "markers": * presence of marker means project was "processed" * value of marker tells what should be done (true = install, false = skipped) UTs adjusted to provide plugin context (was null before). --- pom.xml | 41 +++++++++-- .../maven/plugins/install/InstallMojo.java | 72 +++++++++++-------- .../plugins/install/InstallFileMojoTest.java | 20 ++++-- .../plugins/install/InstallMojoTest.java | 24 +++++-- 4 files changed, 108 insertions(+), 49 deletions(-) diff --git a/pom.xml b/pom.xml index ca7b8764..ba7ae25e 100644 --- a/pom.xml +++ b/pom.xml @@ -63,8 +63,9 @@ - 3.0 - 7 + 3.2.5 + 1.7.32 + 8 2020-04-07T21:04:00Z @@ -73,13 +74,22 @@ org.apache.maven maven-plugin-api ${mavenVersion} + provided org.apache.maven maven-artifact ${mavenVersion} + provided + + + org.apache.maven + maven-core + ${mavenVersion} + provided + org.apache.maven.shared maven-artifact-transfer @@ -96,13 +106,13 @@ junit junit - 4.13.1 + 4.13.2 test org.apache.maven.plugin-testing maven-plugin-testing-harness - 2.1 + 3.3.0 test @@ -120,13 +130,31 @@ org.slf4j slf4j-api - 1.7.30 + ${slf4jVersion} provided org.slf4j slf4j-nop - 1.7.30 + ${slf4jVersion} + test + + + org.eclipse.aether + aether-api + 1.1.0 + test + + + org.eclipse.aether + aether-util + 1.1.0 + test + + + org.eclipse.aether + aether-impl + 1.1.0 test @@ -150,6 +178,7 @@ org.apache.maven.plugins maven-invoker-plugin + 3.2.2 ${project.build.directory}/it true diff --git a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java index b01c1141..080bafa5 100644 --- a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java +++ b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java @@ -20,19 +20,18 @@ */ import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.Map; +import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; +import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.LifecyclePhase; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; import org.apache.maven.project.MavenProject; -import org.apache.maven.project.ProjectBuildingRequest; import org.apache.maven.shared.transfer.artifact.install.ArtifactInstallerException; import org.apache.maven.shared.transfer.project.NoFileAssignedException; import org.apache.maven.shared.transfer.project.install.ProjectInstaller; @@ -48,24 +47,20 @@ public class InstallMojo extends AbstractInstallMojo { + private static final String INSTALL_PROCESSED_MARKER = InstallMojo.class.getName() + ".processed"; - /** - * When building with multiple threads, reaching the last project doesn't have to mean that all projects are ready - * to be installed - */ - private static final AtomicInteger READYPROJECTSCOUNTER = new AtomicInteger(); - - private static final List INSTALLREQUESTS = - Collections.synchronizedList( new ArrayList() ); - - /** - */ @Parameter( defaultValue = "${project}", readonly = true, required = true ) private MavenProject project; @Parameter( defaultValue = "${reactorProjects}", required = true, readonly = true ) private List reactorProjects; + @Parameter( defaultValue = "${session}", required = true, readonly = true ) + private MavenSession session; + + @Parameter( defaultValue = "${plugin}", required = true, readonly = true ) + private PluginDescriptor pluginDescriptor; + /** * Whether every project should be installed during its own install-phase or at the end of the multimodule build. If * set to {@code true} and the build fails, none of the reactor projects is installed. @@ -91,53 +86,68 @@ public class InstallMojo public void execute() throws MojoExecutionException, MojoFailureException { + + final String projectKey = project.getGroupId() + ":" + project.getArtifactId() + ":" + project.getVersion(); boolean addedInstallRequest = false; if ( skip ) { + getPluginContext().put( INSTALL_PROCESSED_MARKER, Boolean.FALSE ); getLog().info( "Skipping artifact installation" ); } else { - // CHECKSTYLE_OFF: LineLength - ProjectInstallerRequest projectInstallerRequest = - new ProjectInstallerRequest().setProject( project ); - // CHECKSTYLE_ON: LineLength - if ( !installAtEnd ) { - installProject( session.getProjectBuildingRequest(), projectInstallerRequest ); + installProject( project ); } else { - INSTALLREQUESTS.add( projectInstallerRequest ); + getPluginContext().put( INSTALL_PROCESSED_MARKER, Boolean.TRUE ); addedInstallRequest = true; } } - boolean projectsReady = READYPROJECTSCOUNTER.incrementAndGet() == reactorProjects.size(); - if ( projectsReady ) + if ( allProjectsMarked() ) { - synchronized ( INSTALLREQUESTS ) + for ( MavenProject reactorProject : reactorProjects ) { - while ( !INSTALLREQUESTS.isEmpty() ) + Map pluginContext = session.getPluginContext( pluginDescriptor, reactorProject ); + Boolean install = (Boolean) pluginContext.get( INSTALL_PROCESSED_MARKER ); + if ( !install ) { - installProject( session.getProjectBuildingRequest(), INSTALLREQUESTS.remove( 0 ) ); + getLog().info( "Project " + projectKey + " skipped install" ); + } + else + { + installProject( reactorProject ); } } } else if ( addedInstallRequest ) { - getLog().info( "Installing " + project.getGroupId() + ":" + project.getArtifactId() + ":" - + project.getVersion() + " at end" ); + getLog().info( "Installing " + projectKey + " at end" ); + } + } + + private boolean allProjectsMarked() + { + for ( MavenProject reactorProject : reactorProjects ) + { + Map pluginContext = session.getPluginContext( pluginDescriptor, reactorProject ); + if ( !pluginContext.containsKey( INSTALL_PROCESSED_MARKER ) ) + { + return false; + } } + return true; } - private void installProject( ProjectBuildingRequest pbr, ProjectInstallerRequest pir ) + private void installProject( MavenProject pir ) throws MojoFailureException, MojoExecutionException { try { - installer.install( session.getProjectBuildingRequest(), pir ); + installer.install( session.getProjectBuildingRequest(), new ProjectInstallerRequest().setProject( pir ) ); } catch ( IOException e ) { diff --git a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java index 8b3b070b..d04b1b14 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java @@ -21,18 +21,20 @@ import java.io.File; import java.io.Reader; +import java.util.concurrent.ConcurrentHashMap; import org.apache.maven.execution.MavenSession; import org.apache.maven.model.Model; import org.apache.maven.model.io.xpp3.MavenXpp3Reader; import org.apache.maven.plugin.testing.AbstractMojoTestCase; -import org.apache.maven.plugins.install.InstallFileMojo; import org.apache.maven.project.DefaultProjectBuildingRequest; import org.apache.maven.project.ProjectBuildingRequest; import org.apache.maven.shared.utils.ReaderFactory; import org.apache.maven.shared.utils.io.FileUtils; -import org.sonatype.aether.impl.internal.EnhancedLocalRepositoryManager; -import org.sonatype.aether.util.DefaultRepositorySystemSession; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.internal.impl.EnhancedLocalRepositoryManagerFactory; +import org.eclipse.aether.repository.LocalRepository; +import org.eclipse.aether.repository.NoLocalRepositoryManagerException; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -215,7 +217,9 @@ public void testInstallFileWithPomAsPackaging() InstallFileMojo mojo = (InstallFileMojo) lookupMojo( "install-file", testPom ); assertNotNull( mojo ); - + + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); + setVariableValueToObject( mojo, "session", createMavenSession() ); assignValuesForParameter( mojo ); @@ -243,7 +247,9 @@ public void testInstallFile() InstallFileMojo mojo = (InstallFileMojo) lookupMojo( "install-file", testPom ); assertNotNull( mojo ); - + + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); + setVariableValueToObject( mojo, "session", createMavenSession() ); assignValuesForParameter( mojo ); @@ -281,11 +287,11 @@ private String dotToSlashReplacer( String parameter ) return parameter.replace( '.', '/' ); } - private MavenSession createMavenSession() + private MavenSession createMavenSession() throws NoLocalRepositoryManagerException { MavenSession session = mock( MavenSession.class ); DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManager( new File( LOCAL_REPO ) ) ); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); ProjectBuildingRequest buildingRequest = new DefaultProjectBuildingRequest(); buildingRequest.setRepositorySession( repositorySession ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); diff --git a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java index 63ac6fa9..6abd82bf 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java @@ -19,17 +19,20 @@ * under the License. */ +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.io.File; import java.util.Collections; import java.util.List; +import java.util.concurrent.ConcurrentHashMap; import org.apache.maven.artifact.Artifact; import org.apache.maven.artifact.metadata.ArtifactMetadata; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugin.testing.AbstractMojoTestCase; import org.apache.maven.plugins.install.stubs.AttachedArtifactStub0; import org.apache.maven.plugins.install.stubs.InstallArtifactStub; @@ -38,8 +41,10 @@ import org.apache.maven.project.ProjectBuildingRequest; import org.apache.maven.shared.transfer.repository.RepositoryManager; import org.apache.maven.shared.utils.io.FileUtils; -import org.sonatype.aether.impl.internal.EnhancedLocalRepositoryManager; -import org.sonatype.aether.util.DefaultRepositorySystemSession; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.internal.impl.EnhancedLocalRepositoryManagerFactory; +import org.eclipse.aether.repository.LocalRepository; +import org.eclipse.aether.repository.NoLocalRepositoryManagerException; /** * @author Allan Ramirez @@ -85,7 +90,8 @@ public void testBasicInstall() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); - + + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -118,6 +124,7 @@ public void testBasicInstallWithAttachedArtifacts() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -160,6 +167,7 @@ public void testUpdateReleaseParamSetToTrue() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -186,6 +194,7 @@ public void testInstallIfArtifactFileIsNull() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -222,6 +231,7 @@ public void testInstallIfPackagingIsPom() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -258,6 +268,7 @@ public void testBasicInstallAndCreate() MavenSession mavenSession = createMavenSession(); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", mavenSession ); @@ -314,6 +325,8 @@ public void testSkip() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); + setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -343,14 +356,15 @@ private String dotToSlashReplacer( String parameter ) return parameter.replace( '.', '/' ); } - private MavenSession createMavenSession() + private MavenSession createMavenSession() throws NoLocalRepositoryManagerException { MavenSession session = mock( MavenSession.class ); DefaultRepositorySystemSession repositorySession = new DefaultRepositorySystemSession(); - repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManager( new File( LOCAL_REPO ) ) ); + repositorySession.setLocalRepositoryManager( new EnhancedLocalRepositoryManagerFactory().newInstance( repositorySession, new LocalRepository( LOCAL_REPO )) ); ProjectBuildingRequest buildingRequest = new DefaultProjectBuildingRequest(); buildingRequest.setRepositorySession( repositorySession ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); + when( session.getPluginContext(any(PluginDescriptor.class), any(MavenProject.class))).thenReturn( new ConcurrentHashMap<>() ); return session; } From c08a7364b814e4187ca8e845de9d7b0bcdc69483 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 09:25:13 +0200 Subject: [PATCH 02/11] Remove unneeded change --- .../org/apache/maven/plugins/install/InstallFileMojoTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java index d04b1b14..de0a32ac 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java @@ -218,8 +218,6 @@ public void testInstallFileWithPomAsPackaging() assertNotNull( mojo ); - setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); - setVariableValueToObject( mojo, "session", createMavenSession() ); assignValuesForParameter( mojo ); @@ -248,8 +246,6 @@ public void testInstallFile() assertNotNull( mojo ); - setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); - setVariableValueToObject( mojo, "session", createMavenSession() ); assignValuesForParameter( mojo ); From 4b03381f4a324ee62a1fed7718a91a5cb52982dc Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 10:25:21 +0200 Subject: [PATCH 03/11] Add aetherVersion property --- pom.xml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index ba7ae25e..dcb9adca 100644 --- a/pom.xml +++ b/pom.xml @@ -65,6 +65,7 @@ 3.2.5 1.7.32 + 1.1.0 8 2020-04-07T21:04:00Z @@ -142,19 +143,19 @@ org.eclipse.aether aether-api - 1.1.0 + ${aetherVersion} test org.eclipse.aether aether-util - 1.1.0 + ${aetherVersion} test org.eclipse.aether aether-impl - 1.1.0 + ${aetherVersion} test From 245f13509bb1dcefa9434e4534474bfa8bc05640 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 14:40:44 +0200 Subject: [PATCH 04/11] Go back to Java7 --- pom.xml | 3 ++- .../java/org/apache/maven/plugins/install/InstallMojoTest.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index dcb9adca..691b171c 100644 --- a/pom.xml +++ b/pom.xml @@ -66,7 +66,8 @@ 3.2.5 1.7.32 1.1.0 - 8 + 2.22.2 + 7 2020-04-07T21:04:00Z diff --git a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java index 6abd82bf..3d108191 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java @@ -364,7 +364,8 @@ private MavenSession createMavenSession() throws NoLocalRepositoryManagerExcepti ProjectBuildingRequest buildingRequest = new DefaultProjectBuildingRequest(); buildingRequest.setRepositorySession( repositorySession ); when( session.getProjectBuildingRequest() ).thenReturn( buildingRequest ); - when( session.getPluginContext(any(PluginDescriptor.class), any(MavenProject.class))).thenReturn( new ConcurrentHashMap<>() ); + when( session.getPluginContext(any(PluginDescriptor.class), any(MavenProject.class))) + .thenReturn( new ConcurrentHashMap() ); return session; } From f03c4bf2e62aa34fd83877e3e2f24c2a2022360f Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 19 Nov 2021 10:36:04 +0100 Subject: [PATCH 05/11] Fix keys and log message. --- .../apache/maven/plugins/install/InstallMojo.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java index 080bafa5..d662e143 100644 --- a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java +++ b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java @@ -86,8 +86,6 @@ public class InstallMojo public void execute() throws MojoExecutionException, MojoFailureException { - - final String projectKey = project.getGroupId() + ":" + project.getArtifactId() + ":" + project.getVersion(); boolean addedInstallRequest = false; if ( skip ) { @@ -115,7 +113,9 @@ public void execute() Boolean install = (Boolean) pluginContext.get( INSTALL_PROCESSED_MARKER ); if ( !install ) { - getLog().info( "Project " + projectKey + " skipped install" ); + getLog().info( + "Project " + getProjectReferenceId( reactorProject ) + " skipped install" + ); } else { @@ -125,10 +125,15 @@ public void execute() } else if ( addedInstallRequest ) { - getLog().info( "Installing " + projectKey + " at end" ); + getLog().info( "Installing " + getProjectReferenceId( project ) + " at end" ); } } + private String getProjectReferenceId( MavenProject mavenProject ) + { + return mavenProject.getGroupId() + ":" + mavenProject.getArtifactId() + ":" + mavenProject.getVersion(); + } + private boolean allProjectsMarked() { for ( MavenProject reactorProject : reactorProjects ) From 4cecdff9411a77d6645063f24e9cc623025ae723 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 12 Dec 2021 20:13:11 +0100 Subject: [PATCH 06/11] Format --- pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/pom.xml b/pom.xml index 691b171c..2595c924 100644 --- a/pom.xml +++ b/pom.xml @@ -84,7 +84,6 @@ ${mavenVersion} provided - org.apache.maven From af0323d751240a3ebbccd298dfa1dca5b8917b52 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 29 May 2022 20:38:37 +0200 Subject: [PATCH 07/11] Undo unrelated changes --- .../apache/maven/plugins/install/InstallFileMojoTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java index 5d587b7a..bf040b15 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java @@ -21,7 +21,6 @@ import java.io.File; import java.io.Reader; -import java.util.concurrent.ConcurrentHashMap; import org.apache.maven.execution.MavenSession; import org.apache.maven.model.Model; @@ -217,7 +216,7 @@ public void testInstallFileWithPomAsPackaging() InstallFileMojo mojo = (InstallFileMojo) lookupMojo( "install-file", testPom ); assertNotNull( mojo ); - + setVariableValueToObject( mojo, "session", createMavenSession() ); assignValuesForParameter( mojo ); @@ -245,7 +244,7 @@ public void testInstallFile() InstallFileMojo mojo = (InstallFileMojo) lookupMojo( "install-file", testPom ); assertNotNull( mojo ); - + setVariableValueToObject( mojo, "session", createMavenSession() ); assignValuesForParameter( mojo ); From 98065bf3662fc69f1aa8e49dfda9d29352cd2bc9 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 29 May 2022 21:23:09 +0200 Subject: [PATCH 08/11] Simplify --- .../java/org/apache/maven/plugins/install/InstallMojo.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java index d662e143..da3826d2 100644 --- a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java +++ b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java @@ -86,7 +86,6 @@ public class InstallMojo public void execute() throws MojoExecutionException, MojoFailureException { - boolean addedInstallRequest = false; if ( skip ) { getPluginContext().put( INSTALL_PROCESSED_MARKER, Boolean.FALSE ); @@ -101,7 +100,7 @@ public void execute() else { getPluginContext().put( INSTALL_PROCESSED_MARKER, Boolean.TRUE ); - addedInstallRequest = true; + getLog().info( "Installing " + getProjectReferenceId( project ) + " at end" ); } } @@ -123,10 +122,6 @@ public void execute() } } } - else if ( addedInstallRequest ) - { - getLog().info( "Installing " + getProjectReferenceId( project ) + " at end" ); - } } private String getProjectReferenceId( MavenProject mavenProject ) From d292de9c404d08a360c7d96267a5a0ea8e50b918 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 29 Jun 2022 15:01:24 +0200 Subject: [PATCH 09/11] Better cover all possible cases, be clearer about intent Still, we MUST use String as these are spanning across projects, hence due classloader, there is no single State enum. --- .../maven/plugins/install/InstallMojo.java | 20 +++++++++---------- .../plugins/install/InstallMojoTest.java | 6 ++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java index da3826d2..7cd47a20 100644 --- a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java +++ b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java @@ -83,24 +83,30 @@ public class InstallMojo @Component private ProjectInstaller installer; + private enum State + { + SKIPPED, INSTALLED, TO_BE_INSTALLED + } + public void execute() throws MojoExecutionException, MojoFailureException { if ( skip ) { - getPluginContext().put( INSTALL_PROCESSED_MARKER, Boolean.FALSE ); getLog().info( "Skipping artifact installation" ); + getPluginContext().put( INSTALL_PROCESSED_MARKER, State.SKIPPED.name() ); } else { if ( !installAtEnd ) { installProject( project ); + getPluginContext().put( INSTALL_PROCESSED_MARKER, State.INSTALLED.name() ); } else { - getPluginContext().put( INSTALL_PROCESSED_MARKER, Boolean.TRUE ); getLog().info( "Installing " + getProjectReferenceId( project ) + " at end" ); + getPluginContext().put( INSTALL_PROCESSED_MARKER, State.TO_BE_INSTALLED.name() ); } } @@ -109,14 +115,8 @@ public void execute() for ( MavenProject reactorProject : reactorProjects ) { Map pluginContext = session.getPluginContext( pluginDescriptor, reactorProject ); - Boolean install = (Boolean) pluginContext.get( INSTALL_PROCESSED_MARKER ); - if ( !install ) - { - getLog().info( - "Project " + getProjectReferenceId( reactorProject ) + " skipped install" - ); - } - else + State state = State.valueOf( (String) pluginContext.get( INSTALL_PROCESSED_MARKER ) ); + if ( state == State.TO_BE_INSTALLED ) { installProject( reactorProject ); } diff --git a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java index a634ea2e..1347e4b0 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java @@ -91,6 +91,7 @@ public void testBasicInstall() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -124,6 +125,7 @@ public void testBasicInstallWithAttachedArtifacts() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -167,6 +169,7 @@ public void testUpdateReleaseParamSetToTrue() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -194,6 +197,7 @@ public void testInstallIfArtifactFileIsNull() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -231,6 +235,7 @@ public void testInstallIfPackagingIsPom() MavenProject project = (MavenProject) getVariableValueFromObject( mojo, "project" ); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", createMavenSession() ); @@ -268,6 +273,7 @@ public void testBasicInstallAndCreate() MavenSession mavenSession = createMavenSession(); updateMavenProject( project ); + setVariableValueToObject( mojo, "pluginContext", new ConcurrentHashMap<>() ); setVariableValueToObject( mojo, "pluginDescriptor", new PluginDescriptor() ); setVariableValueToObject( mojo, "reactorProjects", Collections.singletonList( project ) ); setVariableValueToObject( mojo, "session", mavenSession ); From b6cd186fb53b4558bffd913ab3c3ff37602e34f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Boutemy?= Date: Sat, 2 Jul 2022 11:30:12 +0200 Subject: [PATCH 10/11] create state management methods to clarify --- .../maven/plugins/install/InstallMojo.java | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java index 7cd47a20..8ea2f30f 100644 --- a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java +++ b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; -import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugin.descriptor.PluginDescriptor; @@ -47,17 +46,12 @@ public class InstallMojo extends AbstractInstallMojo { - private static final String INSTALL_PROCESSED_MARKER = InstallMojo.class.getName() + ".processed"; - @Parameter( defaultValue = "${project}", readonly = true, required = true ) private MavenProject project; @Parameter( defaultValue = "${reactorProjects}", required = true, readonly = true ) private List reactorProjects; - @Parameter( defaultValue = "${session}", required = true, readonly = true ) - private MavenSession session; - @Parameter( defaultValue = "${plugin}", required = true, readonly = true ) private PluginDescriptor pluginDescriptor; @@ -88,25 +82,44 @@ private enum State SKIPPED, INSTALLED, TO_BE_INSTALLED } + private static final String INSTALL_PROCESSED_MARKER = InstallMojo.class.getName() + ".processed"; + + private void putState( State state ) + { + getPluginContext().put( INSTALL_PROCESSED_MARKER, state.name() ); + } + + private State getState( MavenProject project ) + { + Map pluginContext = session.getPluginContext( pluginDescriptor, project ); + return State.valueOf( (String) pluginContext.get( INSTALL_PROCESSED_MARKER ) ); + } + + private boolean hasState( MavenProject project ) + { + Map pluginContext = session.getPluginContext( pluginDescriptor, project ); + return pluginContext.containsKey( INSTALL_PROCESSED_MARKER ); + } + public void execute() throws MojoExecutionException, MojoFailureException { if ( skip ) { getLog().info( "Skipping artifact installation" ); - getPluginContext().put( INSTALL_PROCESSED_MARKER, State.SKIPPED.name() ); + putState( State.SKIPPED ); } else { if ( !installAtEnd ) { installProject( project ); - getPluginContext().put( INSTALL_PROCESSED_MARKER, State.INSTALLED.name() ); + putState( State.INSTALLED ); } else { - getLog().info( "Installing " + getProjectReferenceId( project ) + " at end" ); - getPluginContext().put( INSTALL_PROCESSED_MARKER, State.TO_BE_INSTALLED.name() ); + getLog().info( "Deferring install for " + getProjectReferenceId( project ) + " at end" ); + putState( State.TO_BE_INSTALLED ); } } @@ -114,8 +127,7 @@ public void execute() { for ( MavenProject reactorProject : reactorProjects ) { - Map pluginContext = session.getPluginContext( pluginDescriptor, reactorProject ); - State state = State.valueOf( (String) pluginContext.get( INSTALL_PROCESSED_MARKER ) ); + State state = getState( reactorProject ); if ( state == State.TO_BE_INSTALLED ) { installProject( reactorProject ); @@ -133,8 +145,7 @@ private boolean allProjectsMarked() { for ( MavenProject reactorProject : reactorProjects ) { - Map pluginContext = session.getPluginContext( pluginDescriptor, reactorProject ); - if ( !pluginContext.containsKey( INSTALL_PROCESSED_MARKER ) ) + if ( !hasState( reactorProject ) ) { return false; } From 63aa94e4cfc10efe0cc62306d782635645caf54b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Boutemy?= Date: Sat, 2 Jul 2022 16:26:45 +0200 Subject: [PATCH 11/11] fix IT --- src/it/install-at-end-fail/verify.groovy | 2 +- src/it/install-at-end-pass/verify.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/it/install-at-end-fail/verify.groovy b/src/it/install-at-end-fail/verify.groovy index f35990b4..a2801f5f 100644 --- a/src/it/install-at-end-fail/verify.groovy +++ b/src/it/install-at-end-fail/verify.groovy @@ -22,5 +22,5 @@ assert !( new File( basedir, "../../local-repo/org/apache/maven/its/install/dae/ File buildLog = new File( basedir, 'build.log' ) assert buildLog.exists() -assert buildLog.text.contains( "[INFO] Installing org.apache.maven.its.install.dae.fail:dae:1.0 at end" ) +assert buildLog.text.contains( "[INFO] Deferring install for org.apache.maven.its.install.dae.fail:dae:1.0 at end" ) diff --git a/src/it/install-at-end-pass/verify.groovy b/src/it/install-at-end-pass/verify.groovy index 3487d52d..f1b7e513 100644 --- a/src/it/install-at-end-pass/verify.groovy +++ b/src/it/install-at-end-pass/verify.groovy @@ -22,5 +22,5 @@ assert new File( basedir, "../../local-repo/org/apache/maven/its/install/dae/pas File buildLog = new File( basedir, 'build.log' ) assert buildLog.exists() -assert buildLog.text.contains( "[INFO] Installing org.apache.maven.its.install.dae.pass:dae:1.0 at end" ) +assert buildLog.text.contains( "[INFO] Deferring install for org.apache.maven.its.install.dae.pass:dae:1.0 at end" )