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.

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

Jump to bottom

[MINSTALL-115] Install At End feature (no extension) #15

Merged
merged 17 commits into from Jul 3, 2022
Merged

[MINSTALL-115] Install At End feature (no extension) #15

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
06d10f9
Install At End feature (no extension)
cstamas Oct 7, 2021
c08a736
Remove unneeded change
cstamas Oct 7, 2021
4b03381
Add aetherVersion property
cstamas Oct 7, 2021
245f135
Go back to Java7
cstamas Oct 7, 2021
aded185
Merge remote-tracking branch 'origin/master' into install-at-end
cstamas Oct 7, 2021
cf070a2
Merge remote-tracking branch 'origin/master' into install-at-end
cstamas Nov 19, 2021
f03c4bf
Fix keys and log message.
cstamas Nov 19, 2021
3c1da73
Merge remote-tracking branch 'origin/master' into install-at-end
cstamas Dec 12, 2021
4cecdff
Format
cstamas Dec 12, 2021
b73cce7
Merge remote-tracking branch 'origin/master' into install-at-end
cstamas Dec 14, 2021
10c039f
Merge remote-tracking branch 'origin/master' into install-at-end
cstamas May 20, 2022
af0323d
Undo unrelated changes
cstamas May 29, 2022
98065bf
Simplify
cstamas May 29, 2022
761fd38
Merge remote-tracking branch 'origin/master' into install-at-end
cstamas Jun 29, 2022
d292de9
Better cover all possible cases, be clearer about intent
cstamas Jun 29, 2022
b6cd186
create state management methods to clarify
hboutemy Jul 2, 2022
63aa94e
fix IT
hboutemy Jul 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 44 additions & 34 deletions src/main/java/org/apache/maven/plugins/install/InstallMojo.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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<ProjectInstallerRequest> INSTALLREQUESTS =
Collections.synchronizedList( new ArrayList<ProjectInstallerRequest>() );

/**
*/
@Parameter( defaultValue = "${project}", readonly = true, required = true )
private MavenProject project;

@Parameter( defaultValue = "${reactorProjects}", required = true, readonly = true )
private List<MavenProject> 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.
Expand All @@ -88,56 +83,71 @@ public class InstallMojo
@Component
private ProjectInstaller installer;

private enum State
{
SKIPPED, INSTALLED, TO_BE_INSTALLED
}

public void execute()
throws MojoExecutionException, MojoFailureException
{
boolean addedInstallRequest = false;
if ( skip )
{
getLog().info( "Skipping artifact installation" );
getPluginContext().put( INSTALL_PROCESSED_MARKER, State.SKIPPED.name() );
Copy link
Member

Choose a reason for hiding this comment

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

If we have objects, why not retain the enum, why use name?

Copy link
Member Author

@cstamas cstamas Jun 29, 2022 •
edited

Choose a reason for hiding this comment

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

These span across several classloaders, there is no single enum.

}
else
{
// CHECKSTYLE_OFF: LineLength
ProjectInstallerRequest projectInstallerRequest =
new ProjectInstallerRequest().setProject( project );
// CHECKSTYLE_ON: LineLength

if ( !installAtEnd )
{
installProject( session.getProjectBuildingRequest(), projectInstallerRequest );
installProject( project );
getPluginContext().put( INSTALL_PROCESSED_MARKER, State.INSTALLED.name() );
}
else
{
INSTALLREQUESTS.add( projectInstallerRequest );
addedInstallRequest = true;
getLog().info( "Installing " + getProjectReferenceId( project ) + " at end" );
getPluginContext().put( INSTALL_PROCESSED_MARKER, State.TO_BE_INSTALLED.name() );
}
}

boolean projectsReady = READYPROJECTSCOUNTER.incrementAndGet() == reactorProjects.size();
if ( projectsReady )
if ( allProjectsMarked() )
Copy link

Choose a reason for hiding this comment

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

Does this work if the last project being built does not have a standard lifecycle ? Or maybe this does not exist ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If m-install-p is not present, will not, but all "standard" life-cycle has them. But true, if no m-install-p not present, will not work. Was thinking about org.apache.maven.execution.MavenSession#getResult (and use org.apache.maven.execution.MavenExecutionResult#getBuildSummary to check "is it done"), wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Neglect that above. Instead: this does NOT modify the existing feature (when it forced you to use plugin as extension), and will fix this in some newer PR

Copy link
Member

@hboutemy hboutemy Jul 3, 2022 •
edited

Choose a reason for hiding this comment

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

if the last project being built does not have a standard lifecycle ?

is this a pure theoretical question (legit when thinking, but...)? Or do you know any concrete case?

IMHO, the best we can do is to add a note in plugins' parameter documentation (and it's not really about "standard lifecycle" but having install goal also run in the last reactor project)

{
synchronized ( INSTALLREQUESTS )
for ( MavenProject reactorProject : reactorProjects )
{
while ( !INSTALLREQUESTS.isEmpty() )
Map<String, Object> pluginContext = session.getPluginContext( pluginDescriptor, reactorProject );
State state = State.valueOf( (String) pluginContext.get( INSTALL_PROCESSED_MARKER ) );
if ( state == State.TO_BE_INSTALLED )
{
installProject( session.getProjectBuildingRequest(), INSTALLREQUESTS.remove( 0 ) );
installProject( reactorProject );
}
}
}
else if ( addedInstallRequest )
}

private String getProjectReferenceId( MavenProject mavenProject )
{
return mavenProject.getGroupId() + ":" + mavenProject.getArtifactId() + ":" + mavenProject.getVersion();
}

private boolean allProjectsMarked()
{
for ( MavenProject reactorProject : reactorProjects )
{
getLog().info( "Installing " + project.getGroupId() + ":" + project.getArtifactId() + ":"
+ project.getVersion() + " at end" );
Map<String, Object> 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 )
{
Expand Down
19 changes: 19 additions & 0 deletions src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -88,6 +91,8 @@ 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() );

Expand Down Expand Up @@ -120,6 +125,8 @@ 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() );

Expand Down Expand Up @@ -162,6 +169,8 @@ 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() );

Expand All @@ -188,6 +197,8 @@ 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() );

Expand Down Expand Up @@ -224,6 +235,8 @@ 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() );

Expand Down Expand Up @@ -260,6 +273,8 @@ 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 );

Expand Down Expand Up @@ -316,6 +331,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() );

Expand Down Expand Up @@ -357,6 +374,8 @@ 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<String, Object>() );
return session;
}

Expand Down