Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build with JDK-11 in all Travis jobs #3744

Merged

Conversation

JaroslavTulach
Copy link

@JaroslavTulach JaroslavTulach commented Mar 10, 2022

NetBeans are supposed to be developed with JDK-11 yet nobody updated Travis jobs to do so yet. Adding nbbuild/travis/ant.sh script which downloads JDK-11. Using it for compilation, but not for testing. Testing continues to be performed on the JDK the job has been setup for.

By merging this PR we shall be ready to start using alternative modular implementations. E.g. introduce org.netbeans.modules.xyz.jdk11 with javac.target=11 providing better integration with JDK-11 where needed.

@JaroslavTulach JaroslavTulach self-assigned this Mar 10, 2022
@neilcsmith-net
Copy link
Member

Can we not consolidate on one method of downloading a JDK in this? install-jdk.sh should also be in the path and not need downloading itself, although that also may not be the best option. A JDK 11 should also be installed already too.

@JaroslavTulach
Copy link
Author

Can we not consolidate on one method of downloading a JDK in this?

If one trusts that the URL remains persistent, downloading from a URL seems acceptable to me. I have decided to trust this JDK provider in my projects. Moreover this primitive technique is portable across CI types - it shall work everywhere curl is installed.

install-jdk.sh should also be in the path and not need downloading itself, although that also may not be the best option.

I've never used it myself. Neither inside Travis, neither locally (e.g. I cannot test the behavior). The PR is however open for edits from maintainers, if users of install-jdk.sh want to step in.

A JDK 11 should also be installed already too.

I wanted to avoid modifying the behavior of the Travis scripts. For example by accidentally switching to JDK11 for testing. Downloading a JDK into temporary directory and using it only in ant.sh guarantees no other part of the scripts is going to be influenced.

@neilcsmith-net
Copy link
Member

neilcsmith-net commented Mar 11, 2022

It wasn't an argument for or against install-jdk.sh. It was an argument for picking one way to do this. We're already using it - eg. https://github.com/apache/netbeans/blob/master/.travis.yml#L445

I wanted to avoid modifying the behavior of the Travis scripts. For example by accidentally switching to JDK11 for testing.

Understand argument for known JDK. But don't see why there's more chance of accidentally switching from using JAVA_HOME with the already installed JDKs vs one downloaded. Either way, had wondered merit of running ant -diagnostics on each switch of JDK for sanity checking?

@mbien
Copy link
Member

mbien commented Mar 11, 2022

I added a few echos to the build to make it easier to quickly check which JDK is in path. It prints it very early so that it is easily to find.
https://github.com/apache/netbeans/pull/3754/files#diff-ef48f302a1dc02030dd4f72d068dd2c51d8c4e27fab06014cd0f7a1ae7cd2fa9R41-R43

@JaroslavTulach
Copy link
Author

I've noticed earlier that NetBeans 13 has broken profiler when running on JDK8. The 933ab50 commit shall fix that. Sorry for not catching it earlier.

@mbien
Copy link
Member

mbien commented Mar 11, 2022

wondering if we can remove the "Compile with JDK-11 and test something on JDK-8 " gh job if this PR makes it in.

I also have to agree with Neil, we should not use two different scripts for downloading JDKs if there is a reasonable way to avoid it. The already existing install-jdk.sh should be able to install JDKs side by side I believe, or is it not?

@mbien mbien added the CI Continuous Integration label Mar 11, 2022
@JaroslavTulach
Copy link
Author

Looks like we are green. I squash the changes, accommodate #3778 and then we can review and merge.

nbbuild/travis/ant.sh Show resolved Hide resolved
nbbuild/travis/ant.sh Show resolved Hide resolved
@JaroslavTulach
Copy link
Author

We are green. Given how hard it is to get green status from all our CI builders, I'd rather merge than do some other changes (like remove the "Compile with JDK-11 and test something on JDK-8 " job) which I've forgotten - they can be done any time later.

By merging this PR we shall be ready to start using alternative modular implementations. E.g. introduce org.netbeans.modules.xyz.jdk11 with javac.target=11 providing better integration with JDK-11 where needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants