Make javax-annotations optional for E4 Injector #792

HannesWell · 2023-10-29T19:00:55Z

Rework the E4 InjectorImpl and introduce the AnnotationLookup class to handle a potential absence of javax-annotations at runtime.

Print a warning if the 'old' javax-annotations are available at runtime to encourage users to migrate.
The warning can be suppressed with the VM-property -Declipse.e4.inject.javax.warning=false.

Make the remaining imports of javax.inject/annotation packages optional.
Annotations who's classes cannot be loaded at runtime are silently ignored by the JVM, as if they where not declared.

Alternatively we could simply remove the javax meta-annotations applied to E4 annotations since the E4 injector can handle the jakarta-pendants. It would just be a break in case there are other tools out there that process those E4 annotations (like org.eclipse.e4.core.di.extensions.OSGiBundle or org.eclipse.e4.core.di.extensions.EventTopic). But I cannot access if that's the case.

Part of eclipse-platform/eclipse.platform.releng.aggregator#1056

This currently also includes the addition of separate javax-annotation tests in #791.

github-actions · 2023-10-29T19:03:16Z

Test Results

    591 files ±0     591 suites ±0 1h 16m 25s ⏱️ + 6m 39s
  3 799 tests +1   3 794 ✔️ +1   5 💤 ±0 0 ❌ ±0
12 003 runs +3 11 967 ✔️ +3 36 💤 ±0 0 ❌ ±0

Results for commit 2cc7b9f. ± Comparison against base commit b434b24.

♻️ This comment has been updated with latest results.

HannesWell · 2023-10-31T09:14:39Z

With the Addition of dedicated javax and jakarta Tests in#791 this now ready and reviews are more than welcome.

The only think left is to add a special test-case to the Jakarta Tests to ensure that the javax-classes are really absent in the test runtime.
But thinking about it, this might not work for I-build Tests since they use a full Installation.

HannesWell · 2023-11-01T17:53:38Z

@laeubi, @akurtakov, @mickaelistria or anybody else, do you want to review this or have any opnion about when to merge this (now or next release cycle)?

The only think left is to add a special test-case to the Jakarta Tests to ensure that the javax-classes are really absent in the test runtime.
But thinking about it, this might not work for I-build Tests since they use a full Installation.

Added this now, but I'm not sure if that will work in I-builds, but maybe we have to try that out.

merks · 2023-11-01T18:26:55Z

Do it now. If there are problems we know we can count on you to fix them.

HannesWell · 2023-11-01T20:21:38Z

Do it now. If there are problems we know we can count on you to fix them.

Then I'll fix the test and submit this on Friday evening so that I have time on the weekend to fix potential early regressions.

jukzi · 2023-11-03T16:47:26Z

...ime/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/AnnotationLookup.java

+	static final AnnotationProxy PRE_DESTROY = createProxyForClasses(jakarta.annotation.PreDestroy.class,
+			() -> javax.annotation.PreDestroy.class);
+	public static final AnnotationProxy POST_CONSTRUCT = createProxyForClasses(jakarta.annotation.PostConstruct.class,
+			() -> javax.annotation.PostConstruct.class);


doesn't javax fail at runtime when javax is not present due to optional?

That's right. But that is why this class is wrapped into a supplier, which is called in a try-catch block that catches NoClassDefFoundErrors.

The e4.core.tests explicitly got a case added to ensure that the javax.inject/annotation classes are not available in those tests-runtime so that a failure due to absense of those classes would show up in any of those tests.
And to ensure javax still works #791 was added.

Rework the E4 InjectorImpl and introduce the AnnotationLookup class to handle a potential absence of javax-annotations at runtime. Print a warning if the 'old' javax-annotations are available at runtime to encourage users to migrate. The warning can be suppressed with the VM-property -Declipse.e4.inject.javax.warning=false Make the remaining imports of javax.inject/annotation packages optional. Annotations who's classes cannot be loaded at runtime are silently ignored by the JVM, as if they where not declared. Part of eclipse-platform/eclipse.platform.releng.aggregator#1056

cdietrich · 2023-11-04T06:33:43Z

@HannesWell could this cause eclipse-mwe/mwe#275

HannesWell · 2023-11-04T06:52:10Z

@HannesWell could this cause eclipse-mwe/mwe#275

I can neither confirm nor negate that with confidence yet, but as far as I can see nothing mentions javax.
Nevertheless it could be causing a failure earlier.
It might be that Tycho surefire 2.x requires javax in the runtime, but I have to check that.

cdietrich · 2023-11-04T07:01:50Z

