apache / netbeans

Apache NetBeans
https://netbeans.apache.org/
Apache License 2.0
2.62k stars 840 forks source link

Jakarta EE 10 - Platform/API/RI support #4692

Closed jGauravGupta closed 1 year ago

jGauravGupta commented 1 year ago

Signed-off-by: Gaurav Gupta gaurav.gupta@payara.fish This PR adds support for Jakarta EE 10 Platform, GlassFish RI v7.x.x, and Payara v6.x.x.

jGauravGupta commented 1 year ago

Help needed from @juneau001 to publish Maven Archetype for Jakarta EE 10 project template. https://github.com/apache/netbeans/blob/master/enterprise/maven.j2ee/src/org/netbeans/modules/maven/j2ee/ui/wizard/archetype/Bundle.properties IMO a common archetype needs to be published to support the range of the Jakarta EE Platform version as a parameter to the Maven Archetype.

juneau001 commented 1 year ago

Help needed from @juneau001 to publish Maven Archetype for Jakarta EE 10 project template. https://github.com/apache/netbeans/blob/master/enterprise/maven.j2ee/src/org/netbeans/modules/maven/j2ee/ui/wizard/archetype/Bundle.properties IMO a common archetype needs to be published to support the range of the Jakarta EE Platform version as a parameter to the Maven Archetype.

I have added a Jakarta EE 10 archetype for full profile: https://mvnrepository.com/artifact/io.github.juneau001/webapp-jakartaee10 However, I will still need to add an archetype for web and core profiles

jGauravGupta commented 1 year ago

Need help related to sha1 code of new external binaries. I have copied the sha1 from the following links, resulting in a build failure. On using the sha1 in uppercase, build passes but commit validation tests fail.

https://repo1.maven.org/maven2/jakarta/platform/jakarta.jakartaee-api/10.0.0/jakarta.jakartaee-api-10.0.0.jar.sha1 https://repo1.maven.org/maven2/jakarta/platform/jakarta.jakartaee-web-api/10.0.0/jakarta.jakartaee-web-api-10.0.0.jar.sha1 https://repo1.maven.org/maven2/jakarta/platform/jakarta.jakartaee-api/10.0.0/jakarta.jakartaee-api-10.0.0-javadoc.jar.sha1

matthiasblaesing commented 1 year ago

The uppercase variant is the right one. It would help to know what failed.

matthiasblaesing commented 1 year ago

This is the problem:

found version 55.0 in jakarta/servlet/jsp/el/ImplicitObjectELResolver$ImplicitObjects$4.class in /home/runner/work/netbeans/netbeans/nbbuild/netbeans/enterprise/modules/ext/jakarta.jakartaee-api-10.0.0.jar

The test ensures, that you declare the right java base line version for the module is declared. See here: http://bits.netbeans.org/dev/javadoc/org-openide-modules/org/openide/modules/doc-files/api.html. You can declare a dependency on the JVM as minimum version. I think adding this:

OpenIDE-Module-Java-Dependencies: Java > 11

should fix the commit validation. Please check, that the module can still be loaded on JDK 11. Contrary to the implication of the symbol I read the code to actually mean ">=":

https://github.com/apache/netbeans/blob/781c6b1f9d8c4d5f83a3de8f7579539915b44bf4/platform/o.n.bootstrap/src/org/netbeans/Util.java#L84-L106

ehsavoie commented 1 year ago

Please note that WildFly 27 is JakartaEE 10 :) I've fixed the issue that was making that plugin unuseable :( If you need a hand I'd love to help here

jGauravGupta commented 1 year ago

Hi @ehsavoie , Your help will be highly appreciated in this PR. Thanks!

matthiasblaesing commented 1 year ago

@jGauravGupta I had a look at the commit validation errors. Looking at the logic it seems to make wrong assumptions. See #4775

mbien commented 1 year ago

travis is reporting compiler errors in "Compile all modules and tests":

 [nb-javac] /home/travis/build/apache/netbeans/enterprise/web.project/test/unit/src/org/netbeans/modules/web/project/WebProjectTest.java:114: error: cannot find symbol

 [nb-javac]         JavaEEProjectSettings.setProfile(webProject, Profile.JAKARTA_EE_91_WEB);

 [nb-javac]                                                             ^

 [nb-javac]   symbol:   variable JAKARTA_EE_91_WEB

 [nb-javac]   location: class Profile

 [nb-javac] /home/travis/build/apache/netbeans/enterprise/web.project/test/unit/src/org/netbeans/modules/web/project/WebProjectTest.java:116: error: cannot find symbol

 [nb-javac]         assertEquals(Profile.JAKARTA_EE_91_WEB, obtainedProfileJakartaEE91);

 [nb-javac]                             ^

 [nb-javac]   symbol:   variable JAKARTA_EE_91_WEB

 [nb-javac]   location: class Profile

RAT is failing with:

/home/runner/work/netbeans/netbeans/scratch/nbbuild/build.xml:2216: Failed Rat test(s):
Unapproved license in 4 file(s) 
https://github.com/apache/netbeans/tree/master/enterprise/j2ee.common/src/org/netbeans/modules/j2ee/common/dd/resources/beans-4.0.xml
https://github.com/apache/netbeans/tree/master/enterprise/j2ee.common/src/org/netbeans/modules/j2ee/common/dd/resources/web-fragment-6.0.xml
https://github.com/apache/netbeans/tree/master/enterprise/j2ee.common/src/org/netbeans/modules/j2ee/common/dd/resources/ear-10.xml
https://github.com/apache/netbeans/tree/master/enterprise/j2ee.common/src/org/netbeans/modules/j2ee/common/dd/resources/web-6.0.xml

veryfy libs and licenses too:

/home/runner/work/netbeans/netbeans/nbbuild/build.xml:1623: Failed VerifyLibsAndLicenses test(s):
Some license files have incorrect headers
enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar is not associated with any license file
Some binaries are duplicated (edit nbbuild/antsrc/org/netbeans/nbbuild/extlibs/ignored-overlaps as needed)
docs/api/package-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and package-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical
docs/api/type-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and type-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical
docs/api/member-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and member-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical

@matthiasblaesing @neilcsmith-net Is it still useful to have the CV matrix test on JDK 8 with IDE modules? Would be unfortunate if this would block features like this PR given that the download page says JDK 11+ since NetBeans 12.6.

ehsavoie commented 1 year ago

@jGauravGupta I don't see the wildfly support I PRed on your branch

jGauravGupta commented 1 year ago

Looking forward to progress on EE10, I need help with CI failures.

mbien commented 1 year ago

hi @jGauravGupta,

we might be able to turn the commit validation tests off on JDK 8. I personally don't think we should put time into it to try to fix them given that NB doesn't technically support running on JDK 8 anymore anyway. But we should discuss this first on the mailing list i believe. @neilcsmith-net right?

the second failure are the lib validation checks:

 BUILD FAILED
/home/runner/work/netbeans/netbeans/nbbuild/build.xml:1625: Failed VerifyLibsAndLicenses test(s):
Some binaries are duplicated (edit nbbuild/antsrc/org/netbeans/nbbuild/extlibs/ignored-overlaps as needed)
docs/api/package-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and package-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical
docs/api/type-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and type-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical
docs/api/member-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and member-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical

I don't have a lot of experience with this ignore list but the message basically says how to fix it: Some binaries are duplicated (edit nbbuild/antsrc/org/netbeans/nbbuild/extlibs/ignored-overlaps as needed) @matthiasblaesing can we put those jars simply on the ignore list?

The failure on travis can be ignored (the test is known to fail sometimes). The next workflow run will only use github CI since everything got migrated.

ehsavoie commented 1 year ago

@mbien all the more so as Jakarta EE 10 requires Java 11 :)

mbien commented 1 year ago

@mbien all the more so as Jakarta EE 10 requires Java 11 :)

@ehsavoie yeah, many third party libs are also dropping JDK 8 support. The next lucene or maven-indexer updates as in this draft #4999 would basically move most of the maven support modules to JDK 11.

We could still fix the CV tests by not letting the test load JDK incompatible modules but why bother at this point.

neilcsmith-net commented 1 year ago

we might be able to turn the commit validation tests off on JDK 8. I personally don't think we should put time into it to try to fix them given that NB doesn't technically support running on JDK 8 anymore anyway. But we should discuss this first on the mailing list i believe. @neilcsmith-net right?

Yes, a discussion for the mailing list, once we know what (sub)projects still require JDK 8, and for what modules.

Let's separate that question out from whether there is a legitimate problem here. As currently set up, any module that requires JDK 8+ needs to specify that in its requirements. I think the (or at least a) problem is j2ee.kit not requiring JDK 11 and/or specifying the dependency optionally. That's possibly a discussion for the mailing list too. Although does the EE10 support strictly require the IDE modules to be running on JDK 11+ or just the projects?

I don't have a lot of experience with this ignore list but the message basically says how to fix it: Some binaries are duplicated (edit nbbuild/antsrc/org/netbeans/nbbuild/extlibs/ignored-overlaps as needed) @matthiasblaesing can we put those jars simply on the ignore list?

Somewhat depends if the duplication is expected / intended?!

As for the addition of the NB17 milestone, would be nice to get this fixed up and in inside the next week.

ehsavoie commented 1 year ago

@neilcsmith-net I won't speak for other JakartaEE modules but for WildFly I will load some of WildFly classes which are now compiled with target Java 11. Also the JakartaEE 10 API module is Java 11 (but I think this will be used only by the project itself).

matthiasblaesing commented 1 year ago

the second failure are the lib validation checks:

 BUILD FAILED
/home/runner/work/netbeans/netbeans/nbbuild/build.xml:1625: Failed VerifyLibsAndLicenses test(s):
Some binaries are duplicated (edit nbbuild/antsrc/org/netbeans/nbbuild/extlibs/ignored-overlaps as needed)
docs/api/package-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and package-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical
docs/api/type-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and type-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical
docs/api/member-search-index.zip in enterprise/jakartaee10.platform/external/generated-jakarta.jakartaee-api-10.0.0-javadoc.jar and member-search-index.zip in enterprise/jakartaee10.platform/external/jakarta.jakartaee-api-10.0.0-javadoc.jar are identical

I don't have a lot of experience with this ignore list but the message basically says how to fix it: Some binaries are duplicated (edit nbbuild/antsrc/org/netbeans/nbbuild/extlibs/ignored-overlaps as needed) @matthiasblaesing can we put those jars simply on the ignore list?

Yes this should go into the ignore list, with a comment like: Jakarta EE API doc is repackaged to align with old NetBeans format, embedded indexes are thus identically and can be ignored.

mbien commented 1 year ago

Yes, a discussion for the mailing list, once we know what (sub)projects still require JDK 8, and for what modules.

someone please start this thread on the dev list - i still have PTSD from the last JDK 8 thread :)

neilcsmith-net commented 1 year ago

@mbien well, that's probably better left for the NB18 timeframe, and is somewhat tangential to fixing the issues to get this in. Mind you, I'm not quite sure what the correct module voodoo is to get this to work as required - possibly OpenIDE-Module-Recommends in j2ee.kit, if not bumping the whole kit to require JDK 11?

mbien commented 1 year ago

@neilcsmith-net the thing is that if I can decide to spend my freetime on making progress in things like #4904 or trying to fix tests on JDKs NB doesn't support anymore its an easy choice. One benefits the project the other one is just added extra steps in my view :)

It would be unfortunate if the last thing what blocks this PR from getting merged to NB 17 is because a CV test on JDK 8 doesn't work. But if this is getting moved to NB 18 for other reasons anyway... its all good then there is no need to hurry. Would be good to get some of the EE stuff in though to start catching up in that area.

morvael commented 1 year ago

I'd very much like this to work in NB 17. For now I have to keep xhtml and taglib files using EE7-9 versions and namespaces for the editor not to explode in red, while the app actually works by the grace of WildFly 27 which somehow doesn't complain about those mismatches. And WildFly 27 as well as Jakarta EE 10 do not work on Java 8, so why are those tests failing so important?

neilcsmith-net commented 1 year ago

I'd like to see this in NB17 too if we can. I don't care about tests on JDK8 much either. But don't assume that test failing there isn't telling you something valid!

Maybe add the JDK 11 requirement to the kit first and see if the test passes and everything still works as expected then?!

mbien commented 1 year ago

@neilcsmith-net I tried OpenIDE-Module-Recommends locally and it didn't pass, I see @jGauravGupta just pushed OpenIDE-Module-Java-Dependencies and it didn't help either.

But everything else is resolved now which is great!

mbien commented 1 year ago

restarting build with all tests enabled just to check what the VSCode extension tests do (they might try to load this module too).

mbien commented 1 year ago

ok they are green which is good to know. Going to take one for the team and open a thread on the dev list.

neilcsmith-net commented 1 year ago

OK, looks like the test might be broken?! Thread on dev@ might be good, but maybe should disable commit validation test on JDK 8 temporarily at least to get this in? I'll try and take a look sometime - it's not just about passing on JDK 8 - I presume we'll have the same issue with this test on JDK 11 when we bring in JDK 17+ modules at some point.

mbien commented 1 year ago

mail is already sent, see you there ;)

I presume we'll have the same issue with this test on JDK 11 when we bring in JDK 17+ modules at some point.

