Finish switch to modello

[gnodet]

This fixes a possible exception when using forked lifecycles.
In my case, this happens when using the spotbugs-maven-plugin:spotbugs plugin which forks the lifecycle (see https://github.com/spotbugs/spotbugs-maven-plugin/blob/spotbugs/src/main/groovy/org/codehaus/mojo/spotbugs/SpotbugsViolationCheckMojo.groovy#L57).

[gnodet]

The second commit removes this exception when the cache is not enabled:

[ERROR] Cannot save incremental build aggregated report
java.lang.IllegalStateException: Cache is not initialized. Actual state: DISABLED
	at com.google.common.base.Preconditions.checkState(Preconditions.java:510)
	at org.apache.maven.caching.xml.CacheConfigImpl.checkInitializedState(CacheConfigImpl.java:577)
	at org.apache.maven.caching.xml.CacheConfigImpl.isRemoteCacheEnabled(CacheConfigImpl.java:480)
	at org.apache.maven.caching.LocalRepositoryImpl.saveCacheReport(LocalRepositoryImpl.java:411)
	at org.apache.maven.caching.CacheControllerImpl.saveCacheReport(CacheControllerImpl.java:779)
	at org.apache.maven.caching.CacheEventSpy.onEvent(CacheEventSpy.java:45)
	at org.apache.maven.eventspy.internal.EventSpyDispatcher.onEvent(EventSpyDispatcher.java:104)
	at org.apache.maven.eventspy.internal.EventSpyExecutionListener.sessionEnded(EventSpyExecutionListener.java:61)
	at org.mvndaemon.mvnd.logging.smart.LoggingExecutionListener.sessionEnded(LoggingExecutionListener.java:80)
	at org.apache.maven.lifecycle.internal.DefaultExecutionEventCatapult.fire(DefaultExecutionEventCatapult.java:64)
	at org.apache.maven.lifecycle.internal.DefaultExecutionEventCatapult.fire(DefaultExecutionEventCatapult.java:42)
	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:137)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
	at org.apache.maven.cli.DaemonMavenCli.execute(DaemonMavenCli.java:825)
	at org.apache.maven.cli.DaemonMavenCli.doMain(DaemonMavenCli.java:244)
	at org.apache.maven.cli.DaemonMavenCli.main(DaemonMavenCli.java:222)
	at org.mvndaemon.mvnd.daemon.Server.handle(Server.java:573)
	at org.mvndaemon.mvnd.daemon.Server.client(Server.java:262)
	at org.mvndaemon.mvnd.daemon.Server.accept(Server.java:230)
	at java.base/java.lang.Thread.run(Thread.java:833)

[gnodet]

@hboutemy should I commit them directly to the MNG-7129 branch ?

[hboutemy]

@hboutemy should I commit them directly to the MNG-7129 branch ?

@gnodet my idea is that you should commit modello migration directly to MNG-7129_Modello when it is about the switch from JAXB to Modello, because I expect that we'll to the swtich in 1 PR that we'll apply to MNG-7129

for other unrelated changes, we need other branches/PRs: instead of creating branches in personal forks, given you have commit rights, I prefer in general branches directly in original Git repository because I think it makes working together easier.

[hboutemy]

how does that relate to MNG-7129 and the switch from JAXB to Modello?

[gnodet]

It does not relate to JAXB, however, this fixes a NPE which happens in the cache system when using forked lifecycles. It may be fixed in a different way, but it did look like the easier fix.
My reasoning is the following : I suppose no-one ever looked for the phase of forked plugin executions and it makes sense to me to have those phases set to the phase of the original mojo execution.
The cache system does look and uses the phase in several locations, and this caused NullPointerException.

[AlexanderAshitkin]

Hi
Thanks for the change! Jaxb is gone for the good. In overall pr looks good for me.
Few notes:

1. as i understand modello schema is located in core. if we move the cache to an extension in future - doest it mean there will be dependency on core from the extension? It might be not convenient to change core if extension model need to be changed. Looks like modello scheme actuall couples the cache and the core.
2. There should be few small jaxb related leftovers - dependency in pom itself and in licenses, but in general it looks good.

[gnodet]

@AlexanderAshitkin I don't see why the modello generated code could not be moved into the extension.

[hboutemy]

ok, now that I reviewed and understood many parts, here my proposal:

1. the first 2 commits should be directly be done in MNG-7129 branch: they are nice little fixes independent from Modello switch
2. last 3 commits should go directly to MNG-7129_Modello branch (that will have to be rebased on MNG-7129)

and the next update I want on MNG-7129_Modello branch is the removal of all dependencies that were added for JAXB: removing them is a key benefit

the full idea behind splitting everything in many buckets is that both original developers of MNG-7129 need to be able to review/understand what is done to their original code: for usual Maven contributors, all the work is about lowering the bar to understanding the global MNG-7129 branch to be able to merge some time later to master

[hboutemy]

thinking a little bit more, I just found that merging the PR to MNG-7129_Modello branch is what I just need to do
and only inject the 2 first commits to MNG-7129

I'll do that and we can continue the work together

[hboutemy]

ok, I did the cherry-picks and the rebase: I think we can close this PR
I'm now cleaning up unused dependencies