search

eclipse-platform / .github

Common contribution content for eclipse-platform repositories
https://www.eclipse.org/eclipse/
5 stars 10 forks source link

Include m2e in Oomph created platform SDK #10

Closed laeubi closed 2 years ago

laeubi commented 2 years ago

I noticed to day while editing a pom.xml that is is only uses a plain text editor. Can we include m2e in the platform SDK?

merks commented 2 years ago

I tried installing it, but it seems to spend an obscene amount of time doing this:

image

I'm not sure where it's saving all that information or why it needs it.

I guess ECF is using it?

image

Then one of PDE's unit test things has an error in the build and every poml.xml you open produces an error about the parent:

image

So that doesn't seem helpful.

Closing the editors fills up the log with NPEs.

image

I don't know if I installed the right things:

<?xml version="1.0" encoding="UTF-8"?>
<setup.p2:P2Task
    xmi:version="2.0"
    xmlns:xmi="http://www.omg.org/XMI"
    xmlns:setup.p2="http://www.eclipse.org/oomph/setup/p2/1.0"
    label="m2e">
  <requirement
      name="org.eclipse.m2e.pde.feature.feature.group"/>
  <requirement
      name="org.eclipse.m2e.lemminx.feature.feature.group"/>
  <repository
      url="https://download.eclipse.org/technology/m2e/snapshots/latest/"/>
</setup.p2:P2Task>

You can use Navigate -> Open Setup -> Installation and copy and paste the above into the installation and the perform setup tasks again, running the p2 task to install what I tried. You could using Navigate -> Open Setup -> User and paste it there to install m2e in all your installations...

So at this point, I don't see that installing m2 buys much and that it causes problems that I don't know how to and don't have time to resolve.

laeubi commented 2 years ago

I don't know if I installed the right things:

I think as a first step m2e core would be enough.

Closing the editors fills up the log with NPEs.

Do you like to report these exceptions (from the stack it seems to originate from lsp4e)

So at this point, I don't see that installing m2 buys much and that it causes problems that I don't know how to and don't have time to resolve.

Given that we build most of the stuff using maven/tycho today I think it is important to have some tooling for that, and if there are issues we should report them so it could be improved (as @mickaelistria mentioned dog-fooding is important to platform):

dogfood https://reqtest.com/general/is-this-what-they-mean-when-they-say-eat-your-own-dog-food/

So maybe its a good opportunity here to incremental fix issues, I also don't think its your task to do so :-)

merks commented 2 years ago

Well, m2e is not my dog food and I really do have many, many, many other things. I imagine if other people are eating this dog food then they are seeing these problems and will be motivated to report them and even to fix them.

I don't know what parts of m2e should be installed that will not have undesirable side effects and still buy me some useful/interesting functionality that most/many contributors will need. The repo says this is available:

image

Please play with a p2 setup task to your Installation.setup until you get things to a state where it does not cause problems when used in combination with this:

https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#create-an-eclipse-development-environment

Then we can revisit the issue.

mickaelistria commented 2 years ago

I'm curious about why m2e would spend some time on Platform projects. Those projects don't have the m2e nature as far as I know, so m2e should just ignore them and not try anything particular.

laeubi commented 2 years ago

org.eclipse.pde.unittest.junit has m2e nature enabled also org.eclipse.ecf.provider.filetransfer.httpclient5 if thats a problem we can for sure remove this, its just very uncomfortable to change the IDE when editing pom files... Beside that the latest release should probably be used in contrast to the snapshots.

merks commented 2 years ago

image

For the full SDK configuration, I try to include all source projects in the Eclipse SDK product from all git clones, so that includes ECF and EMF (where only the parts used the the platform are in the workspace)...

@laeubi Snapshots are the freshest dog food! If there is a new bug you won't notice it until it's released and then it won't get fixed the next day... Given the IDE itself installs the latest I-Build is seems the best dog food approach one can take...

Is there a way to get rid of the "parent" errors? Or to suppress them as warnings?

I don't see any m2e-related preferences which seems strange:

image

These the the plugins that are installed:

image

I imagine the PDE connector is useful if you want to work on the *.target file....

laeubi commented 2 years ago

For the full SDK configuration, I try to include all source projects in the Eclipse SDK product from all git clones, so that includes ECF and EMF (where only the parts used the the platform are in the workspace)...

That's fine but we might remove the m2e nature from the project, not the project itself!

Given the IDE itself installs the latest I-Build is seems the best dog food approach one can take.

But then we also need to accept some quirks I think (e.g. NPE on shutdown), I'm fine with snapshots of course just though a (hopefully stable) release would lower the barrier.

Is there a way to get rid of the "parent" errors? Or to suppress them as warnings?

This might be cause because the individual bundles (does it include parent references as well?) is not included @mickaelistria do you know how this could be configured in m2e? Beside that we can (for now) disable m2e nature for this projects I think as the .project file is checked in I don't see why one want to enable this only for this particular project (but not for others).

I don't see any m2e-related preferences which seems strange:

Yes there should be "Maven" at the top level with several options.

I imagine the PDE connector is useful if you want to work on the *.target file....

Correct, that might be the most visible value for platform as platform now uses maven items in the target, but I don't see the target included in the platform SDK maybe @akurtakov knows where this is located?

akurtakov commented 2 years ago

Target platform is set at https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/master/eclipse-platform-parent/pom.xml#L241 . There is a reason to not include M2E (and EGit too as it's even more critical for daily work) and that is circular dependency. Eclipse SDK is built from source always and entirely and some of the changes require adjustments in projects that depend on it (M2E/EGit and thus LSP4E, Lemminx, TM4E, etc. transitively) if these are included in the SDK that would introduce instability in I builds and literally make it impossible to do many changes (e.g. bumping a version may break resolving and thus the I build). What you seem to be looking for is (more) "regular" builds (than milestones) of https://www.eclipse.org/downloads/packages/release/2022-03/r/eclipse-ide-eclipse-committers . This could be helpful for someone trying to use latest of everything of that would become part of this package. Many of the Eclipse SDK contributors do not use Maven regularly (unfortunately!) but we can not afford to introduce yet another cyclic dependency in the build. It's enough struggle with ECF (which I even want to remove as a dependency - in order to have one less cycle) and quite smooth with EMF - although still a burden for both @merks (to release EMF a week prior to each Eclipse milestone/rc/release) and for platform-releng. I don't think we even have the capacity to do that at @m2e . With all of the above please try to drive this through EPP project and the Committer package - it can't happen at the Platform SDK level. The sole intention for this package to exist was to be used by people contributing regularly. @guw as Committer EPP maintainer it might be best for you and @laeubi to discuss such and opportunity.

merks commented 2 years ago

The target is located here:

image

I added a task to create the .project file, which is ignored by the .gitignore.

merks commented 2 years ago

@akurtakov This is more a question here of what to automatically install into the IDE via the Oomph setup(s), not what's in the actual Eclipse SDK build. Obviously most contributors want to use EGit so it's installed in the setup, but including it in the SDK build I don't think makes sense, as you say. Maybe m2e is useful for some/many contributors too, but the workspace must be free of errors so if installing it causes errors then I think it creates confusion...

akurtakov commented 2 years ago

OK, my bad in this case. Retitle and reopen the issue

laeubi commented 2 years ago

@merks is right, m2e should not be part of the projects, but as a plugin, for that as mentioned above, I would also be fine with a released version if that helps (even though it does not offer a super fast feedback cycle), for two use cases:

  1. Editing target files that include maven locations
  2. Editing the pom-files e.g. version bumps or configuration changes

for both it is not strictly required that a project has the m2e nature, m2e just needs to be installed so it provides the editor extensions.

Beside that, there was recently some discussions to do some stuff we currently do in ant with maven instead, in that case m2e must be enabled for the project to make this work. Also @tjwatson mentioned BND usage in some cases, also here m2e would be needed for enable the correct hooks for maven/pde integration, but that's all more future ideas :-)

akurtakov commented 2 years ago

So I see two issues discussed now:

laeubi commented 2 years ago

parent poms - I don't see how pom.xml editing could be useful without being able to resolve parent poms.

At least I can install them once to satisfy maven / m2e (maybe could be triggered automatic by the Oomph setup?)

having maven nature - if it's dicussed to remove it from projects, why have it all created and supported by m2e?

This is not about generally remove the nature, it is just that in PDE / Platform it seems to be enabled for only those four projects and I can't see what m2e is supposed to do here? So it more seems like being enabled by accident than by intention.

we should have at m2e as if we can make things works without a nature

The nature is only important if you want m2e to do certain things, e.g execute some mojos or manage your dependencies. As these projects seem to work fine and no one else has complained (beside me) until now that m2e is missing I assume they do not depend on m2e to be enabled for them.

