Towards building on JDK11 and running on JDK8 #2761

jtulach · 2021-02-14T07:21:36Z

As observed by Jan: lookup module requires 1.8 - e.g. every module that depends on it, requires JDK8 as well.

let's update the javac.target to 1.8 - 7dbc4f9
some scripts define just javac.source - let's set the default target to 1.8: 4661771
Modify our javac task to use setRelease(${javac.target}) to avoid using new than 1.8 APIs
Update to Ubuntu 18.4 LST to get version of Ant that understands setRelease
Such update switches from Oracle JDK8 to OpenJDK8 - e.g. there is no JavaFX anymore and tests had to be updated
Drop permit.jdk9.builds flag, allow building on any JDK8+
Enhance ant commit-validation to verify all generated classfiles have JDK8's major/minor version

Things seem to be working. Approvals welcomed.

…ified

lkishalmi · 2021-02-16T22:33:35Z

platform/libs.javafx/manifest.mf

@@ -38,5 +38,4 @@ OpenIDE-Module-Provides: javafx.animation,
    javafx.util,
    javafx.util.converter,
    netscape.javascript
-Class-Path: ${java.home}/lib/ext/jfxrt.jar
-
+Class-Path: %24%7Bjava.home%7D%2F/lib/ext/jfxrt.jar


Well, this is ugly. Cant we just have JAVA_HOME/lib/ext/jfxrt.jar ?

Very ugly, I agree. I was also considering defining some new token instead of ${java.home}, but that would mean API changes, versioning, etc. At the end I decided to apply a simple bugfix.

lkishalmi · 2021-02-16T22:34:47Z

platform/o.n.bootstrap/src/org/netbeans/StandardModuleData.java

+                    ext = URLDecoder.decode(tok.nextToken(), "UTF-8");
+                } catch (UnsupportedEncodingException ex) {
+                    throw new IllegalStateException(ex);
+                }
                File extfile;
                if (ext.equals("${java.home}/lib/ext/jfxrt.jar")) { // NOI18N


It seems we do not really require that $ sign.
Also there are some JDK 1.7 checks which could be removed as well.

Yes, the ${java.home} string is totally artificial. I have chosen it as it had no meaning for javac (in JDK8, who could expect JDK11 starts to recognize URLs in Class-Path attribute!).

There already was some URI-like processing (replacing %20 by ) and using URLDecoder (not available on JDK7 at the time of writing this code) can be seen as a "proper fix" of processing the URI component.

Right, the JDK7 specific code can be removed.

FWIW, you can probably use java.nio.charset.StandardCharsets.UTF_8 instead of "UTF-8", and avoid the need to handle the UnsupportedEncodingException.

Alas, I cannot. That method has been added in JDK10! While it seems available in the editor, when compiling with the new --release 8 the compilation fails. I tried and had to revert back to specifying the encoding as string and catching the exception.

neilcsmith-net · 2021-02-17T15:26:43Z

+1 to the changes so far, -1 to the title. Not sure we're ready for dropping building on JDK 8? And so far this does build fine with it.

matthiasblaesing · 2021-02-17T17:59:27Z

+1 to the changes so far, -1 to the title. Not sure we're ready for dropping building on JDK 8? And so far this does build fine with it.

I understood, that the aim is to build with the "release" option from javac. That was added with java 9 and is not present on 8. It would be possible that ant emulates release 8 on JDK 8 by just using the classpath, but it would IMHO not be correct.

Bumping target version is not that interesting to me.

neilcsmith-net · 2021-02-17T18:06:10Z

@matthiasblaesing yes, Ant basically ignores it (unlike Maven which requires fun with profiles). So it should be pretty safe to allow both, seen as 8 is the only option without "release" that will work? Dropping support entirely for compiling on JDK 8 seems a bigger step to take, particularly given we produce source releases.

jtulach · 2021-02-21T06:50:35Z

To use the Ant's <javac release="8"/> tag I had to upgrade travis to use Ubuntu 18.04 which provides newer Ant. That triggered a lot of new failures as the "bionic" docker image is no longer using OracleJDK, but OpenJDK. OpenJDK doesn't have JavaFX installed. A lot of tests were failing due to that. Now everything seems to be fine.

1067e68 adds new commit validation test - it checks if all the generated .class files are executable on JDK8. I am going to fix it now.

matthiasblaesing · 2021-02-21T15:19:35Z

This looks interesting and an improvement. I have the following remarks/questions:

There are adjustments to javac.target but not javac.source. This I don't understand - is the intention here to allow javac to generate more modern code, that uses features only present on Java 8+, but not suffer from code incompatibilites by using old source parser/behavior?
Why is javac.default.target overriden to 1.8, instead of relying on the default behavior in nbbuild/templates/common.xml - there the default target is initialized to javac.source, which sounds sane to me?
I noticed some changes to build.xml files, which add release="8" target="1.8" - on JDK 9+ target is not necessary, as release substitutes it. I would expect target to default to 8 on JDK 8, so this feels like a duplicate.

The bump of the baseline linux distribution for unittests looks sane to me. Thank you for working on this.

jtulach · 2021-02-21T17:30:33Z

This looks interesting and an improvement.

It is a small step forward (e.g. we still support compilation with JDK8 as @neilcsmith-net requested) but I believe it is a step in the right direction. The most visible improvement is the dropping of permit.jdk9.builds flag. Now we can compile with JDK8+ and always get the same result (except maybe few modules where we continue to use bootclasspath and the --release 8 API isn't enforced).

I noticed some changes to build.xml files, which add release="8" target="1.8" - on JDK 9+ target is not necessary, as release substitutes it. I would expect target to default to 8 on JDK 8, so this feels like a duplicate.

The release flag is ignored on JDK8, so I am specifying both release and target. It feels safer to be explicit than to rely on some defaults. Anyway the outcome is important - and that's what is verified by the commit validation ValidateClassFilesTest.

Why is javac.default.target overriden to 1.8, instead of relying on the default behavior in nbbuild/templates/common.xml - there the default target is initialized to javac.source, which sounds sane to me?

@lkishalmi decided we want our modules to use JDK8 whenever there is a clash between the javac.target and the used API. One option to achieve that is to set source and target explicitly to 1.8 in all such modules. The other option was to make sure the default for NetBeans is 1.8. Laszlo wrote "let the past go", so I decided to go with the second option.

There are adjustments to javac.target but not javac.source. This I don't understand - is the intention here to allow javac to generate more modern code, that uses features only present on Java 8+, but not suffer from code incompatibilites by using old source parser/behavior?

The value of javac.target influences not just generated bytecode, but also the API one compiles against (via the --release flag). I bumped javac.target=1.8 with a sed-like utility. But that wasn't enough, so I also updated the default. The javac.source hasn't been changed as @jlahoda warned that it could change the semantics.

matthiasblaesing

I left a few inline comments. In general this looks good. I don't see the benefit of bumping the javac.target without bumping javac.source as this will still fail hard on newer JDKs. For me this add noise without real benefit.

...orm/o.n.core/test/qa-functional/src/org/netbeans/core/validation/ValidateClassFilesTest.java

enterprise/web.monitor/build.xml

java/api.debugger.jpda/nbproject/project.properties

profiler/profiler/nbproject/project.properties

lkishalmi

Well, looks fine enough. I'd vote to have this in, if there would be some shortcomings detected we could fix that later. As of setting all javac.source to 1.8, AFAIK there could be some differences in spec between the spec, like the Override annotation in Java 5 and Java 6. So I'd leave that for another day.
BTW If you read that @lkishalmi decided in the comments, I'd mark that I do not decide, I merely suggest.

jtulach · 2021-02-23T07:00:32Z

BTW If you read that @lkishalmi decided in the comments, I'd mark that I do not decide, I merely suggest.

Changing directions when the code is ready is costly. As such I discussed the approach first, listened to the suggestions and implemented what seemed to be a mutually agreeable intersection.

As soon as I started coding the suggestions (hopefully correctly selected by me) have become decisions. I have attributed the decisions to you guys to stress, that I did listen to your suggestions before trying to move the project to the right direction without pushing it inappropriately too far.

ebresie · 2021-02-23T13:46:51Z

Once this rolls out, recommend update the readme.md to account for changes (i.e. sounds like the jdk9 ant parameter not needed, minimum ant version, if any new parameters are needed, etc.)

neilcsmith-net · 2021-02-23T14:48:41Z

Once this rolls out, recommend update the readme.md to account for changes

Actually, that's a good point - that doc change might be better included in this PR?

ebresie · 2021-02-23T18:56:15Z

As long as considering readme items...readme says needs "Ant 1.9.9 or above" but commit 140cb7e reference to ant 1.9.8. Does the 1.9.8 need to be changed to 1.9.9 to be in line with the doc?

matthiasblaesing · 2021-02-23T20:34:55Z

@ebresie I had a look at the history of the README and the ant version is there since the beginning. I also remember, that there were broken ant installations in various linux distributions, I would keep it as is. People reading the README have no problem, all people with recent ant version have no problem, only people on ant version which should never have been used to build netbeans will see the message.

Lets get this in.

jtulach · 2021-02-24T07:04:16Z

The 0c11e9e fix is needed to accommodate new tests added by #2731

lkishalmi · 2021-02-24T17:21:53Z

Geat job! Thank you!

matthiasblaesing · 2021-02-27T16:02:10Z

@jtulach @jlahoda I'm not sure, but I think I seen an issue with the module path in netbeans with this change (I don't think the change is broken, just another adjustment, that is needed).

I can build NetBeans with JDK 8 and JDK 11 and system ant just fine. So it is not a general problem, but when opening netbeans modules in the IDE, I see issues.

I run a build from current master and open the project openide.util.lookup. When the IDE is loaded on JDK 8, the modules can be rebuild using "Clean and Build" from the context menu.

Compiling 31 source files to /home/matthias/src/netbeans/platform/openide.util.lookup/build/classes
Ignoring source, target and bootclasspath as release has been set
error: release version 8 not supported
Usage: javac <options> <source files>
use --help for a list of possible options

I tracked this down into the java compiler source in (I used git sources from jdk-11+28):

src/jdk.compiler/share/classes/com/sun/tools/javac/platform/PlatformUtils.java

If I understand it correctly, the list of PlatformProviders is empty, while it should at least provide com.sun.tools.javac.platform.JDKPlatformProvider.

I have some hope that one of you two has an idea how to fix this.

Rareitor · 2021-03-19T11:43:51Z

On a related note, should the instructions for building from source on the website (https://netbeans.apache.org/download/dev/index.html) be updated to note that the -Dpermit.jdk9.builds=true flag is not needed anymore?
I did the last modification, but missed the chance to do it again.
It would be an update for the next release anyway, since it still needs the flag to build.

Jaroslav Tulach added 5 commits February 14, 2021 08:16

Let's upgrade javac.target to 1.8 across the code base

7dbc4f9

By default use javac.target 1.8

4661771

Set release based on javac.target unless -Xbootclasspath flag is spec…

96720b9

…ified

Disable release flag by setting bootclasspath

94aebf0

setRelease has been added in Ant 1.9.8

140cb7e

jtulach force-pushed the jtulach/UseJDK11 branch from 0621a08 to 140cb7e Compare February 14, 2021 12:28

Jaroslav Tulach added 2 commits February 15, 2021 06:24

Avoid access to sun.security.util package in the test

741fd03

Removing the old bytecode test

bc005a0

lkishalmi reviewed Feb 16, 2021

View reviewed changes

jtulach changed the title ~~Towards building just on JDK11.~~ Towards building on JDK11 and running on JDK8 Feb 17, 2021

JDK11's javac doesn't like $ in Class-Path attribute

1b96b56

jtulach force-pushed the jtulach/UseJDK11 branch from 3f6a54f to 1b96b56 Compare February 17, 2021 18:53

Jaroslav Tulach added 6 commits February 20, 2021 06:50

Removing the -Dpermit.jdk9.builds=true flag

240c5b3

Skip the api.htmlui tests if JFXPanel cannot be instantiated

6bfbf96

Skip the tests if JavaFX cannot be initialized

7760c5d

Don't test JavaFX Ant project when there is no JavaFX

b48dd4d

Only perform the test when JFXPanel is available

d9b2df1

Validate the generated class files can be executed on JDK8

1067e68

Always specify target/release when using javac

454ce06

matthiasblaesing reviewed Feb 21, 2021

View reviewed changes

lkishalmi added the Need Squashing label Feb 22, 2021

lkishalmi approved these changes Feb 22, 2021

View reviewed changes

lkishalmi removed the Need Squashing label Feb 23, 2021

Removing note about -Dpermit.jdk9.builds=true from readme

1fc8f5a

Making up-to-date with master

4e20b24

Skip the wizard test when there is no JavaFX around

0c11e9e

jtulach force-pushed the jtulach/UseJDK11 branch from 3695501 to 0c11e9e Compare February 24, 2021 07:23

JaroslavTulach merged commit 8ec89eb into apache:master Feb 24, 2021

jlahoda mentioned this pull request Feb 27, 2021

Register JDKPlatformProvider when creating javac in ant's ClassLoader. #2784

Merged

JaroslavTulach mentioned this pull request Jan 6, 2022

[NETBEANS-6285] Shield against usage of JDK-11 API. #3418

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Towards building on JDK11 and running on JDK8 #2761

Towards building on JDK11 and running on JDK8 #2761

jtulach commented Feb 14, 2021 •

edited

lkishalmi Feb 16, 2021

jtulach Feb 17, 2021

lkishalmi Feb 16, 2021

jtulach Feb 17, 2021

jlahoda Feb 17, 2021

jtulach Feb 17, 2021

neilcsmith-net commented Feb 17, 2021

matthiasblaesing commented Feb 17, 2021

neilcsmith-net commented Feb 17, 2021

jtulach commented Feb 21, 2021 •

edited

matthiasblaesing commented Feb 21, 2021

jtulach commented Feb 21, 2021

matthiasblaesing left a comment

lkishalmi left a comment

jtulach commented Feb 23, 2021

ebresie commented Feb 23, 2021

neilcsmith-net commented Feb 23, 2021

ebresie commented Feb 23, 2021

matthiasblaesing commented Feb 23, 2021

jtulach commented Feb 24, 2021 •

edited by JaroslavTulach

lkishalmi commented Feb 24, 2021

matthiasblaesing commented Feb 27, 2021

Rareitor commented Mar 19, 2021

Towards building on JDK11 and running on JDK8 #2761

Towards building on JDK11 and running on JDK8 #2761

Conversation

jtulach commented Feb 14, 2021 • edited

lkishalmi Feb 16, 2021

Choose a reason for hiding this comment

jtulach Feb 17, 2021

Choose a reason for hiding this comment

lkishalmi Feb 16, 2021

Choose a reason for hiding this comment

jtulach Feb 17, 2021

Choose a reason for hiding this comment

jlahoda Feb 17, 2021

Choose a reason for hiding this comment

jtulach Feb 17, 2021

Choose a reason for hiding this comment

neilcsmith-net commented Feb 17, 2021

matthiasblaesing commented Feb 17, 2021

neilcsmith-net commented Feb 17, 2021

jtulach commented Feb 21, 2021 • edited

matthiasblaesing commented Feb 21, 2021

jtulach commented Feb 21, 2021

matthiasblaesing left a comment

Choose a reason for hiding this comment

lkishalmi left a comment

Choose a reason for hiding this comment

jtulach commented Feb 23, 2021

ebresie commented Feb 23, 2021

neilcsmith-net commented Feb 23, 2021

ebresie commented Feb 23, 2021

matthiasblaesing commented Feb 23, 2021

jtulach commented Feb 24, 2021 • edited by JaroslavTulach

lkishalmi commented Feb 24, 2021

matthiasblaesing commented Feb 27, 2021

Rareitor commented Mar 19, 2021

jtulach commented Feb 14, 2021 •

edited

jtulach commented Feb 21, 2021 •

edited

jtulach commented Feb 24, 2021 •

edited by JaroslavTulach