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 8d32542..1eda589 100644 --- a/src/main/java/org/apache/maven/plugins/install/InstallMojo.java +++ b/src/main/java/org/apache/maven/plugins/install/InstallMojo.java @@ -19,14 +19,13 @@ package org.apache.maven.plugins.install; import java.nio.file.Files; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.stream.Collectors; import org.apache.maven.api.Artifact; -import org.apache.maven.api.MojoExecution; import org.apache.maven.api.ProducedArtifact; import org.apache.maven.api.Project; import org.apache.maven.api.Session; @@ -57,12 +56,11 @@ public class InstallMojo implements org.apache.maven.api.plugin.Mojo { @Inject private Project project; - @Inject - private MojoExecution mojoExecution; - /** * 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. + * set to {@code true} and the build fails before the deferred installation has started, none of the reactor + * projects is installed. If a failure occurs during the deferred installation itself, an explicit inventory of the + * projects already installed and those skipped is logged. * (experimental) * * @since 2.5 @@ -100,6 +98,14 @@ private enum State { private static final String INSTALL_PROCESSED_MARKER = InstallMojo.class.getName() + ".processed"; private static final String PROJECTS_USING_PLUGIN_KEY = InstallMojo.class.getName() + ".projectsUsingPlugin"; + /** + * Guards the mark-check-fire sequence of the deferred ({@code installAtEnd}) install: without it, two + * modules finishing simultaneously in a parallel build ({@code -T}) can both observe "all projects + * marked" and each run the full deferred-install loop. The plugin classloader (and therefore this lock) + * is shared across all reactor threads of a build. + */ + private static final Object DEFERRED_INSTALL_LOCK = new Object(); + public InstallMojo() {} private void putState(State state) { @@ -111,6 +117,10 @@ private void putState(State state, ArtifactInstallerRequest request) { session.getPluginContext(project).put(ArtifactInstallerRequest.class.getName(), request); } + private void putState(Project project, State state) { + session.getPluginContext(project).put(INSTALL_PROCESSED_MARKER, state.name()); + } + private State getState(Project project) { Map pluginContext = session.getPluginContext(project); return State.valueOf((String) pluginContext.get(INSTALL_PROCESSED_MARKER)); @@ -138,12 +148,16 @@ private List getProjectsUsingPlugin() { k -> allProjects.stream().filter(this::usingPlugin).collect(Collectors.toList())); } + /** + * Whether the given reactor project takes part in the deferred install. Grouping is by plugin presence + * (any execution not bound to phase {@code none}), not by execution-id equality: matching only the current + * execution's id would let a module binding the goal under a custom id observe a singleton project set, + * trivially satisfy {@link #allProjectsMarked(List)}, and install mid-build, breaking the all-or-nothing + * contract documented on {@link #installAtEnd}. + */ private boolean usingPlugin(Project project) { Plugin plugin = project.getBuild().getPluginsAsMap().get("org.apache.maven.plugins:maven-install-plugin"); - return plugin != null - && plugin.getExecutions().stream() - .anyMatch(e -> Objects.equals(e.getId(), mojoExecution.getExecutionId()) - && !"none".equals(e.getPhase())); + return plugin != null && plugin.getExecutions().stream().anyMatch(e -> !"none".equals(e.getPhase())); } @Override @@ -163,19 +177,55 @@ public void execute() { } List projectsUsingPlugin = getProjectsUsingPlugin(); - if (allProjectsMarked(projectsUsingPlugin)) { - for (Project reactorProject : projectsUsingPlugin) { - State state = getState(reactorProject); - if (state == State.TO_BE_INSTALLED) { - Map pluginContext = session.getPluginContext(reactorProject); - ArtifactInstallerRequest request = - (ArtifactInstallerRequest) pluginContext.get(ArtifactInstallerRequest.class.getName()); - installProject(request); + synchronized (DEFERRED_INSTALL_LOCK) { + if (allProjectsMarked(projectsUsingPlugin)) { + List installedProjects = new ArrayList<>(); + for (Project reactorProject : projectsUsingPlugin) { + State state = getState(reactorProject); + if (state == State.TO_BE_INSTALLED) { + Map pluginContext = session.getPluginContext(reactorProject); + ArtifactInstallerRequest request = + (ArtifactInstallerRequest) pluginContext.get(ArtifactInstallerRequest.class.getName()); + try { + installProject(request); + } catch (MojoException e) { + logPartialInstallInventory(projectsUsingPlugin, installedProjects, reactorProject); + throw e; + } + installedProjects.add(reactorProject); + // exactly-once: transition state so a concurrent or repeated trigger (parallel + // build, or a second execution of the goal in the same session) skips completed work + putState(reactorProject, State.INSTALLED); + } } } } } + /** + * The contract documented on {@link #installAtEnd} is all-or-nothing; when a deferred install fails + * mid-loop that contract can no longer be met, so leave an explicit inventory of which projects already + * reached the local repository and which were skipped, instead of failing silently into a mixed state. + */ + private void logPartialInstallInventory( + List projectsUsingPlugin, List installedProjects, Project failedProject) { + getLog().error("Failed to install " + gav(failedProject) + + "; the local repository is in a partially installed state:"); + for (Project reactorProject : projectsUsingPlugin) { + if (installedProjects.contains(reactorProject)) { + getLog().error(" installed: " + gav(reactorProject)); + } else if (reactorProject == failedProject) { + getLog().error(" failed: " + gav(reactorProject)); + } else if (getState(reactorProject) == State.TO_BE_INSTALLED) { + getLog().error(" not installed: " + gav(reactorProject)); + } + } + } + + private static String gav(Project project) { + return project.getGroupId() + ":" + project.getArtifactId() + ":" + project.getVersion(); + } + private boolean allProjectsMarked(List projectsUsingPlugin) { return projectsUsingPlugin.stream().allMatch(this::hasState); } 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 59e43ae..df6a474 100644 --- a/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java @@ -19,11 +19,14 @@ package org.apache.maven.plugins.install; import java.io.File; +import java.lang.reflect.Method; import java.nio.file.Paths; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import org.apache.maven.api.Artifact; @@ -35,6 +38,10 @@ import org.apache.maven.api.di.Priority; import org.apache.maven.api.di.Provides; import org.apache.maven.api.di.Singleton; +import org.apache.maven.api.model.Build; +import org.apache.maven.api.model.Plugin; +import org.apache.maven.api.model.PluginExecution; +import org.apache.maven.api.plugin.Log; import org.apache.maven.api.plugin.Mojo; import org.apache.maven.api.plugin.MojoException; import org.apache.maven.api.plugin.testing.InjectMojo; @@ -61,8 +68,15 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @MojoTest @@ -161,6 +175,109 @@ void skip(InstallMojo mojo) throws Exception { assertNull(execute(mojo)); } + @Test + @InjectMojo(goal = "install") + void usingPluginMatchesAnyExecutionId(InstallMojo mojo) throws Exception { + assertNotNull(mojo); + // a module binding the goal under a custom execution id must not observe a singleton + // project set and install mid-build (broken all-or-nothing contract) + assertTrue(invokeUsingPlugin(mojo, projectUsingInstallPlugin("custom-install-id", "install"))); + } + + @Test + @InjectMojo(goal = "install") + void usingPluginIgnoresExecutionsBoundToNone(InstallMojo mojo) throws Exception { + assertNotNull(mojo); + assertFalse(invokeUsingPlugin(mojo, projectUsingInstallPlugin("default-install", "none"))); + } + + @Test + @InjectMojo(goal = "install") + void midLoopFailureLogsPartialInstallInventory(InstallMojo mojo) throws Exception { + assertNotNull(mojo); + setVariableValueToObject(mojo, "session", session); + Log log = mock(Log.class); + setVariableValueToObject(mojo, "log", log); + mojo.setSkip(true); + + Project moduleA = reactorProject("module-a"); + Project moduleB = reactorProject("module-b"); + ArtifactInstallerRequest requestA = mock(ArtifactInstallerRequest.class); + ArtifactInstallerRequest requestB = mock(ArtifactInstallerRequest.class); + when(session.getProjects()).thenReturn(Arrays.asList(moduleA, moduleB)); + when(session.getPluginContext(moduleA)).thenReturn(deferredContext(requestA)); + when(session.getPluginContext(moduleB)).thenReturn(deferredContext(requestB)); + doThrow(new MojoException("install failed")).when(artifactInstaller).install(requestB); + + assertThrows(MojoException.class, mojo::execute); + + verify(log, atLeastOnce()).error(contains("partially installed state")); + verify(log).error(contains("installed: org.apache.maven.test:module-a:1.0-SNAPSHOT")); + verify(log).error(contains("failed: org.apache.maven.test:module-b:1.0-SNAPSHOT")); + } + + @Test + @InjectMojo(goal = "install") + void deferredInstallRunsExactlyOncePerProject(InstallMojo mojo) throws Exception { + assertNotNull(mojo); + setVariableValueToObject(mojo, "session", session); + setVariableValueToObject(mojo, "log", mock(Log.class)); + mojo.setSkip(true); + + Project moduleA = reactorProject("module-a"); + Project moduleB = reactorProject("module-b"); + when(session.getProjects()).thenReturn(Arrays.asList(moduleA, moduleB)); + when(session.getPluginContext(moduleA)).thenReturn(deferredContext(mock(ArtifactInstallerRequest.class))); + when(session.getPluginContext(moduleB)).thenReturn(deferredContext(mock(ArtifactInstallerRequest.class))); + + // first trigger runs the deferred loop for both projects... + mojo.execute(); + // ...a repeated trigger in the same session (e.g. `mvn install install`) must not re-install + mojo.execute(); + + verify(artifactInstaller, times(2)).install(any(ArtifactInstallerRequest.class)); + } + + private Project reactorProject(String artifactId) { + Project project = mock(Project.class); + when(project.getBuild()).thenReturn(installPluginBuild("default-install", "install")); + when(project.getGroupId()).thenReturn("org.apache.maven.test"); + when(project.getArtifactId()).thenReturn(artifactId); + when(project.getVersion()).thenReturn("1.0-SNAPSHOT"); + return project; + } + + private static Map deferredContext(ArtifactInstallerRequest request) { + Map pluginContext = new HashMap<>(); + pluginContext.put(InstallMojo.class.getName() + ".processed", "TO_BE_INSTALLED"); + pluginContext.put(ArtifactInstallerRequest.class.getName(), request); + return pluginContext; + } + + private static Project projectUsingInstallPlugin(String executionId, String phase) { + Project project = mock(Project.class); + when(project.getBuild()).thenReturn(installPluginBuild(executionId, phase)); + return project; + } + + private static Build installPluginBuild(String executionId, String phase) { + Plugin plugin = Plugin.newBuilder() + .groupId("org.apache.maven.plugins") + .artifactId("maven-install-plugin") + .executions(Collections.singletonList(PluginExecution.newBuilder() + .id(executionId) + .phase(phase) + .build())) + .build(); + return Build.newBuilder().plugins(Collections.singletonList(plugin)).build(); + } + + private static boolean invokeUsingPlugin(InstallMojo mojo, Project project) throws Exception { + Method usingPlugin = InstallMojo.class.getDeclaredMethod("usingPlugin", Project.class); + usingPlugin.setAccessible(true); + return (Boolean) usingPlugin.invoke(mojo, project); + } + @Provides @Singleton @Priority(10)