akurtakov commented 2 years ago

Is there still work left to do here guys?

laeubi commented 2 years ago

Yes, I think this is something we should start with the new m2e release as platform is using more an more maven artifacts.

merks commented 2 years ago

I get annoying logged warnings about the missing PDE m2e stuff like this already:

org.eclipse.core.runtime.CoreException: Could not find a org.eclipse.pde.core.targetLocations extension for type: Maven
    at org.eclipse.pde.internal.core.target.TargetPersistence38Helper.deserializeBundleContainer(TargetPersistence38Helper.java:249)
    at org.eclipse.pde.internal.core.target.TargetPersistence38Helper.initFromDoc(TargetPersistence38Helper.java:101)
    at org.eclipse.pde.internal.core.target.TargetDefinitionPersistenceHelper.initFromXML(TargetDefinitionPersistenceHelper.java:188)
    at org.eclipse.pde.internal.core.target.TargetDefinition.setContents(TargetDefinition.java:802)
    at org.eclipse.pde.internal.core.target.AbstractTargetHandle.getTargetDefinition(AbstractTargetHandle.java:34)
    at org.eclipse.oomph.util.pde.TargetPlatformUtil$5.run(TargetPlatformUtil.java:302)
    at org.eclipse.oomph.util.pde.TargetPlatformUtil$5.run(TargetPlatformUtil.java:1)
    at org.eclipse.oomph.util.pde.TargetPlatformUtil.runWithTargetPlatformService(TargetPlatformUtil.java:120)
    at org.eclipse.oomph.util.pde.TargetPlatformUtil.getTargetDefinitions(TargetPlatformUtil.java:289)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.eclipse.oomph.targlets.provider.TargletContainerItemProvider.getTargetDefinitions(TargletContainerItemProvider.java:325)
    at org.eclipse.oomph.targlets.provider.TargletContainerItemProvider.getTargetDefinitionNames(TargletContainerItemProvider.java:298)
    at org.eclipse.oomph.targlets.provider.TargletContainerItemProvider.getTargetDefinitionNames(TargletContainerItemProvider.java:288)
    at org.eclipse.oomph.targlets.provider.TargletContainerItemProvider.collectNewChildComposedTargetDescriptors(TargletContainerItemProvider.java:426)
    at org.eclipse.oomph.setup.targlets.provider.TargletTaskItemProvider.collectNewChildDescriptors(TargletTaskItemProvider.java:623)
    at org.eclipse.emf.edit.provider.ItemProviderAdapter.getNewChildDescriptors(ItemProviderAdapter.java:801)
    at org.eclipse.emf.edit.domain.AdapterFactoryEditingDomain.getNewChildDescriptors(AdapterFactoryEditingDomain.java:748)

But in the end, if installing all of m2e leads to all kinds of errors in the workspace then it seems a no go to me. Errors make people think there is a problem; that's the point of an error message isn't it? And then it makes them think the problem needs to be fixed; why is it an error if I can ignore it and everything just works? You can't tell people to ignore errors; you need to avoid them, disable them, or prevent them...

laeubi commented 2 years ago

But in the end, if installing all of m2e leads to all kinds of errors in the workspace then it seems a no go to me.

As said before, these error are because some projects enable the m2e nature what obviously is not required, so simple fix would be to remove the m2e nature and only add it if we later on require it.

merks commented 2 years ago

I'm more than happy if the natures would be removed and then if installing m2e caused no problems this was not a problem at all.

I'll try it again on a disposable IDE and see specifically what problems it currently causes and get back to you here.

merks commented 2 years ago

Here are errors I see if I install just the m2e PDE feature:

image

Did you want to install anything beyond that feature?

laeubi commented 2 years ago

I think one needs to remove the <name>org.eclipse.m2e.core.maven2Builder</name> and the <nature>org.eclipse.m2e.core.maven2Nature</nature> from such projects so m2e do not tries to build the pom (what only would work if the parent is in the workspace and also imported as a project)

merks commented 2 years ago

Do you want to install anything other that just this?

image

This already causes the above problems, which are easy to fix I think, but if you want more installed there might be more problems, so I need to know what to expect so that it's not a slippery slope...

laeubi commented 2 years ago

I think the usual ones would be

merks commented 2 years ago

I made the changes to eliminate the errors and modified the setup to install those m2e features.