support maven properties for different classpath entries and paths

[maggu2810]

The changes are split into two commits.

The first commit only changes some coding format. Mainly it adds final keyword where possible. If you agree that it doesn't make anything more bad, it would be great if we could use it.

The second commit contains the real code changes.

If you review the changes you should perhaps look at the second commit only.

Fixes: #24

[vorburger]

@sylvainlaurent is this something you would be interested in to look over?

@maggu2810 I'll review and hopefully merge it if @sylvainlaurent doesn't get to it. Thank You for the 2 separate commits. If you had even raised it into 2 PRs, then I would have merged the only formatting one ASAP, and taken more time to review the other one... 😄

And on the specific topic of adding final more FYI and for fun, the following is NOT a review comment for this PR, I really don't care about it in this context and am happy to merge this, BUT... 😸 ... personally I actually sit on the OTHER side of that particular debate! Using a quality control check such as http://errorprone.info/bugpattern/Var I prefer to assume final is default... and remove final, as started to do e.g. here https://git.opendaylight.org/gerrit/#/c/65151/. Totally off topic and just as an FYI.

[maggu2810]

Now there is only one commit remaining.
The plugin should be backward compatible and old setups should work as previously.

[ctron]

LGTM

[sylvainlaurent]

I'll have a look, hopefully in a few hours

[sylvainlaurent]

some thoughts:

• while we are adding m2e.eea.annotationpath.*properties, why not rename the global property with the same prefix? This would break existing users, unless we support both the old and new property.
• why do we make the different levels mutually exclusive? one could have external anotations at the maven-library level and also use lastnpe.org-provided jars for specific dependencies inside the maven-library level.
  Of course the "global" level should stay mutually exclusive with library-level since the global one just applies the same path to the library levels.

[maggu2810]

• while we are adding m2e.eea.annotationpath.*properties, why not rename the global property with the same prefix? This would break existing users, unless we support both the old and new property.

See #24 (comment)

For backwards compatibility for any other existing users, IMHO it's probably best not to change m2e.jdt.annotationpath, but have new large ones, something like say m2e.maven.annotationpath and m2e.jre.annotationpath.

why do we make the different levels mutually exclusive?

This PR adds a new feature, it does not change the logic that has already been present.
IMHO changing the logic should not done in a PR that adds "support maven properties for different classpath entries and paths".

[maggu2810]

@vorburger ping

[vorburger]

@maggu2810 sorry for the delay. I've finally looked this one over now, and can see what we're doing here.. and I have no objection to this. (And I won't "nitpick" on the use of Optional as arg which AFAIK generally isn't the idea, some people say Optional as return value only; or the extra private getSingleProjectWideAnnotationPath instead of just setting default arg in caller. This is a very small project so I want to encourage contributions instead of being a PITA! 😄 )

@sylvainlaurent are you OK if we merge this as is, or do you have reservations? Hope it's OK that if we do not hear from you within 1 week I'll merge this?

[vorburger]

@solf given your comments in #24 would you like to confirm or even test this PR to confirm that it would address "your" #24 and that could be closed when this is merged?

[solf]

@vorburger only partially -- it works but there's still no way to have a global default.

Global default can be achieved by e.g. modifying ClasspathConfigurator.getSingleProjectWideAnnotationPath like this:

        String property = properties.getProperty(propertyName);
        if (property == null) {
                property = System.getProperty(propertyName);
        }

With this in place it is possible to specify in eclipse.ini something like this:
-Dm2e.eea.annotationpath.maven=/ext-eclipse-annotations/maven

If someone's willing to explain to me how to create pull request for this, I would create one. I already spent a bunch of time trying to figure out how to get @maggu2810 changes locally via git -- and I failed completely. I only managed to download zip copy of repository with his changes so that I could test it (e.g. this didn't work for me: https://docs.gitlab.com/ee/user/project/merge_requests/#checkout-locally-by-modifying-git-config-for-a-given-repository -- or at least I couldn't figure out what the proper checkout command is).

[sylvainlaurent]

No reserve from me

[vorburger]

@solf I've commented over in #24, but will merge this change contributed by @maggu2810 now as @sylvainlaurent has no objections.