Fixed #12: Restrict lifecycle-mapping

[vorburger]

Trigger the ClasspathConfigurator only for "eclipse" or "jdt" compiler.
Otherwise running the extension is just unnecessary overhead.

Similar to
https://github.com/jbosstools/m2e-jdt-compiler/blob/master/org.jboss.tools.m2e.jdt.core/lifecycle-mapping-metadata.xml

[vorburger]

I'm not sure if both jdt as well as eclipse are required.. I've only used jdt in the examples, and eclipse seems to be for plexus-compiler-eclipse, which ... is old? Would the JDT null analysis ever work with it? I was tempted to not add it, but then went for "better safe than sorry" - unless you guys all agree its not needed?

@kwin this is for your #12 - looks about right?

@fbricon OK?

[fbricon]

I know next to nothing about how null analysis works, but I think doesn't hurt to have eclipse set. JDT null analysis running in Eclipse doesn't care what maven compilerId is running in CLI. You can even set it for javac-with-errorprone too. Now whether null analysis is enabled (and how) during CLI, I have no idea.

[vorburger]

Actually, guys... especially @kwin ... on thinking this through further, I now think we do NOT want this!

JDT null analysis running in Eclipse doesn't care what maven compilerId is running in CLI.
You can even set it for javac-with-errorprone too.

This feedback got me thinking more about this change, based on @kwin issue #12 ... I now think we got this wrong - IMHO the point of the eclipse-external-annotations-m2e-plugin is (only) that it configures JDT null analysis properties for us (incl. EEA paths). Whether or not one chooses to also enforce this same on the mvn CLI build is an entirely different question, no? Sure I'm the first one to agree that it is nice to do that. But should this M2E extension enforce that, and only work for POM which do that? I actually don't think so, no. One could well just be interested in using this plugin to have it set EEA path from Maven dependencies - without ever wanting to enforce it on the CLI mvn build (thus not changing the compilerId). Therefore, IMHO we proably should NOT at all restrict the lifecycle-mapping to trigger the ClasspathConfigurator only for certain compilerId?

So I would actually like to close this PR and issue #12, without merging. OK with you @kwin or NOK?

BTW: If @kwin you had raised this for M2E workspace performance reasons (which is very important! had you noticed it slowing your workspace down, I'm curious?) then I think we should rather try to find some other way for this plugin to add the least overhead possible to M2E if it's "not used" (as in if it has nothing to do , because there are no properties AND no EEA to configure).

I know next to nothing about how null analysis works

@fbricon we got that covered here in this project... see http://www.lastnpe.org !

Now whether null analysis is enabled (and how) during CLI, I have no idea.

If you're interested, you can see how this can be done in https://github.com/lastnpe/eclipse-null-eea-augments/blob/master/examples/maven/parent/pom.xml. And these slides here have some background.

[kwin]

IMHO there are different aspects of this m2e extension. It is used for the following aspects:

1. for setting the annotationpath option (https://help.eclipse.org/oxygen/index.jsp?topic=%2Forg.eclipse.pde.doc.user%2Ftasks%2Fpde_compiler_options.htm) to the appropriate value from the m-c-p configuration option (the value classpath cannot even with Eclipse Oxygen be set through the UI, therefore this functionality is crucial for the EEA to be picked up). This option can only be extracted in case the compiler id is either eclipse or jdt because no other compilers support that option. I am not 100% sure this option is really useful within Eclipse because it seems that in any case all EEA dependencies are automatically bound. In any case it seems that all compiler options are being transferred to the Eclipse Project's Java Configuration. I am not sure how useful that is and how much of this functionality is already covered by other/standard m2e lifecycle mappings.
2. For linking the correct EEA paths to the given dependencies.

For 1. it is useful to also consider the compiler id, because only the options given for either eclipse or jdt make sense to transfer over to the java compiler settings of the according Eclipse project, for 2. it should not be used.

Therefore I agree that in general disabling the m2e extension for other compiler ids is not a good idea.

But maybe you can quickly comment on point 1. Which compiler options are useful to transfer from the m-c-p settings to the Eclipse Project's compiler settings? Is this also useful in case the CLI build uses javac? Then an according mapping table should be used, but IMHO the most important compiler configuration option are already being configured through some other m2e extension (which ship with m2e by default).

[nfekete]

Not sure if it would affect it in any way, but there's the aspectj compiler, which handles pretty much a super-set or java, probably using jdt in the background. In eclipse it appears as a builder in a project, it's named AspectJ Builder and replaces the Java Builder in the project properties. Although I'm not completely sure if nullness checking is working with an aspectj project, but I agree with the conclusion that the presence of the metadata itself would not hurt.