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

java.source.base: Fix for Java 11, update args, swap import #572

Closed
wants to merge 1 commit into from
Closed

java.source.base: Fix for Java 11, update args, swap import #572

wants to merge 1 commit into from

Conversation

wltjr
Copy link
Contributor

@wltjr wltjr commented Jun 2, 2018

Fixes the following issues

src/org/netbeans/api/java/source/SourceUtils.java:427:
error: constructor NamedImportScope in class NamedImportScope cannot be applied to given types;
                NamedImportScope importScope = new NamedImportScope(unit.packge, unit.toplevelScope);
                                               ^
  required: Symbol
  found: PackageSymbol,WriteableScope
  reason: actual and formal argument lists differ in length

src/org/netbeans/modules/java/source/NoJavacHelper.java:69:
error: cannot find symbol
               unsafe.defineClass("com.sun.tools.javac.code.Scope$WriteableScope", classData, 0, classData.length, scopeClass.getClassLoader(), scopeClass.getProtectionDomain());
                          ^
  symbol:   method defineClass(String,byte[],int,int,ClassLoader,ProtectionDomain)
  location: variable unsafe of type Unsafe

Fixes the following issues

src/org/netbeans/api/java/source/SourceUtils.java:427:
error: constructor NamedImportScope in class NamedImportScope cannot be
applied to given types;
                NamedImportScope importScope = new
NamedImportScope(unit.packge, unit.toplevelScope);
                                               ^
  required: Symbol
  found: PackageSymbol,WriteableScope
  reason: actual and formal argument lists differ in length

src/org/netbeans/modules/java/source/NoJavacHelper.java:69:
error: cannot find symbol

unsafe.defineClass("com.sun.tools.javac.code.Scope$WriteableScope",
classData, 0, classData.length, scopeClass.getClassLoader(),
scopeClass.getProtectionDomain());
                          ^
  symbol:   method
defineClass(String,byte[],int,int,ClassLoader,ProtectionDomain)
  location: variable unsafe of type Unsafe
@wltjr
Copy link
Contributor Author

wltjr commented Jun 2, 2018

These changes will not work under JDK 10, they are 11+.

@matthiasblaesing
Copy link
Contributor

We have not yet dropped support for JDK8, so from my POV this is not mergeable if it is indeed 11 only.

@wltjr
Copy link
Contributor Author

wltjr commented Jun 2, 2018

Sure no worries. If it is not merged that is fine. If anything its a heads up for things that will beak under 11. Backward compliance is always difficult. It one of my biggest gripes with Java. But that is up to each project. At least JDK8 is still some what current. Though I am 11 in dev, and 10 in production. I suspect Oracle is wanting to shed some of the legacy stuff as well with the increased releases. I think it will get really interesting for backward compliance. Good luck there!

@matthiasblaesing
Copy link
Contributor

You could try to find a solution, that works for both worlds 8 + 11.

@wltjr
Copy link
Contributor Author

wltjr commented Jun 2, 2018

Well Unsafe is moving locations so not sure how to address that for both. The other I guess you could add a wrapper to NamedImportScope or something to add back the constructor with 2 arguments and discard the 2nd.

I do not seen any clean solution to either offhand. I will think on it more and see if I can come up with a more elegant solution. But I would not hold my breath. Legacy compatibility is the last thing on my list.

It is likely something a build system will have to switch. Like a java11 folder or something with the modified files, and for the rest can use existing. Short of finding a solution that works for both.

I am not modifying the build aspect with any of my PRs. For many they will need a new import of javax-annotations. A bunch now need that which did not before as it was in the JDK.

@matthiasblaesing
Copy link
Contributor

The problem is deeper: The classes are also missing at runtime, so just building against java 11 will not help. You can try to use reflection to work around it for now, but Unsafe#defineClass will also go away.

@wltjr
Copy link
Contributor Author

wltjr commented Jun 2, 2018

That is good to know. I have not run into any runtime issues yet. But still updating other packages. I have switched other projects over to the new location of Unsafe under JDK11. Even if I can make a work around. I cannot test easily. My env no longer supports JDK8. I am 10+ only.

@jlahoda
Copy link
Contributor

jlahoda commented Jun 3, 2018

Please note the Unsafe part is only important when running on JDK 8. My plan was to use the reflection if we got to a place where we would use JDK 9+ to compile with "--release 8".

