Upgrade the generator to use the archetype version of the maven runtime

[laeubi]

Now as everything is decoupled, we can start upgrading the components...

[laeubi]

This shows the removed support for a repository in the Archetype 3.x:

https://ci.eclipse.org/m2e/job/m2e/job/PR-769/1/testReport/junit/org.eclipse.m2e.tests/ProjectConfigurationManagerTest/testResolutionOfArchetypeFromRepository/

@mickaelistria @HannesWell as maven archetype do not support this anymore by direct invocation I would say we should remove the corresponding test as well?

[mickaelistria]

/> as maven archetype do not support this anymore by direct invocation I would say we should remove the corresponding test as well?

Can't we migrate the test to verify the new behavior?

[laeubi]

Can't we migrate the test to verify the new behavior?

Sure if you think its worth it one could simply assert the failure of course...

[mickaelistria]

Sure if you think its worth it one could simply assert the failure of course...

I don't have a strong opinion as I'm presonally not using artifact and haven't seen many requests on those. You're the expert now, it's your call to decide whether removing tests is what's best or whether they still have value that will help sustainability, maintenance and innovation later.

[laeubi]

You're the expert now, it's your call to decide whether removing tests is what's best or whether they still have value that will help sustainability, maintenance and innovation later.

I finally migrated the test to the newer archetype-aproach to define the repository in the settings.xml

[laeubi]

@HannesWell I cherry-picket your adjustments to use latest archetype in the code as well and removed the deprecated repository code now, if that works we can merge and this should finally be done:

• 514993 - Maven archetype post generated script is not executed while creating from maven module wizard  #249

[laeubi]

Sorry that was to early, this will only partly fix #249 as we use batch-mode so there is a slight adjustment required afterwards to allow the user use interactive mode...

[HannesWell]

@HannesWell I cherry-picket your adjustments to use latest archetype in the code as well and removed the deprecated repository code now, if that works we can merge

In my last attempt there were test-failures and one open TODO associated to it. But maybe you can fix that. As written in #753 (comment) this is likely due to missing/faulty configuration.

[HannesWell]

Btw. could you please also update the m2e-core-tests pointer with this PR after you have merged the adjusted tests.

[laeubi]

Btw. could you please also update the m2e-core-tests pointer with this PR after you have merged the adjusted tests.

do you have something in mind here I should do immediately or before merging?

[laeubi]

In my last attempt there were test-failures and one open TODO associated to it. But maybe you can fix that.

It becomes a bit more complex (and maybe that a reason a very old version was used before) as the archetype is actually a maven plugin and thus, if run from maven cli, has an own plugin classloading realm.

Now in m2e, we simply merge the two class pathes, what do not work well as both seem to use slightly different implementations and thus container injection fails later on... I'll try if we can replicate this behavior.

That said, it might even be beneficial that we do not upgrade the internal archetype at all as the generation of the archetype itself is now done without a problem by maven itself, thus as a user one wont notice that and we just use that for the parsing of the catalogues, where we might even be able to call the code directly.

[HannesWell]

Sounds problematic.

As shortly outlined in #249 (comment) do you think it would be doable to just embed archetype-catalog and archetype-descriptor and drop archetype-common? We would then not need any of the other dependencies any more so the m2e.archetype bundle would be very small and maybe this also avoids the classpath issue you just described (but that's just a hope while having only very few knowledge).
I tested that locally and it looks like mainly org.apache.maven.archetype.ArchetypeManager and org.apache.maven.archetype.ArchetypeGenerationRequest has to be replaced then.
I can update #753 to show that.

[laeubi]

If I can fix the container issue we can most probably remove everything that we do not reference as a type in m2e...

[laeubi]

do you think it would be doable to just embed archetype-catalog and archetype-descriptor and drop archetype-common

I think we can't get rid of commons, but at least my approach to build a plugin realm that is connected to some classes failed :-
What would be required here, is that we write an abstraction layer over the maven impl, then create a maven component we inject into the plugin realm and lookup our facade.

Even though I have all that together from the maven side, I think I'll first start with a much simpler approach as we actually only need ArchetypeArtifactManager and ArchetypeDataSource from the container.

[laeubi]

@HannesWell this is now all green, I think we can reduce dependencies further (e.g. velocity and groovy should not be required anymore, even maybe more of the plexus stuff) but I'll leave this exercise to the interested reader :-)

[HannesWell]

@HannesWell this is now all green, I think we can reduce dependencies further (e.g. velocity and groovy should not be required anymore, even maybe more of the plexus stuff) but I'll leave this exercise to the interested reader :-)

Great. Thanks for your work.

I can have a look at that, but if I understand it correctly arechtype-commons is still required so a dependency-exclusion cannot be derived from the fact that it is provided somewhere else but I have to know that the code really does not access the dependency? This requires much better knowledge of what is exactly happening in the code. Therefore I would consider this 'dangerous' and would only exclude the groovy/ivy jars because they are by far the largest once. For the few kilobytes the other jars have, I would not risk having runtime-errors due to failed class-loading.

[laeubi]

but I have to know that the code really does not access the dependency?

correct

This requires much better knowledge of what is exactly happening in the code.

You could remove a dependency, generate the metadata and then run ArtefactManagerTest as a first iteration.

Therefore I would consider this 'dangerous'

Why? The code path is well covered by tests and these instantly fail if you remove too much, so don't worry too much.

[HannesWell]

Therefore I would consider this 'dangerous'

Why? The code path is well covered by tests and these instantly fail if you remove too much, so don't worry too much.

That's good then and not to worry about.