search

eclipse-m2e / m2e-core

Eclipse Public License 2.0
110 stars 112 forks source link

Embedd org.eclipse.m2e.workspace.cli into a m2e.maven.ext core-extension #551

Open HannesWell opened 2 years ago

HannesWell commented 2 years ago

The project org.eclipse.m2e.workspace.cli, which is published via m2e-core's repository is currently located in a separate repository: https://github.com/eclipse-m2e/org.eclipse.m2e.workspace

At the moment the small project is inactive for years. On Maven-Central only for Takari artifacts use it: https://mvnrepository.com/artifact/io.takari.m2e.workspace/org.eclipse.m2e.workspace.cli

It is licensed under EPLv1.0

As discussed in PR #533 we should consider to include respectively embedded its classes (as an internal) into the m2e-core code base, unless it provides some API functionality used by other in order to 'connect' to m2e. We could then immediately discard deprecated code.

laeubi commented 2 years ago

@HannesWell what do you think, should we simply include this plugin (maybe under a new name) to further proceed here?

HannesWell commented 2 years ago

@HannesWell what do you think, should we simply include this plugin (maybe under a new name) to further proceed here?

Good question. I have not thought about this further and cannot tell. Maybe @fbricon can tell if there is any chance that project evolves further in the future?

But IIRC m2e is using it only internally anyway so we could include it now and again replace it later in case the project evolves in the future. On the other hand do we have a benefit if we embedded it? We could first update it. I'm not sure why I have not done it yet, but I can have a look.

laeubi commented 2 years ago

I think we effectively would "adopt" the project then, so we ca change/evolve the code and others probably then use this if they want to.

HannesWell commented 2 years ago

I think we effectively would "adopt" the project then, so we ca change/evolve the code and others probably then use this if they want to.

Yes. Do you see a need to adjust it? I don't see a good one, but also didn't thought about it further. Since we can include it at any time we want (it is just used internally and I don't think that anybody relies on the specific workspace-state file format?) I suggest to do it at the time we need it. One reason I hesitate a bit to include the workspace.cli is that it has to be in a dedicated plug-in that can be included into a launched Maven-runtime. See MavenLaunchUtils.getCliResolver().

And I think it is not worth it to add another plug-in just because a dependency is not actively developed anymore if it is working as we need it to work.

As far as I can tell the workspace.cli is only used to enabled 'workspace dependency resolution'. Do you know what this exactly means? Does it mean that one can run a Maven build where a reactor project depends on another project in the workspace that is not included in the reactor and also not available from a repository or to overwrite the project in the local .m2 cache?

If I searched correctly it is the following Option for a Maven launch:

grafik

I wonder if we need this, but if one can really 'override' a dependency to use the one from the workspace that sounds really useful for testing. On the other hand this would likely only work for the build time, not when you install the projects being build in the local maven cache. I'm thinking here for example about using a locally modified version of p2 in Tycho. M2E would then first have to handle the Tycho-polyglot poms, but then it could be possible to build Tycho with a modified version of P2 just by having both in the same workspace. Using this could even make a Tycho install obsolete when you want to test Tycho modifications in a build launched from the Tycho workspace.

laeubi commented 2 years ago

Do you see a need to adjust it?

Last time there where some "deprecated" / "don't use anymore" methods

As far as I can tell the workspace.cli is only used to enabled 'workspace dependency resolution'. Do you know what this exactly means?

In a normal mvn command-line build (and that's what we are doing here!) everything is only resolved from the current reactor build, then from the repository and then fails.

To prevent this, maven allows to register a so called "Workspace Reader", that allows to inject additional "workspace" artifacts (e.g. projects in an eclipse workspace) so the order is Reactor > Workspace Reader > Repository > Fail and that's what this plugin offers a bridge for. That's why I think its safe to use an own artifact for this, and probably we want to further enhance this later on that it become more clever than simply include everything...

HannesWell commented 2 years ago

To prevent this, maven allows to register a so called "Workspace Reader", that allows to inject additional "workspace" artifacts (e.g. projects in an eclipse workspace) so the order is Reactor > Workspace Reader > Repository > Fail and that's what this plugin offers a bridge for. That's why I think its safe to use an own artifact for this, and probably we want to further enhance this later on that it become more clever than simply include everything...

Thanks for the elaboration. That is indeed usefull. Do you know if this also works for plug-ins of the Maven-build (for example in the Tycho-workspace) or does this require something like: https://github.com/ifedorenko/com.ifedorenko.m2e.mavendev#maven-development-tools ?

laeubi commented 2 years ago

I think @akurtakov can better tell here as he is a contributor of that project :-)

akurtakov commented 2 years ago

I think @akurtakov can better tell here as he is a contributor of that project :-)

