Skip to content

poc: embed nb-javac compiler into the javac plugin - #244

Open
robstryker wants to merge 10 commits into
eclipse-jdtls:mainfrom
robstryker:nb-javac-poc
Open

poc: embed nb-javac compiler into the javac plugin#244
robstryker wants to merge 10 commits into
eclipse-jdtls:mainfrom
robstryker:nb-javac-poc

Conversation

@robstryker

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread org.eclipse.jdt.core.javac/.classpath Outdated
@datho7561

Copy link
Copy Markdown
Contributor

Something's up, the CI job isn't finishing in the usual amount of time.

@robstryker

robstryker commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

● nb-javac Shading Project Report

Work Completed

Successfully created a Maven-based shading solution for nb-javac to prevent OSGi/Equinox classloader conflicts
in Eclipse extensions. The repository at https://github.com/nb-javac-shaded/nb-javac-shaded publishes two shaded
artifacts via JitPack.

Shading Implementation

Complete package relocation of all nb-javac classes to shaded.* prefix:

  • com.sun.tools.* → shaded.com.sun.tools.*
  • com.sun.source.* → shaded.com.sun.source.*
  • javax.annotation.processing.* → shaded.javax.annotation.processing.*
  • javax.lang.* → shaded.javax.lang.*
  • javax.tools.* → shaded.javax.tools.*

Multi-release JAR support - Critical discovery and fix:

  • Maven Shade plugin rewrites bytecode in META-INF/versions/* but doesn't relocate directory structure
  • Created relocate-mrjar-classes.sh post-processing script to move versioned classes from
    META-INF/versions//com/sun/tools/ to META-INF/versions//shaded/com/sun/tools/
  • Preserves Java 16+ language features (records, sealed classes, pattern matching) - 691 versioned classes
    properly relocated
  • Without this fix, JVM would fail to load Java 16+ optimized classes due to path/package mismatch

Service file transformation:

  • Added ServicesResourceTransformer to relocate META-INF/services/ entries
  • Both filenames and contents updated: javax.tools.JavaCompiler → shaded.javax.tools.JavaCompiler
  • Enables ServiceLoader to find services using shaded package names

Source JAR generation for IDE debugging:

  • Enabled createSourcesJar=true and shadeSourcesContent=true in Maven Shade plugin
  • Created relocate-source-dirs.sh to reorganize source directories (Maven Shade rewrites package declarations
    but not directory structure)
  • Flattened OpenJDK module structure (src/MODULE/share/classes/* → direct package paths) for IDE compatibility
  • Preserved nbjavac package sources: nb-javac stores these in OpenJDK directory structure but with package
    nbjavac; declarations - script now extracts from make/ and relocates files based on package declarations
  • Final structure: shaded/com/sun/tools/javac/Main.java with package shaded.com.sun.tools.javac; - directories
    and package declarations perfectly aligned

Build hygiene:

  • Changed sources dependency scope from compile to provided to prevent source files being bundled into main JARs
  • Aligned Maven version with Git tag (jdk-26-35 using dashes, not plus signs, for JitPack compatibility)

Outstanding Issue: DocLint Service Loading

Problem Statement

Eclipse plugin cannot receive JCDiagnostic objects for Javadoc validation. The root cause is that DocLint
service lookup fails when using shaded nb-javac JARs.

Technical Details

Current behavior:

  • nb-javac contains com.sun.tools.doclint.DocLint (interface) and DocLint$NoDocLint (fallback implementation)
  • We shaded these to shaded.com.sun.tools.doclint.DocLint
  • nb-javac code calls: ServiceLoader.load(DocLint.class, ClassLoader.getSystemClassLoader())
  • After shading, this becomes: ServiceLoader.load(shaded.com.sun.tools.doclint.DocLint.class, ...)
  • ServiceLoader looks for providers of shaded.com.sun.tools.doclint.DocLint
  • But the JDK's jdk.javadoc module provides com.sun.tools.doclint.DocLint (unshaded)
  • Mismatch → service not found → falls back to NoDocLint → no Javadoc diagnostics

Challenges Preventing Resolution

Option 1: Exclude DocLint from shading

  • Keep com.sun.tools.doclint.* at original package name
  • Problem: Recreates the original ClassCastException issue
    • Eclipse/Equinox might load DocLint from system JDK's javadoc module (via parent classloader)
    • Plugin bundles its own version from nb-javac JAR
    • When types are loaded by different classloaders: ClassCastException
    • This is the exact problem shading was meant to solve

Option 2: Programmatic service registration (adapter pattern)

  • Create adapter in Eclipse plugin: implements shaded.com.sun.tools.doclint.DocLint, delegates to unshaded JDK
    version
  • Register adapter as service provider for shaded interface
  • Problem: ServiceLoader limitations
    • No runtime API to register services programmatically
    • Would need META-INF/services/shaded.com.sun.tools.doclint.DocLint in plugin JAR
    • nb-javac code uses ClassLoader.getSystemClassLoader() which won't see plugin's services
    • Would require modifying nb-javac to accept different classloader (defeats purpose of using upstream
      nb-javac)

Option 3: OSGi Import-Package control

  • Keep com.sun.tools.doclint unshaded
  • Configure Eclipse bundle to NOT import com.sun.tools.doclint from system
  • Force bundle classloader to load it locally from nb-javac JAR
  • Unknown: Whether Eclipse/Equinox allows preventing system JDK package imports for com.sun.tools.* packages
    • Need to investigate Equinox classloader delegation rules for JDK-internal packages
    • May require specific OSGi framework properties or bundle manifest configuration
  • Risk: May still hit parent classloader delegation depending on Equinox configuration

Open Questions

  1. Can Eclipse/Equinox be configured to prevent loading com.sun.tools.doclint.* from the system classloader?
  2. Is the DocLint interface in nb-javac identical to the JDK's javadoc module version (would help assess safety
    of Option 1)?
  3. What is the actual classloader hierarchy in the Eclipse plugin when nb-javac code executes?
  4. Does Equinox have boot delegation rules that force certain com.sun.* packages to load from the system?

@robstryker

Copy link
Copy Markdown
Contributor Author

We are down to 20 regressions, which is pretty solid.

364a365
> org.eclipse.jdt.core.tests.javac.RegressionTests testlombok
515,516d515
< org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest15 testConcatToTextBlock7
< org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest15 testNoConcatToTextBlock8
528,531d526
< org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest1d8 testConvertToLambda15
< org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest1d8 testConvertToLambda31
< org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest1d8 testNoRedundantNonNullInConvertIterableForLoop
< org.eclipse.jdt.ui.tests.quickfix.AssistQuickFixTest21 testDoNotConvertPatternInstanceofToSwitch1
589d583
< org.eclipse.jdt.ui.tests.quickfix.ConvertIterableLoopQuickFixTest testWrongIteratorMethod_bug411588
608d601
< org.eclipse.jdt.ui.tests.quickfix.JavadocQuickFixTest9 testMissingModuleComment
640d632
< org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest testCastMissingInFieldDecl
664,665d655
< org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest testTypeParametersToRawTypeReference09
< org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest testUncaughtException
667d656
< org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest testUncaughtExceptionOnSuper1
670d658
< org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest testUnimplementedMethodsExtendingGenericType1
790d777
< org.eclipse.jdt.ui.tests.quickfix.NullAnnotationsQuickFixTest1d8 testBug531511_unconfigurable_decl
844d830
< org.eclipse.jdt.ui.tests.quickfix.NullAnnotationsQuickFixTest testRemoveRedundantAnnotation6
848d833
< org.eclipse.jdt.ui.tests.quickfix.QuickFixTest14 testAddDefaultCaseSwitchStatement1
853d837
< org.eclipse.jdt.ui.tests.quickfix.QuickFixTest15 testTextBlockGetNeedHigherComplianceProposal
858a843
> org.eclipse.jdt.ui.tests.quickfix.QuickFixTest17 testAddSealedAsDirectSuperTypeProposal1
883d867
< org.eclipse.jdt.ui.tests.quickfix.QuickFixTest22 testRenameToUnnamedProposal5
970,971d953
< org.eclipse.jdt.ui.tests.quickfix.UnresolvedMethodsQuickFixTest testMethodAssignedToWildcard2
< org.eclipse.jdt.ui.tests.quickfix.UnresolvedMethodsQuickFixTest testMethodInAnonymousCovering3
975d956
< org.eclipse.jdt.ui.tests.quickfix.UnresolvedMethodsQuickFixTest testSuperMethodInvocation
984d964
< org.eclipse.jdt.ui.tests.quickfix.UnresolvedVariablesQuickFixTest testVarInConstructurInvocation

fbricon and others added 4 commits May 1, 2026 21:24
Signed-off-by: Fred Bricon <fbricon@gmail.com>
Signed-off-by: Rob Stryker <rob@oxbeef.net>

Update WIP with newest jars

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Configure Maven to download nb-javac shaded JARs from JitPack

- Add JitPack repository
- Add dependencies for shaded nb-javac artifacts (jdk-26-35)
- Configure maven-dependency-plugin to copy JARs to lib/
- Ignore downloaded JARs in .gitignore

JARs are no longer committed to git and are downloaded during build.

Bad values in .classpath

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Disable problematic suite

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Use newest jars, for now

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Rebase update

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Update source location for shaded jars

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Try to use shaded doclint

Signed-off-by: Rob Stryker <rob@oxbeef.net>

nnb-doclint-shaded jar was not added due to gitignore

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Fix ServiceLoader discovery for shaded DocLint implementation

Renamed service file from com.sun.tools.doclint.DocLint to
shaded.com.sun.tools.doclint.DocLint to match the relocated
package structure in nb-javac-shaded. ServiceLoader requires
the service file name to match the fully qualified interface
name, including the shaded. prefix.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Rob Stryker <rob@oxbeef.net>
Signed-off-by: Rob Stryker <rob@oxbeef.net>
robstryker added 2 commits May 5, 2026 12:30
Signed-off-by: Rob Stryker <rob@oxbeef.net>
Signed-off-by: Rob Stryker <rob@oxbeef.net>
Signed-off-by: Rob Stryker <rob@oxbeef.net>
robstryker and others added 3 commits May 6, 2026 15:49
Changed jdt-core-fork-tests repository URL from Jenkins artifact URL
(which requires authentication) to local build directory. The
Jenkinsfile stage 1 already builds these tests locally, so we can
reference them directly without needing HTTP access.

This eliminates the HTTP 403 error when accessing lastSuccessfulBuild
artifacts.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed from ${session.executionRootDirectory} to
${maven.multiModuleProjectDirectory} which is properly resolved
in Tycho builds. The previous property caused "Illegal character
in path" error.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
p2 repository requires absolute URL with file:// protocol.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Comment thread pom.xml
<id>jdt-core-fork-tests</id>
<layout>p2</layout>
<url>https://ci.eclipse.org/ls/job/jdt-core-incubator/job/dom-with-javac/lastSuccessfulBuild/artifact/repository/target/repository/</url>
<url>file://${maven.multiModuleProjectDirectory}/forkedTests/repository/target/repository/</url>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's built locally it should be installed locally as well, so it won't need to be set up as a repo I think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants