Repository navigation
Materialize jdt:// sources to cache files for clients that only open file paths - #649
sisyphusend wants to merge 1 commit into
Conversation
…file paths
Stack frames paused in classes packaged in jars or the JRT runtime are
reported with a jdt://contents/... source uri, which only VS Code can
render through the jdt TextDocumentContentProvider registered by the
Red Hat Java extension. Any other DAP client can only open absolute
file paths, so those frames are rendered without source.
Materialize the source of such classes (attached source, or decompiled
source when java.debug.settings.debugSupportOnDecompiledSource is on)
into a cache file under
{java.io.tmpdir}/java-debug-sources/{jarName}/{package}/{Type}.java and
report its path instead. The decompiled line mapping APIs accept the
cache path and resolve it back to the class file.
When getFileURI returns null for a class file (JDTUtils.toUri() returns
null for jar class files in some language server versions, e.g. the
jdt.ls 1.63.0 used by the Zed extension), fall back to resolving the
class file via the source containers so the source can still be
materialized. Unresolvable sources fall back to SourceType.REMOTE,
matching the contract documented on ISourceLookUpProvider#getSource.
Refs zed-extensions/java#326
|
@microsoft-github-policy-service agree |
f39a1b3 to
fdfbd3c
Compare
The upstream zed-industries/java-debug fork is archived, so point JAVA_DEBUG_PLUGIN_FORK_URL at a personal build that materializes jdt:// sources into cache files (microsoft/java-debug#649), making JDK/dependency sources visible while stepping in Zed.
wenyt (wenytang-ms)
left a comment
There was a problem hiding this comment.
Thanks for investigating this and providing the detailed reproduction. The compatibility issue reported in [zed-extensions/java#326](zed-extensions/java#326) is valid, but I’m not comfortable merging this approach.
This change replaces jdt:// sources with cached files for all clients, including VS Code. Preserving the existing VS Code Java experience is a requirement, and opening cached files changes how those documents integrate with the Java extension. The acknowledged breakpoint limitation and cache invalidation concerns also make this unsuitable as a global replacement.
Please keep the existing VS Code behavior unchanged and scope any alternative source handling to clients that explicitly opt in. Supporting DAP source requests would be worth exploring before introducing a file cache. The null-URI behavior associated with #624 should be investigated separately.
Problem
When a debug session pauses inside a class packaged in a jar or the JRT runtime (any JDK or third-party library class), the stack frame source is reported as a
jdt://contents/...uri withsourceReference: 0. That uri can only be rendered by clients that register ajdtcontent provider — in practice only VS Code with the Red Hat Java extension. Any other DAP client can only open absolute file paths or fetch contents through the DAPsourcerequest, so for those clients every library frame is source-less: stepping into JDK or dependency code works, but the editor shows nothing.Reported from Zed, which consumes the java-debug server through the (now archived)
zed-industries/java-debugfork: zed-extensions/java#326.Fix
JdtSourceLookUpProvider#getSourcenow materializes the source ofjdt://class files into a local cache file and reports its absolute path instead of the virtual uri:{java.io.tmpdir}/java-debug-sources/{jarName}/{package}/{Type}.java, written on demand and reused while present.src.zip, maven*-sources.jar): the attached source is written verbatim. Its line numbers match the class line tables exactly, so frames highlight correctly.java.debug.settings.debugSupportOnDecompiledSourceisON, the class is decompiled with the sameContentProviderManagerdecompiler the VS Code client uses. The original/decompiled line-mapping APIs accept the cache path (they resolve it back to thejdt://uri), so the existing remap keeps working.SourceType.REMOTE(asourceReferenceis assigned andSourceRequestHandlerservesgetSourceContents), which finally matches the contract already documented onISourceLookUpProvider#getSource: "In case of 'remote' a follow up 'source' request will be issued by the client".One visible behavior change for VS Code: frames in library classes will now open the materialized file instead of the
jdt://virtual document (same contents, real file). If that's undesirable for the VS Code experience, the materialization could instead be gated on the client (e.g. only when theinitializeclientIDis not a known jdt-content-provider client) — happy to adjust based on maintainer preference.Verification
Scripted end-to-end session against jdt.ls 1.63.0 with this plugin (built from this branch) loaded via the
bundlesinitialization option. Maven project withcommons-lang3:3.17.0, launchdemo.Main,stopOnEntry, then repeatedstepIn:Main.main(String[]) :7...\src\main\java\demo\Main.javaMain.main(String[]) :9StringUtils.capitalize(String) :538...\Temp\java-debug-sources\commons-lang3-3.17.0.jar\org\apache\commons\lang3\StringUtils.java— attached source, line 538 is exactlypublic static String capitalize(final String str)String.length() :1583...\Temp\java-debug-sources\java.base\java\lang\String.java— JDKsrc.zipsource, line 1583 is exactlypublic int length()No frame reported a non-absolute path anymore;
mvn checkstyle:checkpasses.Additional finding:
JDTUtils.toUri(classFile)returns null in some language server versionsWhile testing this patch against the jdt.ls 1.63.0 used by the Zed extension, library frames had no source at all — and a control run with an unpatched build of current
mainbehaves identically. The cause is #624:getFileURI(IClassFile)now delegates toJDTUtils.toUri(classFile), which returnsnullfor jar class files in that jdt.ls version, and a null uri makesconvertDebuggerSourceToClientreturn no source. This patch adds a fallback for that case (resolve the class file via the source containers directly), which restores source resolution even whentoUriyields null — it may be worth a look on its own.Notes
{jarName}so entries invalidate naturally; a JDK upgrade reuses thejava.basedirectory until the temp dir is cleared.Refs zed-extensions/java#326