[releng] Bypass jdt.annotation Java 7/8 difficulties

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 449520 | | Status | RESOLVED FIXED | | Importance | P3 normal | | Reported | Oct 31, 2014 16:36 EDT | | Modified | May 26, 2015 06:34 EDT | | Version | 5.0.0 | | See also | 449110, 434307 | | Reporter | Ed Willink |

Description

See bug 449110.

The original org.eclipse.jdt.annotation for Java 5/6/7 had limited but useful source-only capabilities.

The Java 8 version exploits type annotations and so became a much more powerful non-source functionality.

These two versions are both useful but incompatible, and are irresponsibly provided as different versions of the same plugin despite the lack of support for source versioning.

This has caused endless trouble since the Java 8 version was forced on us.

The Java 5/6/7 functionality that we must use till we can lowerbound at Java 8 is a source-only capability, so we can redistribute it ourselves.

Preliminary experiments show that an org.eclipse.ocl.jdt.annotation7 containing the org.eclipse.jdt.annotation code works just as it did before. Since it has a clearly distinct bundle-id, P2 and similar tooling should not be confused.

The only difficulty seems to be the correct auto-generation of directives into code generated EMF plugins, but that was probably broken and unuseable anyway.

[eclipse-ocl-bot]

By Ed Willink on Oct 31, 2014 16:49

(In reply to Ed Willink from comment #0)

The only difficulty seems to be the correct auto-generation of directives into code generated EMF plugins, but that was probably broken and unuseable anyway.

So long as Bug 401862 imposes a OCL codegen dependency on users, there is no free choice for users to use Java 8 annotations, so we can force Java 7. Else users must suppress null annotations in their generated code.

[eclipse-ocl-bot]

By Ed Willink on Nov 01, 2014 07:16

The ewillink/449520 branch builds successfully on Hudson. The resulting workspace has no org.eclipse.jdt.annotation at all demonstrating independence from the JDT java 7/8 confusion. org.eclipse.ocl.jdt.annotation7 is included in the examples (and editors) feature since users who code generate OCL to Java may need to reference it.

The three classic plugins e.g. ocl.common that have @NonNull annotations are revised to put org.eclipse.ocl.jdt.annotation7 on their build path rather than requiring the org.eclipse.jdt.annotation 1.1-2.0 bundle. This a reversion to Kepler.

The org.eclipse.ocl.jdt.annotation7 has to be in the classic feature and all annotated plugins must have build.properties so that the build path is defined for users who Import Plugin as Source.

The normal source plugin seems useless for org.eclipse.ocl.jdt.annotation7; the sources are not fetched. Putting them in the non-source plugin ensures that Import Plugin with source works. This perhaps provides more insight on platform/JDT bugs. Without in-workspace org.eclipse.ocl.jdt.annotation7 source, JDT navigation of @NonNull takes the user to org.eclipse.jdt.annotation despite that not being on any classpath. It would appear that there is an implicit package name to plugin binding.

[eclipse-ocl-bot]

By Ed Willink on Nov 02, 2014 05:17

With @NonNull source in the 'binary' plugins the problems seem solved. There is a minor gotcha for those who import as source; an Eclipse restart is needed to ensure that the imported plugins are properly analyzed.

Please review the net effect of commits on branch ewillink/449520.

?? do we want this in Luna SR2 as well ?? probably not. Source users are prepared to use more recent code/GIT.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Nov 04, 2014 12:58

I'm probably far behind of the problem, and

• you mention lots of complains without evidences, to better understand it.
• No repro neither.
• This quite shocks me:\ "This has caused endless trouble since the Java 8 version was forced on us."

Trying to help in somehow, I'll have a closer look to the branch, but a priori the following doesn't sound good:

(In reply to Ed Willink from comment #2)

The org.eclipse.ocl.jdt.annotation7 has to be in the classic feature and all annotated plugins must have build.properties so that the build path is defined for users who Import Plugin as Source.

build.properties is something related to building a jar, so I don't see any reasonable argument to have to distribute it in the binaries. At most, I'd introduce it as part of the sources jar, not into the binaries one. On the other hand, I've checked that the IDE complains with a warning if you include the build.properties file as part of the sources >.<

Providing that you are bundling your own plugin with different namespace, could you explain me what's wrong with the optional dependency in the manifest?.

-1 for including the change in previous versions.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Nov 04, 2014 13:56

Currently some plugins do and some don't have plugin.properties distributed in the binary; no particular reason.

Yes, build.properties is normally instructions for the JAR, but it is also the additional classpath, which is what we need and must enforce to ensure that Java 7 rather than Java 8 @NonNull is used.

(See also Bug 434307.)

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Nov 05, 2014 09:00

(In reply to Ed Willink from comment #5)

Currently some plugins do and some don't have plugin.properties distributed in the binary; no particular reason.

Yes, build.properties is normally instructions for the JAR, but it is also the additional classpath, which is what we need and must enforce to ensure that Java 7 rather than Java 8 @NonNull is used.

(See also Bug 434307.)

Hi Ed,

plugin.properties is OK in the binary jar, because you need it at run-time. Including the build.properties is not OK. As far as I understand, you are curing the case of importing a plugin as a source in the workspace, so what you'd need is including that file in the source bundle instead. That would the best option, as long as the annoying warning didn't come up in the IDE. Tough.

After reading some other bugzillas you mentioned, it seems that using the optional dependency in the manifest might potentially cause issues if our upstream dependencies re-export the jdt.annotation bundle, which was happening in the past but it's not occurring anymore, right?... The point now is, since we are using our own bundle with a different namespace, should that potential problem not appear anymore ?

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Nov 06, 2014 06:04

You don't seem to understand the problem.

Prior to Luna M6, there was only org.eclipse.jdt.annotation 1.x that was a source only capability whose recommended usage required build.properties to contain

additional.bundles = org.eclipse.jdt.annotation

so that the Java 5/6/7 annotation capabilities could be used.

Java 8 introduced type annotations within class files and org.eclipse.jdt.annotation 2.x exploits them. Excellent for Java 8 users. Disastrous for Java 5/6/7 users.

Suggested workaround is to reference org.eclipse.jdt.annotation 1.x as a run-time requirement so that versioning nearly 'works'. It is this workaround that is just not fit for purpose. We are using a source annotation capability but fighting with tooling that has no support for source path versioning. Consequently at every opportunity tooling gives us the wrong version.

Solution is to go back to where we were both Luna M6; source annotations on the build classpath. Unfortunately there is now an incompatible plugin so we must change name to avoid it. Hence org.eclipse.ocl.jdt.annotation.

Now we just have to ensure that source code uses it. Which now requires build.properties to contain

additional.bundles = org.eclipse.ocl.jdt.annotation

It is certainly reasonable to expect that it would be sufficient to put build.properties in the sources plugin. But that doesn't work. Putting it in the binary plugin does work. It seems the source plugin is for unzipping to src/... only.

The only reason I can see to exclude build.properties from the binary plugin is bloat. build.properties is a very small file so the bloat is insignificant and it turns out that quite a few, perhaps half, or our plugins have build.properties in the binary plugin anyway.

If I had nothing else to do there are probably at least four platform/JDT bugs to pursue in this area. I just do not have time. And any fixes are unlikely to be in Luna SR2, so backward installation on Luna will be a permanent problem. The branch demonstrates a perfectly adequate solution that will hopefully tide us over for a couple of years till we can move to Java 8, or until a credible Java 7 solution is made available. At least we do not have to edit any source files.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Nov 06, 2014 07:33

Again it's not clearly to me that you have checked that the tooling gets confused when you (optionally) depend on a plugin with a different namespace, because now why is it a problem of "versioning" ? Is just you don't like the runtime optional dependency (the workaround for one problem)? because I don't like a build.properties in the binary jar (another workaround for another problem) :P

I think that the confusion has been that you are trying to solve different issues at once:

1. Avoid the problems the tooling causes developers when adding the optional dependency to the manifest. And I'm OK to the alternative, in essence use our own annotations plugin and, use that compile-time dependency.
2. Make that build.properties be in your workspace when you import an installed plugin as a source, which I guess it should not have worked ever, even prior to Luna M6 (unless we were shipping the build.properties in the jars)

For the last problem I could:\ a) either recall your own argument:\ "?? do we want this in Luna SR2 as well ?? probably not. Source users are prepared to use more recent code/GIT."\ b) or consider to check that the tooling doesn't get confused when the run-time optional dependency is set on our plugin.\ c) or remind you that as project lead you have the final decision of shipping that bloating build.properties files for every plugin which needs that compile-time dependency (I think Dennis Roy would prefer to a void this one ;P).

I think we have exposed possible different alternatives, which is good. Just let me know which one you like, to have some deep review to the final branch and built artefacts.

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Nov 10, 2014 05:49

Let's leave Luna unchanged.

After some experiments with builds and installs, this change seems to make things more tolerant. Code changed to org.eclipse.ocl.jdt.annotation7 is using source annotations and source declarations and so has no run-time requirement, classpath effect, or binding consequence. Mixing and matching is not a problem so long as all org.eclipse.jdt.annotation ensure that the 1.1.0 version is used and that the 2.0.0 version is not conflicting.

Pushed to master for M3 and merged to ewillink/ocl25.