That surprised me a lot (looks like I never really contributed anything but a releng fix) :) . I'm not sure I understand what the question is exactly but workspace reader is a general concept that applies to Maven builds in general and we relied on it in XMvn to override version with system one in RPM builds.

laeubi commented 2 years ago

@HannesWell as the story about upgrade to latest archetype might require additional extensions to be loaded for maven runs:

How is the status here? Are we "safe" to adopt this class or do we need a CQ here? My plan is that we just have a m2e.maven.ext plugin inside m2e where we place such classes to be placed on the ext-classpath of a maven run then.

HannesWell commented 2 years ago

Sorry I didn't have the time/got distracted to continue here. Since those few classes are just used internally I think from a technological point of we are safe to adopt those classes within m2e. If there is work on a standalone workspace.cli that we should use we can still switch back to that at any time. Regarding the CQ I cannot tell.

My plan is that we just have a m2e.maven.ext plugin inside m2e where we place such classes to be placed on the ext-classpath of a maven run then.

Is there a dedicated ext-classpath in Maven? Or is it just the regular classpath to which MavenEmbeddedRuntime.initClasspath() adds all fragments of o.e.m2e.maven.runtime? Nevertheless such a plug-in that is placed (as plain jar) on the classpath of the Maven-application could also be required for other purposes like

Also some cool features like the maven-build view from https://github.com/ifedorenko/com.ifedorenko.m2e.mavendev (which I would like to import into m2e in the future, will open a dedicated issue for that in the future) use such a maven-side project.

With that there is the question if we should only have one such maven-side project or if we should have one per subject? This would also bring in the question if we should have only one connection between a maven-process and m2e or one per subject. Since we now can use Java-17 (🎉) we could use the Java-16's Unix Domain Sockets for that, which should be faster and don't require Networking Ports etc. So one connection per topic sounds feasible.

By the way, where do I get 48h days? 😅

laeubi commented 2 years ago

Regarding the CQ I cannot tell.

I just wondering if you have opened one or I should do so.

Is there a dedicated ext-classpath in Maven?

In maven CLI, yes there is a special folder, for m2e might compute the classpath different.

With that there is the question if we should only have one such maven-side project or if we should have one per subject?

One for all, we can simply enable/disable them if required by passing a system option.

if we should have only one connection between a maven-process and m2e

Don't understand this, what do you mean by connection? if we need IPC then it will of course depend, e.g. one might simply write out relevant state to a file and simply pass that file as an argument and then the extension can read it.

Since we now can use Java-17 (tada) we could use the Java-16's Unix Domain Sockets for that

You mean we can drop windows support?

HannesWell commented 2 years ago

Regarding the CQ I cannot tell.

I just wondering if you have opened one or I should do so.

No I have not. If you can/want to do it, please go ahead.

if we should have only one connection between a maven-process and m2e

Don't understand this, what do you mean by connection? if we need IPC then it will of course depend, e.g. one might simply write out relevant state to a file and simply pass that file as an argument and then the extension can read it.

Yes I was referring to IPC. That would work in some cases but in some cases would at least be not ideal. But yes it depends. :)

Since we now can use Java-17 (tada) we could use the Java-16's Unix Domain Sockets for that

You mean we can drop windows support?

That would be bad, because then I couldn't use m2e anymore. But I was told that contrary to the name, Unix domain sockets also work on Windows. Maybe only Windows 10 and later but I assume that earlier versions are still relevant.

laeubi commented 2 years ago

No I have not. If you can/want to do it, please go ahead.

Stupid me didn't noticed that we already have this imported :-D

So it would just be a mater of merging it here.... and then archive the former one...