IDEA-186628 - Provide Maven Tycho support (no POM-less)

[lppedd]

trafficstars

This PR aims at providing a basic - but multimodule - Maven Tycho support.
Before it can be considered worth reviewing what must be done is (and I need help for these):

• restore plugins/maven/maven36-server-impl/src/org/slf4j/impl/StaticLoggerBinder.java
• restore plugins/maven/maven36-server-impl/src/org/jetbrains/idea/maven/server/Maven3WrapperSl4LoggerFactory.java
• cleanup Maven3XServerEmbedder#doResolveProject

Why do we have two deleted files? See https://youtrack.jetbrains.com/issue/IDEA-186628/Tycho-support-is-broken#focus=Comments-27-6412160.0-0

Also, a good addition would be improving the MANIFEST scanning code. I've quickly come up with a recursive solution but I don't like it. And maybe check if code duplication can be reduced (but I think it's worth it, Tycho might require additional changes in the future).

My idea is to provide basic support by enhancing the IntelliJ source code. All the other enhancements could be contributed via a plugin. Example: reload project on MANIFEST changes, or completion of bundle names.

[nskvortsov]

You did a great job of investigating Maven Tycho!

Maven server embedder was not designed with extensibility in mind. Do you think it would be possible to do all the customization of the resolved Maven projects subsets inside IDEA process (e.g., in MavenProjectResolver.java )? Having a separate plugin would allow improving Tycho support outside IDEA release cycles.

Speaking of hacking the MavenProjectResolver.java. As @zulkar points out here, it is better to keep tycho-specific logic in a plugin. I suggest you add a new extension point in MavenProjectResolver that will allow a plugin author to customise the set of resolved maven projects.

WRT to deleted files. They should be kept intact for IntelliJ to be able to pick up Maven logging as it does now. It looks like a problem with some Tycho dependencies. Maybe detecting and excluding the colliding slf4j implementation during dependency resolution will help?

[lppedd]

Hi @nskvortsov, and thanks! I'll try to answer your questions based and what I was able to understand.

Do you think it would be possible to do all the customization in MavenProjectResolver?

I don't think so. As you could see I had to change Maven3XServerEmbedder a bit.
The Tycho extension looks for MavenSession#getProjects only, so internally the IJ server implementation has to have a different behavior compared to the classic Maven.

Now, Maven3XServerEmbedder#doResolveProject only receive a list of files, representing the POMs. How can we describe to this method/class the different behavior to apply, in a generic way?

I suggest you add a new extension point in MavenProjectResolver that will allow a plugin author to customise the set of resolved maven projects

This one is feasible and I could do it. But then I think on your part (and I say that because you know the code better than me) you should take some time to understand what the extension point should cover. There are multiple places that could take advantage of this new EP imho. An example is the different repository layout indexer, for p2. This now seems fixed to the standard Maven Nexus one. I had a look at the indexing process and it's pretty complex unfortunately.

Maybe detecting and excluding the colliding slf4j implementation during dependency resolution

Might me, but speaking in technical terms, how do you imagine this implemented?

[nskvortsov]

@lppedd I think we should look for a way to modify Maven3XServerEmebedder in such a way, that it knows nothing particular about Tycho project nature. AFAIU we need to change the loadExtensions behavior. That is ok. But can we change it so it works for both Tycho and "non-tycho" projects?

Indexer can also be made extendable. It does not have to share extension point with MavenProjectResolver, so let's concentrate on sync (import) here.

but speaking in technical terms, how do you imagine this implemented?

Given there is a Tycho IJ plugin built with Gradle, the build script can filter out rogue transitive dependency (the best case) from the plugin distribution. Or repack this dependency, removing extra logging backend (worst case).

[lppedd]

@nskvortsov Hi! I've returned to working a bit on this. Before going any further, should I worry about the new workspace model? What will happen in the future?

[nskvortsov]

@lppedd You should not worry. The current API is far from being deprecated. The new workspace model is already used as underlying implementation, and now we started using it directly in some places.

[lppedd]

Sad news everyone. Doesn't work anymore for Tycho 4. I'm investigating.

[lppedd]

P2ResolverFactoryImpl: seems like an old version of the service is injected.
Here it's trying to invoke getService(Class), but we only have getService(String).

---

This is where the duplicated dependencies are extracted and used to construct the Tycho Maven plugin ClassRealm.

There is no conflict resolution, even thought a context value is injected.

[lppedd]

@nskvortsov I have noticed IJ doesn't support toolchains when building the Maven request.
I'm adding it quickly but would love to see it done in the main branch.

[lppedd]

Proposal: instead of relying only on the client code of IntelliJ, can we open Maven3XServerEmbedder to plugins? A plugin could contribute its own extension of Maven3XServerEmbedder. The important thing about external contribution is they should be able to control:

1. Maven dependencies (e.g. the provided version of Maven Core/Compat)
2. How the Maven context is initialized

We could have two extension points.

1. To alter the IJ MavenProjects submitted to the embedder
2. To actually get the custom embedder To do this we need the ability to extend MavenServer and select it based on project settings.
   In case of Tycho for example, I could take Maven36ServerImpl/Maven3ServerIndexerImpl/Maven36ServerEmbedderImpl as starting point

Anyway, a starting point is a good diagram of classes and a good cleanup 😄

[lppedd]

FYI I'm implementing a facade over Maven (using maven-embedded). This is going to be a pluggable system that have clear separation between Maven versions. I will then use this system to provide enhanced features for Tycho.

[lppedd]

So a small good news. I've introduced a new EP at the IJ resolver level (not in the embedder) to customize the to-be-resolved projects.

public interface MavenResolverCustomizer {
  boolean isEnabled();

  @NotNull
  Collection<MavenProject> customizeProjects(
    @NotNull final Collection<MavenProject> projects,
    @NotNull final MavenProgressIndicator indicator);
}

On the embedder level I removed every Tycho reference, and instead I always set MavenSession#setAllProjects with the actual projects list before calling the lifecycle listeners, and not only with Collections#singletonList. The lifecycle listeners get now called a single time, not once per project.

@nskvortsov do you happen to know why it was done like that?

[nskvortsov]

@lppedd the current loop with singletone lists looks like a nine-years-old suboptimal implementation. Can be improved.

[lppedd]

@nskvortsov I don't have much time to work on this now. But that piece of code isn't there anymore in the implementation I haven't pushed yet.

[nskvortsov]

This draft is abandoned. Please feel free to pick it up and refine.