[MINSTALL-115] Install At End feature (no extension) #15
Conversation
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).
src/main/java/org/apache/maven/plugins/install/InstallMojo.java
Outdated
Show resolved
Hide resolved
|
@cstamas This should not be affected by apache/maven#625. The reason is that when the session is cloned (this is already the case currently), the map containing the plugin contexts [1] is simply copied into the new session, so that they are actually shared. |
| { | ||
| getLog().info( | ||
| "Project " + getProjectReferenceId( reactorProject ) + " skipped install" | ||
| ); |
There was a problem hiding this comment.
This looks redundant with the log statement printed earlier "Skipping artifact installation". This will print lots of line when the artifacts are actually installed for no benefit imho.
There was a problem hiding this comment.
The message you refers to is when maven is invoked with -Dmaven.install.skip, when whole m-install-p is skipped.
src/main/java/org/apache/maven/plugins/install/InstallMojo.java
Outdated
Show resolved
Hide resolved
| addedInstallRequest = true; | ||
| } | ||
| } | ||
|
|
||
| boolean projectsReady = READYPROJECTSCOUNTER.incrementAndGet() == reactorProjects.size(); | ||
| if ( projectsReady ) | ||
| if ( allProjectsMarked() ) |
There was a problem hiding this comment.
Does this work if the last project being built does not have a standard lifecycle ? Or maybe this does not exist ?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
|
@gnodet @michael-o ping, Guillaume, you have change requests here (unsure what). But let's agree all this PR does is:
So IMHO is def an improvement |
|
Will check on train tomorrow |
src/main/java/org/apache/maven/plugins/install/InstallMojo.java
Outdated
Show resolved
Hide resolved
Still, we MUST use String as these are spanning across projects, hence due classloader, there is no single State enum.
| getLog().info( "Skipping artifact installation" ); | ||
| getPluginContext().put( INSTALL_PROCESSED_MARKER, State.SKIPPED.name() ); |
There was a problem hiding this comment.
If we have objects, why not retain the enum, why use name?
There was a problem hiding this comment.
These span across several classloaders, there is no single enum.
| addedInstallRequest = true; | ||
| } | ||
| } | ||
|
|
||
| boolean projectsReady = READYPROJECTSCOUNTER.incrementAndGet() == reactorProjects.size(); | ||
| if ( projectsReady ) | ||
| if ( allProjectsMarked() ) |
There was a problem hiding this comment.
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)
This PR makes installAtEnd work as expected even
if maven-install-plugin is not used as extension.
How it works: it uses mojo Contexts to store "state markers":
UTs adjusted to provide plugin context (was null before).
Plugin updated (as required by maven-plugin-testing-harness) and
cleaned up.
https://issues.apache.org/jira/browse/MINSTALL-115