The NamedImportScope may need some reflection handling, unless we drop both 9 and 10 :-(. Ideally, we would rewrite the method to not use the internal class, but not sure if we can do it in near future.

@wltjr
Copy link
Contributor Author

wltjr commented Jun 3, 2018

@jlahoda I am not sure you can use Unsafe with --release 8. It requires either for 9 and 10 --add-exports jdk.unsupported/sun.misc=ALL-UNNAMED or for 11 --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED. But the problem is you cannot use -add-modules or --add-exports when using --release. There maybe some other way to use that stuff. But I believe even under older releases they do not expose that stuff. I have not really been able to use release with anything that requires modules or exports.

Let this serve as more of a heads up than direct fixes. Surely for NamedImportScope. Though not sure there is much point in supporting 9. I think its more just 8 and 10, then 11. Of course I would say go 11+ only for like NB 10. But that is likely not practical.

Though IMHO why build the latest Netbeans using an older JDK. Or even allow it to run on older JDK. Unless you mean development support. Even then IMHO they can use older version of Netbeans or other for older JDK. It is always fascinating to me for those to use newer libraries or applications and older JDK. I myself have never had to stick with an older JDK. Even like in Gentoo people still ran older versions of Tomcat. Which I never understoood either. My stuff has always been able to move to newer without issue. Even some legacy stuff from 1.4 days I keep bugging the client to update :)

@wltjr
Copy link
Contributor Author

wltjr commented Jun 4, 2018

Looks like for defineClass there is another solution that is 9+. Though I think the jdk internal Unsafe is there for 9+ as well. Still this is proper for 11+. I can update the PR for that part. Though likely will not work for 8.

@wltjr
Copy link
Contributor Author

wltjr commented Jun 7, 2018

That other solution for defineClass is not a drop in replacement. The arguments differ. Switching the imports is a drop in replacement. I need to check for 9, but exists in 10, jdk.internal.misc.Unsafe

@matthiasblaesing
Copy link
Contributor

A way that could be explored is using MR-JARs to be able to expose the same interface, but use different implementations. JDK 8 would be the baseline implementation, implementations for 9,10,11 could be pushed as multi-release enhancements (the support was added with 9).

@matthiasblaesing
Copy link
Contributor