this problem is largely avoidable by not creating a situation where half the project moves to a different JDK version. We really should try to keep things simple. When we move to JDK X we stop testing on X-1 even when some of it might still work.

I wouldn't like to see JDK 17 editor modules now unless we bump the IDE requirement to 17, we would have to explain this on release notes, have more complicated tests etc. All unnecessary if avoidable IMO. If its a small separate feature.. fine, but nothing in the core please.

matthiasblaesing commented 1 year ago

OK, looks like the test might be broken?! Thread on dev@ might be good, but maybe should disable commit validation test on JDK 8 temporarily at least to get this in? I'll try and take a look sometime - it's not just about passing on JDK 8 - I presume we'll have the same issue with this test on JDK 11 when we bring in JDK 17+ modules at some point.

In this case the test is questionable in its current form. The problem is:

org.netbeans.modules.j2ee.kit depends on org.netbeans.modules.jakartaee10.platform and org.netbeans.modules.jakartaee10.api which are JDK 11+ only. The test accounts for modules that can't be loaded because the JDK requirement is not satisfied. But the kit fails to load because transitive modules can't be loaded.

The test is right in that the j2ee kit is broken on a JDK < 11.

neilcsmith-net commented 1 year ago

The test accounts for modules that can't be loaded because the JDK requirement is not satisfied.

Does it?! That's why I hoped adding the Java dependency on the kit might get it to pass. Other JDK 11+ modules use token provision to enable rather than dependency though.

neilcsmith-net commented 1 year ago

@neilcsmith-net I tried OpenIDE-Module-Recommends locally and it didn't pass, I see @jGauravGupta just pushed OpenIDE-Module-Java-Dependencies and it didn't help either.

@mbien @jGauravGupta I think this might work - https://github.com/neilcsmith-net/netbeans/commit/5d3aa156d7807aeec74821ec0911c6bebdafc27d

Did you add is.autoload=true? Took me a couple of failed attempts to realise that was the missing element compared to the working Disco module. Couldn't get recommends to work with the module name, so added tokens and provides. Passes tests locally and running on JDK 8 and JDK 11 seems to work as expected. EE area is not one I know, though!

mbien commented 1 year ago

@neilcsmith-net I did try autoload locally but I believe only combined with requires. The permutations I tried was recommends, requires, requires+autoload :)

Awesome that it works!

jGauravGupta commented 1 year ago

Big thanks, All tests passed finally :)

neilcsmith-net commented 1 year ago

We need to get this merged in the next 24hrs if it's going into NB17. Would be a shame to miss the cut. I'm normally happy to help with squashing commits if need be, but will not be able to until after the cutoff. @jGauravGupta can you handle? If not in timescale, can @mbien or @matthiasblaesing maybe handle that step? Thanks!

matthiasblaesing commented 1 year ago

I created #5301, which holds a squashed version. This is not a complete squash, but I think it is reasonable. @jGauravGupta please have a look at that variant, as you are the author. If you agree it would be great if you either update this PR with the contents of my branch (this will need a force push) or you say it is ok and I'll push that branch myself.

matthiasblaesing commented 1 year ago

@mbien I assume you added the "do not merge" label to mark this PR as "don't merge in the current state". I'll try to push the new tree currently running tests via #5301 through this so that this discussion stays connected.

mbien commented 1 year ago

@matthiasblaesing yes exactly. Only wanted to make sure this isn't merged by accident since its reviewed.

matthiasblaesing commented 1 year ago

5301 is green and the additional commits from @pepness look sane to me. I do a final build on my machine and will then try to force push into @jGauravGupta branch. This should trigger tests again here, when this works and they come back green I'll merge this. If that fails, I intent to merge #5301 and close this.

mbien commented 1 year ago

i can't select a Java EE profile on new maven "Web Application" projects when Wildfly 27 is selected (it works for GF and others). But this is probably something we can fix during RC, this shouldn't hold this PR off.

quick debugging showed that org.netbeans.modules.javaee.wildfly.ide.WildflyJ2eePlatformFactory.J2eePlatformImplImpl.getSupportedProfiles returns a set of FULL profiles which are later removed in the UI (ServerSelectionHelper.updatePlatformVersionModel) since it is a web project which causes the combo box to remain empty.

matthiasblaesing commented 1 year ago

The test failure in NetBeans / VSCode Extension Tests on JDK 8 (pull_request) was caused by a broken catalog.xml.gz file. I regenerated that right now, but won't rerun the VScode test.

matthiasblaesing commented 1 year ago

So tests are green. Lets get this finally in.