Conversation
mbien
added
Java
|
javadoc doesn't build on JDK 25, i saw that already in #8482 but hoped that it would work as soon we bump nb-javac (like it was the case in the past) see #8633 for javadoc fix for hints |
mbien
added
JavaDoc
|
FWIW, I've opened: |
@lahodaj thanks! I can refresh this PR once a staged build is available and drop the experimental nb-javac17 bits. |
|
switched to a temporary hosted build of nb-javac 25b29 until its available, the same tests are failing as before (when the experimental build was used) |
...on/test/unit/src/org/netbeans/modules/java/completion/JavaCompletionTask114FeaturesTest.java
Outdated
Show resolved
Hide resolved
|
|
Thanks. FWIW, I've opened: (The failure in LSP tests/ |
|
will take a look at the (hopefully) last remaining failing test later (java.source.base). update:
which is due to this new transformation within nb-javac: java.nio.file.FileSystems.newFileSystem($path, $env, $classloader) :: $path instanceof java.nio.file.Path
=>
java.nio.file.FileSystems.newFileSystem($path.toUri(), $env, $classloader)
;;the fascinating detail here is that the second factory method fails, since the uri checks are different from the path checks: // JDK 13+ API
FileSystem fs1 = FileSystems.newFileSystem(
Path.of("/mnt/ram/netbeans/java/java.source.base/build/test/unit/work/o.n.m.j.s.p.M/testJavac/mr.jar"),
Map.of(),
(ClassLoader)null
);
// JDK 8 API
FileSystem fs2 = FileSystems.newFileSystem(
Path.of("/mnt/ram/netbeans/java/java.source.base/build/test/unit/work/o.n.m.j.s.p.M/testJavac/mr.jar").toUri(),
Map.of(),
(ClassLoader)null
);first call creates a Javadoc makes also slightly different promises path variant, uri variant. javac 24 worked since it didn't use the env param and used the path variant, essentially ending up doing something similar to the Easiest fix would be to bump nb-javac to 17 to drop some of the transformations which aren't needed for NB anyway. But we would have to find a new maven namespace for that most likely. Second option is to use the path+env variant on JDK 13+ via reflection, and the path (without env) variant on <13. |
mbien
force-pushed
the
nb-javac25ea
branch
2 times, most recently
from
258f58f to
7411bdc
Compare
This library exists to support NetBeans, so why is upgrading to JDK 17 an issue?! If it needs to move to new hosting and to a new Maven namespace to ensure it's properly responsive to our requirements, then let's work out how that happens. Perhaps a dev@ thread? I had previously mentioned to @geertjanw that FoAN would be a better home for this. Of course, doing all that within the week left to get this required change in before freeze might be tricky! Still, this upgrade needs to be in NB27. |
|
taking a look at update: testcase is named after 190101, unfortunately I can't find the issue outside of https://github.com/emilianbold/netbeans-releases/commit/dc3c8858a8b5a507227370ebd7968df9b6f1cc78 will likely turn the test off |
|
yey, all java tests green! will rebase/squash to a reasonable amount of commits, remove merge commits and give co-author credits. PR ready to review, all what is missing is a nb-javac release + updating artifact locations. (keeping it as draft so that nobody merges it by accident with the dependency pointing to my raspi) |
|
noticed during code scanner smoke test: public static void main(String[] args) {
new WeakReference<>(null) {
private Object hardRef = null;
};
}will now cause a javac error. |
Uhhhh. Thanks!!! Filled as: |
|
squashed into logical units, the idea is to revert or drop the last commit with the next nb-javac / JDK build. |
|
last showstopper is having no line numbers in stack traces the emitted classes in nb-javac do seem to have a line number table ( stack trace looks like: building nb-javac without passing other curiosity:
opened JaroslavTulach/nb-javac#32 |
|
there is now a nb-javac build based on 25b31 available
everything should be still green after that |
mbien
force-pushed
the
nb-javac25ea
branch
2 times, most recently
from
a41c4a8 to
f3cb49c
Compare
|
something is wrong, JDK-8361445 doesn't seem to be in the nb-javac build sources of the Lint class do also not seem to include the fix: // Given a @SuppressWarnings annotation, extract the recognized suppressions
private EnumSet<LintCategory> suppressionsFrom(Attribute.Compound suppressWarnings) {
EnumSet<LintCategory> result = LintCategory.newEmptySet();
Attribute.Array values = (Attribute.Array)suppressWarnings.member(names.value);
for (Attribute value : values.values) {
Optional.of((String)((Attribute.Constant)value).value)^^L 533 JDK diff shows that it should be in it openjdk/jdk@jdk-25+29...jdk-25+31 edit: decompiled the class to double check and it seems to confirm it that the release isn't based on build 31: ...
private EnumSet<LintCategory> suppressionsFrom(Attribute.Compound suppressWarnings) {
EnumSet<LintCategory> result = LintCategory.newEmptySet();
Attribute.Array values = (Attribute.Array)suppressWarnings.member(this.names.value);
for (Attribute value : values.values) {
Optional.of((String)((Attribute.Constant)value).value).flatMap(LintCategory::get).filter(lc -> lc.annotationSuppression).ifPresent(result::add);
}
return result;
}here the JDK commit for comparison (compare missing |
mbien
added
the
do not merge
mbien
force-pushed
the
nb-javac25ea
branch
2 times, most recently
from
39d5d08 to
a793032
Compare
- SourceUtils: update main method detector to work on JDK 25 without preview flag set - ErrorHintsProviderTest: update for javac 25 (error code changed) - account for internal API changes
- fixes ConvertVarToExplicitTypeTest and LambdaTest Co-authored-by: Michael Bien <mbien42@gmail.com>
- mostly whitespace changes
JavaRefactoringActionsProviderTest#test190101: - javac 25 assumes everything without a class declaration is a compact source file -> disabled JavaCompletionTaskXXXFeaturesTest: - Records (final) are a JDK 16+ feature. Reflecting this in the tests. - moving into the JDk 16 completion tests, using the appropriate source level. Co-authored-by: Jan Lahoda <jan.lahoda@oracle.com>
mbien
removed
the
do not merge
|
second attempt to switch to release artifacts (25+31.1) |
lahodaj
left a comment
lahodaj
left a comment
There was a problem hiding this comment.
Looks good to me, thanks!
| assertEquals(++expectedCount, called.get()); | ||
| } | ||
|
|
||
| // hg blame: "#190101: preventing exceptions from various refactorings for very broken sources (without a top-level class)" |
There was a problem hiding this comment.
Maybe there should be a GH Issue for this (although we have many GH issues, of course).
There was a problem hiding this comment.
we could potentially fix the test in a followup PR if we know what it should do.
My thoughts were that I wasn't sure if it was still needed. Because if javac inserts the compact-file body, the original scenario can't really happen anymore most likely (refactoring plugins throwing exceptions when the class decl line was missing).
btw the reason it failed was because assertNull(ui); was not null.
I couldn't find the issue the test referenced unfortunately (#8572 (comment))
|
thanks a lot for the help @lahodaj and review! merging. |
| 9FB1613756338EE706C23D53F1423E6925D127C8 com.dukescript.nbjavac:nb-javac:jdk-24-ga | ||
| AD9963C5C1DA19B72628F48364562C014A972B69 com.dukescript.nbjavac:nb-javac:jdk-24-ga:api | ||
| CD9EA8DDE23DF41A129D563A51C9A2E502BD85BF com.dukescript.nbjavac:nb-javac:jdk-25+31.1 | ||
| 183E5391BCCC0A27AE0F5014A3A9FA7F93214911 com.dukescript.nbjavac:nb-javac:jdk-25+31.1:api |
4 participants
Upgrade javac integration to nb-javac based on JDK 25 build 31
this is not meant to be integrated yet, the wrapped nb-javac is an experimental build which was backported to JDK 17 instead of 8 to reduce the delta to upstream. It was built from this old branch I created a while ago.nb-javac 25 is not available yet, uses a temporary build for now with extra patches.refs to javac regressions which were discovered during testing: