Skip to content

chore: harden installAtEnd integrity contracts #445

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.

Sign up for GitHub

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

Open
gnodet wants to merge 1 commit into
base: master
Choose a base branch
from
+185 −18
Open

chore: harden installAtEnd integrity contracts #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 68 additions & 18 deletions src/main/java/org/apache/maven/plugins/install/InstallMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
* <strong>(experimental)</strong>
*
* @since 2.5
Expand Down Expand Up @@ -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) {
Expand All @@ -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<String, Object> pluginContext = session.getPluginContext(project);
return State.valueOf((String) pluginContext.get(INSTALL_PROCESSED_MARKER));
Expand Down Expand Up @@ -138,12 +148,16 @@ private List<Project> 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
Expand All @@ -163,19 +177,55 @@ public void execute() {
}

List<Project> projectsUsingPlugin = getProjectsUsingPlugin();
if (allProjectsMarked(projectsUsingPlugin)) {
for (Project reactorProject : projectsUsingPlugin) {
State state = getState(reactorProject);
if (state == State.TO_BE_INSTALLED) {
Map<String, Object> pluginContext = session.getPluginContext(reactorProject);
ArtifactInstallerRequest request =
(ArtifactInstallerRequest) pluginContext.get(ArtifactInstallerRequest.class.getName());
installProject(request);
synchronized (DEFERRED_INSTALL_LOCK) {
if (allProjectsMarked(projectsUsingPlugin)) {
List<Project> installedProjects = new ArrayList<>();
for (Project reactorProject : projectsUsingPlugin) {
State state = getState(reactorProject);
if (state == State.TO_BE_INSTALLED) {
Map<String, Object> pluginContext = session.getPluginContext(reactorProject);
ArtifactInstallerRequest request =
(ArtifactInstallerRequest) pluginContext.get(ArtifactInstallerRequest.class.getName());
try {
installProject(request);

@slawekjaranowski slawekjaranowski Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 3.x branch we build request wich contains all artifacts from all projects, and finally repositorySystem.install is called only once, so we resolver take care about install all artifacts

@gnodet gnodet Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct — 3.x collects all artifacts into a single InstallRequest and calls repositorySystem.install() only once, so:

  • f005 (usingPlugin): 3.x already matches by plugin presence (hasExecution checks goals + phase ≠ none), not by execution-id — no fix needed
  • f005 (partial-install inventory): the single atomic install means there is no per-project loop that could fail mid-way — no partial state to log
  • f006 (DEFERRED_INSTALL_LOCK): even if two threads race into the deferred block, the second call just re-installs the same request — idempotent, no risk of mixed state

No backport to 3.x planned for this PR.

} 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<Project> projectsUsingPlugin, List<Project> 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<Project> projectsUsingPlugin) {
return projectsUsingPlugin.stream().allMatch(this::hasState);
}
Expand Down
117 changes: 117 additions & 0 deletions src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<String, Object> deferredContext(ArtifactInstallerRequest request) {
Map<String, Object> 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)
Expand Down
Loading