Skip to content

fix enterprise cluster build on JDK 17 + add upstream-JDK build job. - #3764

Merged
mbien merged 1 commit into
apache:masterfrom
mbien:build-on-jdk17
Mar 13, 2022
Merged

fix enterprise cluster build on JDK 17 + add upstream-JDK build job.#3764
mbien merged 1 commit into
apache:masterfrom
mbien:build-on-jdk17

Conversation

@mbien

@mbien mbien commented Mar 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

it turns out that the enterprise cluster was the last cluster with build problems on JDK 17. Java cluster was fixed back in #3278 and gradle #3326.

@matthiasblaesing must have fixed the javascript cluster without telling anyone :)

everything builds locally, lets see if CI agrees.

this fixes the following build error:

 [repeat] Note: generating beans in org.netbeans.modules.j2ee.deployment.impl.gen.nbd
   [repeat] Note: parsing: file:///mnt/ram/netbeans/enterprise/j2eeserver/src/org/netbeans/modules/j2ee/deployment/plugins/api/netbeans-deployment.dtd
   [repeat] /mnt/ram/netbeans/enterprise/j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/gen/nbd/package-info.java:28: error: Failed to process
   [repeat] package org.netbeans.modules.j2ee.deployment.impl.gen.nbd;
   [repeat] ^
   [repeat] error: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @392c130a
   [repeat]   	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
   [repeat]   	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
   [repeat]   	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:178)
   [repeat]   	at java.base/java.lang.reflect.Field.setAccessible(Field.java:172)
   [repeat]   	at org.netbeans.modules.schema2beansdev.gen.JavaUtil.getOptimialHashMapSize(JavaUtil.java:855)
   [repeat]   	at org.netbeans.modules.schema2beansdev.gen.JavaUtil.getOptimialHashMapSize(JavaUtil.java:839)
   [repeat]   	at org.netbeans.modules.schema2beansdev.JavaBeanClass.genExtendBaseBean(JavaBeanClass.java:4421)
   [repeat]   	at org.netbeans.modules.schema2beansdev.JavaBeanClass.genAllParts(JavaBeanClass.java:119)
   [repeat]   	at org.netbeans.modules.schema2beansdev.JavaBeanClass.generate(JavaBeanClass.java:54)
   [repeat]   	at org.netbeans.modules.schema2beansdev.BeanBuilder.doGeneration(BeanBuilder.java:906)
   [repeat]   	at org.netbeans.modules.schema2beansdev.BeanBuilder.process(BeanBuilder.java:556)
   [repeat]   	at org.netbeans.modules.schema2beansdev.GenBeans.doIt(GenBeans.java:841)
   [repeat]   	at org.netbeans.modules.schema2beansdev.Schema2BeansProcessor.handle(Schema2BeansProcessor.java:163)
   [repeat]   	at org.netbeans.modules.schema2beansdev.Schema2BeansProcessor.process(Schema2BeansProcessor.java:68)
   [repeat]   	at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:1023)
...

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) CI continuous integration changes labels Mar 11, 2022
@mbien mbien added this to the NB14 milestone Mar 11, 2022
JaroslavTulach
JaroslavTulach reviewed Mar 11, 2022
View reviewed changes
Comment thread ide/schema2beans/src/org/netbeans/modules/schema2beansdev/gen/JavaUtil.java
@@ -852,7 +852,7 @@ public static int getOptimialHashMapSize(Object[] keys, int maxSize) {
int defaultAnswer = keyLength + 1;
try {
java.lang.reflect.Field tableField = HashMap.class.getDeclaredField("table");

@JaroslavTulach JaroslavTulach Mar 11, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about removing the reflection all together and just assuming that keys.length * 3 / 2 is usually good enough?

@mbien mbien Mar 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i actually wanted to deprecate the method - i forgot, Lets see if it builds, if it does I push again.

@mbien mbien Mar 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deprecated it + added keylength * 3 / 2 as default return value. Its what the JDK uses anyway if i remember correctly.

JaroslavTulach
JaroslavTulach reviewed Mar 11, 2022
View reviewed changes
Comment thread ide/schema2beans/src/org/netbeans/modules/schema2beansdev/gen/JavaUtil.java
return defaultAnswer;
} catch (RuntimeException ex) {
// todo: remove this workaround post JDK 9+ migration
// modules on modern JDKs should prefere immutable collections anyway, e.g Map.of();

@JaroslavTulach JaroslavTulach Mar 11, 2022 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a way to use Map.of. Described in my Run on JDK8, use JDK11 APIs post - if you believe this case deserves that design pattern (I don't), feel free to do implement it.

@mbien mbien Mar 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please no - things are complex enough already lets try to remove workarounds instead of adding more :)

@mbien
mbien marked this pull request as ready for review March 11, 2022 15:50
@mbien
mbien requested a review from neilcsmith-net March 11, 2022 15:50
@mbien

mbien commented Mar 11, 2022

Copy link
Copy Markdown
Member Author

it works, full-cluster github job is green

@mbien
mbien requested a review from matthiasblaesing March 11, 2022 15:52
matthiasblaesing
matthiasblaesing approved these changes Mar 12, 2022
View reviewed changes

@matthiasblaesing matthiasblaesing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sane to me, though I would make more sense just to return defaultAnswer straight away. That way we would get consistent results from this and not rely on runtime behavior. I have no strong feelings either way, just a thought.

@mbien

mbien commented Mar 13, 2022

Copy link
Copy Markdown
Member Author

Looks sane to me, though I would make more sense just to return defaultAnswer straight away. That way we would get consistent results from this and not rely on runtime behavior. I have no strong feelings either way, just a thought.

the only usage of this method is a code generator here:

netbeans/ide/schema2beans/src/org/netbeans/modules/schema2beansdev/JavaBeanClass.java

Line 4421 in 4ae01ea

jw.writeEol("private java.util.Map propByName = new java.util.HashMap("+(JavaUtil.getOptimialHashMapSize(propByNameKeys)), ", 1.0f)");

this is called at buildtime. It might be also used at runtime somewhere - I haven't investigated further, my main intention was to make NB buildable on JDK 17 without changing the behavior too much. This can be still improved later if needed.

@mbien
mbien merged commit 50fb68f into apache:master Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthiasblaesing matthiasblaesing matthiasblaesing approved these changes
@neilcsmith-net neilcsmith-net Awaiting requested review from neilcsmith-net
+1 more reviewer
@JaroslavTulach JaroslavTulach JaroslavTulach left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI continuous integration changes Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Milestone

NB14

Development

Successfully merging this pull request may close these issues.

3 participants

@mbien @matthiasblaesing @JaroslavTulach