search

apache / netbeans

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

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

Closed wltjr closed 6 years ago

wltjr commented 6 years ago

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 commented 6 years ago

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

matthiasblaesing commented 6 years ago

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

wltjr commented 6 years ago

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 commented 6 years ago

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

wltjr commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Closing this PR as it is incompatible with Java 8 and it was indicated, that there is no intention to fix that. (see comment https://github.com/apache/incubator-netbeans/pull/572#issuecomment-394120138)

wltjr commented 6 years ago

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 commented 6 years ago

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

wltjr commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Please use the mailinglist for discussions and questions.

Am 19. August 2018 00:12:20 MESZ schrieb William L Thomson Jr notifications@github.com:

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.

-- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/apache/incubator-netbeans/pull/572#issuecomment-414089308

-- Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

wltjr commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

matthiasblaesing commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

matthiasblaesing commented 6 years ago

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.