ENTRY org.eclipse.tycho.surefire.junit4 2 0 2023-11-04 07:30:50.192
!MESSAGE Could not resolve module: org.eclipse.tycho.surefire.junit4 [188]
Unresolved requirement: Import-Package: javax.annotation

iloveeclipse · 2023-11-04T07:03:33Z

Many JDT tests did not run at all with https://download.eclipse.org/eclipse/downloads/drops4/I20231103-1800.

Looks like sue javax errors, see for example https://download.eclipse.org/eclipse/downloads/drops4/I20231103-1800/testresults/ep430I-unit-cen64-gtk3-java17_linux.gtk.x86_64_17/directorLogs/director-org.eclipse.jdt.core.tests.model.log

Starting application: 7831
Installing org.eclipse.jdt.core.tests.model 3.12.200.v20231103-1659.
Installing org.eclipse.test 3.5.300.v20230714-0537.
Installing org.eclipse.ant.optional.junit 3.3.400.v20230714-1309.
Installing org.eclipse.test.performance 3.19.400.v20231103-2009.
Installing org.eclipse.test.performance.win32 3.1.600.v20230714-1309.
Cannot complete the install because one or more required items could not be found.
	Software being installed: Java Model Tests 3.12.200.v20231103-1659 (org.eclipse.jdt.core.tests.model 3.12.200.v20231103-1659)
	Missing requirement: Java Compiler Tests 3.13.200.v20231102-2239 (org.eclipse.jdt.core.tests.compiler 3.13.200.v20231102-2239) requires 'java.package; javax.annotation [1.3.0,2.0.0)' but it could not be found
	Cannot satisfy dependency:
		From: Java Model Tests 3.12.200.v20231103-1659 (org.eclipse.jdt.core.tests.model 3.12.200.v20231103-1659)
		To: osgi.bundle; org.eclipse.jdt.core.tests.compiler [3.4.0,4.0.0)
There were errors. See log file: /home/jenkins/agent/workspace/AutomatedTests/ep430I-unit-cen64-gtk3-java17/workarea/I20231103-1800/eclipse-testing/test-eclipse/eclipse/configuration/1699066775488.log
Installation failed.
!SESSION 2023-11-04 02:59:35.183 -----------------------------------------------
eclipse.buildId=4.30.0.I20231103-1800
java.version=17.0.2
java.vendor=Oracle Corporation
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Framework arguments:  -application org.eclipse.equinox.p2.director -flavor tooling -installIUs org.eclipse.jdt.core.tests.model,org.eclipse.test,org.eclipse.ant.optional.junit,org.eclipse.test.performance,org.eclipse.test.performance.win32 -repository file:/home/jenkins/agent/workspace/AutomatedTests/ep430I-unit-cen64-gtk3-java17/workarea/I20231103-1800/eclipse-testing/testRepo
Command-line arguments:  -application org.eclipse.equinox.p2.director -consoleLog -debug -flavor tooling -installIUs org.eclipse.jdt.core.tests.model,org.eclipse.test,org.eclipse.ant.optional.junit,org.eclipse.test.performance,org.eclipse.test.performance.win32 -repository file:/home/jenkins/agent/workspace/AutomatedTests/ep430I-unit-cen64-gtk3-java17/workarea/I20231103-1800/eclipse-testing/testRepo

!ENTRY org.eclipse.equinox.p2.director 4 10053 2023-11-04 02:59:45.481
!MESSAGE Cannot complete the install because one or more required items could not be found.
!SUBENTRY 1 org.eclipse.equinox.p2.director 4 0 2023-11-04 02:59:45.481
!MESSAGE Software being installed: Java Model Tests 3.12.200.v20231103-1659 (org.eclipse.jdt.core.tests.model 3.12.200.v20231103-1659)
!SUBENTRY 1 org.eclipse.equinox.p2.director 4 0 2023-11-04 02:59:45.481
!MESSAGE Missing requirement: Java Compiler Tests 3.13.200.v20231102-2239 (org.eclipse.jdt.core.tests.compiler 3.13.200.v20231102-2239) requires 'java.package; javax.annotation [1.3.0,2.0.0)' but it could not be found
!SUBENTRY 1 org.eclipse.equinox.p2.director 4 1 2023-11-04 02:59:45.481
!MESSAGE Cannot satisfy dependency:
!SUBENTRY 2 org.eclipse.equinox.p2.director 4 0 2023-11-04 02:59:45.481
!MESSAGE From: Java Model Tests 3.12.200.v20231103-1659 (org.eclipse.jdt.core.tests.model 3.12.200.v20231103-1659)
!SUBENTRY 2 org.eclipse.equinox.p2.director 4 0 2023-11-04 02:59:45.482
!MESSAGE To: osgi.bundle; org.eclipse.jdt.core.tests.compiler [3.4.0,4.0.0)

iloveeclipse · 2023-11-04T07:05:56Z

Should we mark lst build unstable?

merks · 2023-11-04T07:13:29Z

Some things to note. The bundle is in the update site:

https://download.eclipse.org/eclipse/updates/4.30-I-builds/I20231103-1800/plugins/jakarta.annotation-api_1.3.5.jar

And in the target platform:

https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/b6dfc4fc01625cf697b2e2a18a738952c60ab0f8/eclipse.platform.releng.prereqs.sdk/eclipse-sdk-prereqs.target#L711-L716

So the question arises, why would the package not be found when it's available in both places where one would expect to look.

HannesWell · 2023-11-04T07:20:59Z

So the question arises, why would the package not be found when it's available in both places where one would expect to look.

I was asking myself the same.
But what I noticed already, that should be fixed, is that the javax.inject providing bundle is not available in the repo.
I'll be at my Computer in about an hour an can fix that and analyze more.

merks · 2023-11-04T07:28:52Z

It is in this repo:

And that is the latest build at the moment:

Am I looking in the wrong place?

merks · 2023-11-04T07:31:06Z

I think it will remain in the repo because this bundle requires it:

laeubi · 2023-11-04T07:47:44Z

So the question arises, why would the package not be found when it's available in both places where one would expect to look.

Because it is not a requirement of the test runtime but of org.eclipse.tycho.surefire.junit4 what is added implicitly (even though I don't know why javax.annotation is required by JUnit4).

As a first test I would just add an extra requirement to the tycho-build ... not sure how to archive this for the eclipse-test-framework.

HannesWell · 2023-11-04T08:37:05Z

Am I looking in the wrong place?

I was referring to jakarta.**inject**.jakarta.inject-api, which seems to be missing now.

I think it will remain in the repo because this bundle requires it:

Looks like I forgot platform.swt when migrating javax to jakarta.
Will have a look at this and will check if there are more.
Once all are really migrated, jakarta.annotation-api version='1.3.5' should be gone from the repo too and we should add it explicitly to the repo again (or should we remove it so that those that don't want to migrate have some effort too)?

Looks like sue javax errors, see for example https://download.eclipse.org/eclipse/downloads/drops4/I20231103-1800/testresults/ep430I-unit-cen64-gtk3-java17_linux.gtk.x86_64_17/directorLogs/director-org.eclipse.jdt.core.tests.model.log

For JDT I created eclipse-jdt/eclipse.jdt.core#1527 already, but it probably needs adjustments and didn't get any attention yet.

So the question arises, why would the package not be found when it's available in both places where one would expect to look.

Because it is not a requirement of the test runtime but of org.eclipse.tycho.surefire.junit4 what is added implicitly (even though I don't know why javax.annotation is required by JUnit4).

I'll check why that's the case, but with recent Tycho that issue seems to be gone since this PR passed as well.
So yes the two solutions are probably to either migrate to Tycho 4 or to add a javax-dependency explicitly.

laeubi · 2023-11-04T08:42:06Z

Important parts in bold ;-)

but with recent Tycho that issue seems to be gone since this PR passed as well.

Fix DependencyComputer missing bundles when they have different versions eclipse-tycho/tycho#2841

HannesWell · 2023-11-04T12:25:11Z

Now that the following PRs are merged

should I trigger an I-build again to see if the JDT tests work again and see if also the javax.inject providing bundle is gone as an ultimate test to be sure there is no dependency anymore.
I can then later provide a PR to explicitly add it to the sdk-repo.

iloveeclipse · 2023-11-04T12:25:56Z

Yes, please

HannesWell · 2023-11-04T12:29:40Z

Just started the 100th I-build for 4.30:
https://ci.eclipse.org/releng/job/Builds/job/I-build-4.30/100/

HannesWell · 2023-11-04T16:38:05Z

Some things to note. The bundle is in the update site:

https://download.eclipse.org/eclipse/updates/4.30-I-builds/I20231103-1800/plugins/jakarta.annotation-api_1.3.5.jar

With the mentioned PRs no javax.inject/annotation providing bundle is in the sdk-repo anymore:
https://download.eclipse.org/eclipse/updates/4.30-I-builds/I20231104-0830/

This is a good news since it confirms that they are not required anymore (only optionally required by the e4 bundles that have optional support for it).

I just created eclipse-platform/eclipse.platform.releng.aggregator#1509 to ensure it is added back to the repo until we remove support for javax entirely.

This was referenced Oct 29, 2023

Support jakarta.inject / jakarta.annotation in E4 as an alternative #696

Merged

Migrate javax.inject and javax.annotations to Jakarta eclipse-platform/eclipse.platform.releng.aggregator#1056

Closed

HannesWell force-pushed the optional-javax branch from 5787e93 to a05089c Compare October 29, 2023 19:43

HannesWell mentioned this pull request Oct 30, 2023

Add o.e.e4.core.javax.tests as javax-using copy of o.e.e4.core.tests #791

Merged

HannesWell force-pushed the optional-javax branch from a05089c to fdcb694 Compare October 30, 2023 22:17

HannesWell marked this pull request as draft October 30, 2023 22:45

HannesWell force-pushed the optional-javax branch from fdcb694 to 8ce8931 Compare October 31, 2023 09:06

HannesWell marked this pull request as ready for review October 31, 2023 09:14

HannesWell force-pushed the optional-javax branch from 8ce8931 to c503887 Compare November 1, 2023 17:49

HannesWell force-pushed the optional-javax branch from c503887 to fe7a07c Compare November 2, 2023 23:26

jukzi reviewed Nov 3, 2023

View reviewed changes

HannesWell force-pushed the optional-javax branch from fe7a07c to 50ae8be Compare November 3, 2023 16:58

HannesWell force-pushed the optional-javax branch from 50ae8be to 2cc7b9f Compare November 3, 2023 19:19

HannesWell merged commit 6beceea into eclipse-platform:master Nov 3, 2023
14 checks passed

HannesWell deleted the optional-javax branch November 3, 2023 20:12

HeikoKlare mentioned this pull request Nov 3, 2023

Ensure proper cleanup after WorkspaceSessionTests #805

Merged

HannesWell mentioned this pull request Nov 4, 2023

Add javax.inject/annotation providing bundles to eclipse.sdk repository eclipse-platform/eclipse.platform.releng.aggregator#1509

Merged

cdietrich mentioned this pull request Nov 5, 2023

Eclipse MAT fails to run its tests with 4.30-I-builds/I20231104-0830/ #821

Closed

HannesWell mentioned this pull request Nov 12, 2023

Support for jakarta namespace eclipse-sisu/sisu-project#92

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make javax-annotations optional for E4 Injector #792

Make javax-annotations optional for E4 Injector #792

HannesWell commented Oct 29, 2023

github-actions bot commented Oct 29, 2023 •

edited

Loading

HannesWell commented Oct 31, 2023

HannesWell commented Nov 1, 2023

merks commented Nov 1, 2023

HannesWell commented Nov 1, 2023

jukzi Nov 3, 2023

HannesWell Nov 3, 2023

cdietrich commented Nov 4, 2023

HannesWell commented Nov 4, 2023

cdietrich commented Nov 4, 2023

iloveeclipse commented Nov 4, 2023

iloveeclipse commented Nov 4, 2023

merks commented Nov 4, 2023

HannesWell commented Nov 4, 2023

merks commented Nov 4, 2023

merks commented Nov 4, 2023

laeubi commented Nov 4, 2023

HannesWell commented Nov 4, 2023 •

edited

Loading

laeubi commented Nov 4, 2023

HannesWell commented Nov 4, 2023

iloveeclipse commented Nov 4, 2023

HannesWell commented Nov 4, 2023

HannesWell commented Nov 4, 2023

Make javax-annotations optional for E4 Injector #792

Make javax-annotations optional for E4 Injector #792

Conversation

HannesWell commented Oct 29, 2023

github-actions bot commented Oct 29, 2023 • edited Loading

Test Results

HannesWell commented Oct 31, 2023

HannesWell commented Nov 1, 2023

merks commented Nov 1, 2023

HannesWell commented Nov 1, 2023

jukzi Nov 3, 2023

Choose a reason for hiding this comment

HannesWell Nov 3, 2023

Choose a reason for hiding this comment

cdietrich commented Nov 4, 2023

HannesWell commented Nov 4, 2023

cdietrich commented Nov 4, 2023

iloveeclipse commented Nov 4, 2023

iloveeclipse commented Nov 4, 2023

merks commented Nov 4, 2023

HannesWell commented Nov 4, 2023

merks commented Nov 4, 2023

merks commented Nov 4, 2023

laeubi commented Nov 4, 2023

HannesWell commented Nov 4, 2023 • edited Loading

laeubi commented Nov 4, 2023

HannesWell commented Nov 4, 2023

iloveeclipse commented Nov 4, 2023

HannesWell commented Nov 4, 2023

HannesWell commented Nov 4, 2023

github-actions bot commented Oct 29, 2023 •

edited

Loading

HannesWell commented Nov 4, 2023 •

edited

Loading