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 5 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
41 changes: 36 additions & 5 deletions pom.xml
View file
Open in desktop
Expand Up @@ -63,7 +63,10 @@
</distributionManagement>

<properties>
<mavenVersion>3.0</mavenVersion>
<mavenVersion>3.2.5</mavenVersion>
<slf4jVersion>1.7.32</slf4jVersion>
<aetherVersion>1.1.0</aetherVersion>
<surefire.version>2.22.2</surefire.version>
<javaVersion>7</javaVersion>
<project.build.outputTimestamp>2020-04-07T21:04:00Z</project.build.outputTimestamp>
</properties>
Expand All @@ -73,13 +76,22 @@
<groupId>org.apache.maven</groupId>
<artifactId>maven-plugin-api</artifactId>
<version>${mavenVersion}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-artifact</artifactId>
<version>${mavenVersion}</version>
<scope>provided</scope>
</dependency>

<!-- This is here to override 3.0 coming with maven-artifact-transfer -->
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-core</artifactId>
<version>${mavenVersion}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.maven.shared</groupId>
<artifactId>maven-artifact-transfer</artifactId>
Expand All @@ -96,13 +108,13 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.1</version>
<version>4.13.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven.plugin-testing</groupId>
<artifactId>maven-plugin-testing-harness</artifactId>
<version>2.1</version>
<version>3.3.0</version>
<scope>test</scope>
</dependency>
<dependency> <!-- used by maven-plugin-testing-harness, don't give it compile scope! -->
Expand All @@ -120,13 +132,31 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.30</version>
<version>${slf4jVersion}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-nop</artifactId>
<version>1.7.30</version>
<version>${slf4jVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.aether</groupId>
<artifactId>aether-api</artifactId>
<version>${aetherVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.aether</groupId>
<artifactId>aether-util</artifactId>
<version>${aetherVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.aether</groupId>
<artifactId>aether-impl</artifactId>
<version>${aetherVersion}</version>
cstamas marked this conversation as resolved.
Show resolved Hide resolved
<scope>test</scope>
</dependency>
</dependencies>
Expand All @@ -150,6 +180,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-invoker-plugin</artifactId>
<version>3.2.2</version>
<configuration>
<cloneProjectsTo>${project.build.directory}/it</cloneProjectsTo>
<cloneClean>true</cloneClean>
Expand Down
72 changes: 41 additions & 31 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 @@ -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() )
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 );
Boolean install = (Boolean) pluginContext.get( INSTALL_PROCESSED_MARKER );
michael-o marked this conversation as resolved.
Show resolved Hide resolved
if ( !install )
{
installProject( session.getProjectBuildingRequest(), INSTALLREQUESTS.remove( 0 ) );
getLog().info( "Project " + projectKey + " skipped install" );
cstamas marked this conversation as resolved.
Show resolved Hide resolved
}
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<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
16 changes: 9 additions & 7 deletions src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -215,7 +217,7 @@ public void testInstallFileWithPomAsPackaging()
InstallFileMojo mojo = (InstallFileMojo) lookupMojo( "install-file", testPom );

assertNotNull( mojo );

setVariableValueToObject( mojo, "session", createMavenSession() );

assignValuesForParameter( mojo );
Expand Down Expand Up @@ -243,7 +245,7 @@ public void testInstallFile()
InstallFileMojo mojo = (InstallFileMojo) lookupMojo( "install-file", testPom );

assertNotNull( mojo );

setVariableValueToObject( mojo, "session", createMavenSession() );

assignValuesForParameter( mojo );
Expand Down Expand Up @@ -281,11 +283,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 );
Expand Down