Closing this PR as it is incompatible with Java 8 and it was indicated, that there is no intention to fix that. (see comment #572 (comment))

@wltjr
Copy link
Contributor Author

wltjr commented Aug 18, 2018

Oracle plans to end public support of Java 1.8 in January. I would not hold back newer JDK's due to older. 11 comes out next month. IMHO its more important to support newer than older. Is Netbeans forward looking or more concerned with legacy compatibility? It is clear Oracle wants to move Java forward faster. I would hope Netbeans is not a IDE holding onto the past.

FYI I also have some 70+ Eclipse packages, maybe close to 100. No where near the amount for Netbeans. At the same time. I have no Java 11 fixes for Eclipse, like I have for Netbeans. Just some food for thought. Its all moot to me, as my packages are pretty different from upstream. But I do not believe I can run NB on JDK 11 presently. Definitely cannot builder under JDK 11. I am running RC1 on 10, it fails on 11. My from source version is still a work in progress. Have some odd issues there I need to resolve.

IMHO if people need to support older JDK's they can use older IDE's, like NB 8.x. Older stuff should not hold back newer. Not when newer is about to be the current release version. Also 11 is a long term support, to replace 8. http://www.oracle.com/technetwork/java/eol-135779.html

@geertjanw
Copy link
Member

Just stop using RC1 please. Apache NetBeans (incubating) 9.0 has been released, end of last month.

@wltjr
Copy link
Contributor Author

wltjr commented Aug 18, 2018

I was wondering about that. I guess the VC3 tag is a release tag. The tagging aspect is still very strange compared to upstream. It would be more clear if there was an official 9.0 tag without any suffix. Or a release, rel, etc. I haven't seen any other use that format, not even other Apache projects.

The version is moot to me as I am mostly doing C development at the moment. I need to work on my package more. I need to make a base package, that just builds enough to run plugin manager. Then can install the rest. As an option to building it all from source. Which I have some open issues.

@wltjr
Copy link
Contributor Author

wltjr commented Aug 18, 2018

I can't use the release version. I had to go back to RC1. There are no available plugins for the release version, no C/C++ stuff, Darcula, etc. I have some of that packaged. I will have to update my stuff to the VC3 tag building from source to have some of the stuff I need. No clue about the C/C++ stuff, cmake, etc.

@matthiasblaesing
Copy link
Contributor

matthiasblaesing commented Aug 18, 2018 via email

@wltjr
Copy link
Contributor Author

wltjr commented Aug 18, 2018

Ok I was just responding to your saying use release over RC1 here. Also I keep getting removed from the mailing list. Not sure whats going on. I have not had that problem on other apache lists, jclouds, maven and tomcat. Occasionally I get bounce warnings. But never removed. This will be the 3rd time I join the apache nb dev mailing list. Having been removed twice without notification. Some sort of bounce related removal. Not anything a human did. Thus nothing from me on that mailing list in a while.

@jlahoda
Copy link
Contributor

jlahoda commented Aug 19, 2018

I'd like to point out that there is a huge difference between the supported/working build requirements and supported/working runtime requirements.

Currently, the build needs JDK 8, and the minimal runtime is JDK 8 as well. But the IDE can (at least somehow) run on (dev builds of) JDK 11/12 - I use these runtimes. (Although it will be necessary to do some changes to keep the IDE running on 12 in the future, I am afraid.)

We could change the build process to run on JDK 10/11 (and keep the minimal runtime env as JDK 8, or not), but someone needs to do the work.

Dropping JDK 8 as the minimal runtime environment is not a decision that could be done in a PR - that needs to be discussed on the mailing list, and I suspect there may be folks that still want to run NB on JDK 8, for various reasons.

Regarding Unsafe and --release - as I noted, when I was thinking of this, the idea was to use reflection. I am fairly sure one can use --release 8 and compile code that invokes Unsafe using reflection on JDK 8. Please see classes like java.lang.reflect.Method.

@geertjanw
Copy link
Member

I can't use the release version. I had to go back to RC1. There are no available plugins for the release version, no C/C++ stuff, Darcula, etc. I have some of that packaged. I will have to update my stuff to the VC3 tag building from source to have some of the stuff I need. No clue about the C/C++ stuff, cmake, etc.

Yes you can: https://blogs.apache.org/netbeans/entry/what-s-happened-to-my

@neilcsmith-net
Copy link
Member

Also 11 is a long term support, to replace 8.

Without wanting to encourage the chitchat monster on here too much, be aware that there is no free Oracle Java LTS. And various OpenJDK vendors, such as AdoptOpenJDK, are committed to OpenJDK 8 support as long as 11 (eg. Sept 2022 - https://adoptopenjdk.net/support.html ) So, yes, +1 to no rush to drop Java 8 support, and discussing it on the mailing list when the time is right.

@wltjr
Copy link
Contributor Author

wltjr commented Aug 19, 2018

Java 8 has become python 2.7... I am talking about building Netbeans under Java 8. I fail to see why people need to build it under 8. Or the benefit to 9 over 8.x for running on Java 8 or developing Java 8 based applications.

@geertjanw
Copy link
Member

You are welcome to provide a pull request enabling NetBeans to be built under Java 9, 10, and 11. That’s “all” that’s needed, someone to put in the work. Whenever you think to yourself “I fail to see why people need to do XYZ” in the context of NetBeans, you should instead think “I fail to see why I don’t invest some time so that people won’t need to do XYZ.”

@wltjr
Copy link
Contributor Author

wltjr commented Aug 19, 2018

Is that not exactly what this PR is? Fixes for JDK 11.

@matthiasblaesing
Copy link
Contributor

You said, that the changes work with JDK 11+ only. The difference: If you make the build compatible with JDK 8 + 11, everybody will cheer up and I'm the first to congratulate, if you create a build, that can't be build (there is not yet a release of JDK 11), you just break the project.

@wltjr
Copy link
Contributor Author

wltjr commented Aug 19, 2018

Yes because of changes in the JDK that are incompatible between Java 8 and 11+. Unsafe is deprecated and location changed, as of Java 9. They just waited till 11 to force it on others. This change does work for JDK 10, the current JDK. Just not Java 8.

Likely have to switch sources or some other means to support 8 and 11+. It is a lot of work to support both via some means. Or cut one off, saying NB needs JDK 10+ to build vs 8. Which would make it more forward looking. Without such changes, next month when JDK 11 is released. The latest Netbeans will not be able to build using the latest/current JDK. We will be right back here again, then instead of now. I do not understand the requirement for the latest Netbeans to be able to build or run under older JDKs. There is NB 8.x For Java 8. The latest NB should be good for 10+, current and upcoming JDKs.

JDK 11 has been available for months, just official release is in September. Even JDK 12 is available, which I need to start building stuff against. I rather be ready for a new JDK before it releases, then after it releases. Playing catch up after a new JDK release. I had to do a tremendous amount of playing catch up for 9. I did not have that problem for 10, or 11, nor will I with 12. Just need to address the things that break under newer. I rather not fall behind again.

@geertjanw
Copy link
Member

Please let’s have this discussion on the dev mailing list, not in a pr.

@matthiasblaesing
Copy link
Contributor

matthiasblaesing commented Aug 19, 2018

You want to change the policy of netbeans, that can't be done with a small PR. You have an opinion and I respect that, but at this point you have yet to prove, that you are willing to work on the project.

Your change breaks building on JDK 8 (at least you said so), but you also did not do the work necessary to make the whole project to build on JDK 11. So after your changeset is applied the project is not buildable anymore. That won't fly.

I gave my reasoning, I offered pointers what could be done, others gave points what could be done. I'll lock the conversation here, so that if interest is there, it is moved to the mailinglist.

@apache apache locked as resolved and limited conversation to collaborators Aug 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants