Theia send canonical paths to the adapter

[wss29]

Feature Description:

When Debug between theia and cdt-gdb-adapter, there is a problem with the path theia sends to the adapter, it is not only about cdt-gdb-adapter, maybe another adapter has the same problem, the discussion detail is eclipse-cdt-cloud/cdt-cloud#42 (reply in thread), should theia send canonical paths to the adapter?

[JonasHelming]

@jonahgraham Can you clarify?

[jonahgraham]

Theia should try to send canonical paths to the adapter as this issue may affect all adapters (not just cdt-gdb-adapter).

The issue is (extracted from the log) that sometimes Theia is sending d: and sometimes D: as drive letter. This seems to indicated that Theia isn't sending canonical paths.

e.g., from the log in eclipse-cdt-cloud/cdt-cloud#42

Initial setBreakpoints before configurationDone:

[07:21:05.912 UTC] From client: setBreakpoints({"source":{"name":"main.cpp","path":"d:\\WorkGroup\\workspace\\workspace\\cpp\\src\\main.cpp"},"sourceModified":false,"breakpoints":[{"line":19},{"line":22}],"lines":[19,22]})

Later setBreakpoints after user unchecked the breakpoint in some way:

[07:21:15.058 UTC] From client: setBreakpoints({"source":{"name":"main.cpp","path":"D:\\WorkGroup\\workspace\\workspace\\cpp\\src\\main.cpp","sourceReference":0},"sourceModified":true,"breakpoints":[{"line":22}],"lines":[22]})

As you can see in the former it uses d: and in the latter it uses D:

VSCode has a similar problem a long time ago, see microsoft/vscode#43959

[tsmaeder]

@jonahgraham I'm not sure this has anything to do with Theia being inconsistent. If I read the code correctly, the only way we would update the path from a lower-case d:\foo\bar.ts to D:\foo\bar.ts is if the debug adapter send us an upper-case drive letter in an event about the breakpoint in question. So if we send upercase drive letters (which you probably mean by "canonical path"), a debug adapter could well send us back a lower-case drive letter in an event. So I don't think this is fixable on the Theia side.
IMO, the most promising approach is to respect what the underlying file system is telling us. NTFS, for example, is case-sensitive in that you can create a file that's called FoOBar.Ts and it will be listed as FoOBar.Ts. Middleware components (and Theia functions as such in this case) fiddling with the casing can only add to the confusion.
In your concrete case, we're converting the source file uri to a lowercase path, so if we stop doing that you should be getting uppercase drive letter, AFAIK.

[tsmaeder]

In the end, the concrete reason we send lowercase drive letters to the debug adapter is because we get the uri from a string map key in MarkerManager.uri2markerCollection. Since the VS Code URI class makes drive letters lowercase in toString(), we get a lowercase drive letter as a key in the map.

Interesting also PR descriptions like this one: microsoft/vscode#83060. I guess the VS Code folks are well aware that their approach is broken, but it's too late to fix it.
May first impulse would be to rip out the VS Code URI class from our implementation, but that might wreak havoc when we interact with Monaco code. Event though I think we would be fine passing URI instances around, we can't be sure the VS Code internals don't do a new Uri(uri.toString()), which would end up with a "normalized" uri instead of what we pass in.

[tsmaeder]

One solution would be to not replace the source of the breakpoint with the one that the debug adapter returns from the setBreakpoints() request but to always send the original path. However I would have to convince myself that this does not lead to breakage in the debugger in the general case.

[tsmaeder]

A bit of a backgrounder on working with file systems in node: https://nodejs.org/en/learn/manipulating-files/working-with-different-filesystems

[jonahgraham]

If I read the code correctly, the only way we would update the path from a lower-case d:\foo\bar.ts to D:\foo\bar.ts is if the debug adapter send us an upper-case drive letter in an event about the breakpoint in question.

That sounds close to what is happening. What I see is insert breakpoint on d:\\WorkGroup\\workspace\\workspace\\cpp\\src\\main.cpp and when the breakpoint is hit, the adapter sends back the following because GDB has normalized the path to uppercase:

[07:21:08.880 UTC] To client: {"seq":0,"type":"response","request_seq":13,"command":"stackTrace","success":true,"body":{"stackFrames":[{"id":1000,"source":{"name":"main.cpp","path":"D:\\WorkGroup\\workspace\\workspace\\cpp\\src\\main.cpp","sourceReference":0},"line":19,"column":0,"name":"_fu1___ZSt4cout","instructionPointerReference":"0x004015a8"}],"totalFrames":1}}

This makes Theia open the D: version of the file, so future clicks on the breakpoint gutter now send D: paths to the adapter.

The adapter side needs a fix in eclipse-cdt-cloud/cdt-gdb-adapter#330. Whether Theia make a similar change on the client side of the adapter I leave up to you.

[tsmaeder]

@jonahgraham happy if you have a solution that works for the moment. I have this issue on my "todo" list, but it's on the back burner for the moment because the right thing to do is not clear to me at the moment.