Casing of drive letter differs between ${workspaceRoot} and setBreakpointRequest

[DanTup]

I'm not really sure you'll consider this a bug in Code, but it's causing bugs for me and I'm not sure if I can easily fix it.

When a user sets a launch config using ${workspaceRoot}, the path comes through with an uppercase drive letter:

C:\Users\danny\Desktop\DartHelloWorld\bin\main.dart

However if I place breakpoint in that file then the setBreakpointRequest comes through with a lowercase drive letter:

c:\Users\danny\Desktop\DartHelloWorld\bin\main.dart

These paths get converted to file:/// URIs by two different applications for my extension (the breakpoints are converted to URIs by my debug adapter, whereas the source file is being converted to a URI inside the VM). These URIs therefore don't match and are being compared case-sensitively and my breakpoint doesn't get hit.

I'm going to ask whether the VM can do case-insensitive comparisons but suspect it's a big job. I'm going to try and put a workaround in my DA for now; but I wonder if having these consistent in Code is an easy change?

[weinand]

Yes, drive-letter casing is a mess.
Different casing should not make a difference on Windows but for node.js it makes a difference and in our node-debugger we register breakpoints via a regular expression that ignores the casing.

IIRC VS Code does not try to normalise the drive letter to either upper or lower case because this doesn't help a lot for the following reason:

There is always the case that you launch a debuggee from the integrated terminal (or even an external terminal): In this case VS Code has no influence on the drive letter case and there is still the possibility that a (normalized) VS Code path would be compared to an un-normalized native path coming from the debuggee.

So my only recommendation is to deal with drive letter case mismatches yourself.

@isidorn please chime in if I misspoke.

[DanTup]

@weinand It actually seems like this has affected a bunch of things and there have already been some fixes in Code that now uppercase the drive letter:

a2ebd67
0d66fa0

Would it make sense to be consistent with them and do the same in the debugger?

[weinand]

@isidorn what's your take?

But as I tried to explain above, normalising "some" paths in VS Code cannot fix the fundamental issue. If you launch your a program in a command shell that results in lower case drive letters, these will not match the upper case drive letters paths received from VS Code.

[DanTup]

Understood; but in this case both paths come from Code (cwd and setBreakpointRequest) so if Code was consistent with itself, this issue at least would be solved (it's possible it used to be consistent before cwd was changed).

Ofc I'm perfectly happy to field any issues resulting from paths that didn't origin from Code.

[weinand]

The path received in "setBreakpointRequest" does not necessarily originates from VS Code.
If you set a breakpoint in a (readonly) source received from the DA, the subsequent "setBreakpointRequest" will use this source path without modification. So in this case the drive letter has the case that your DA returned previously.

[DanTup]

@weinand Understood; though those paths will match the VM already. The breakpoint sending lowercase paths here is one I'm adding on a normal source file that I've opened from the explorer and just pressed F9 on.

I've put a workaround in for now; but I do think Code should be try to be consistent with the paths that originate from it since it looks like this is a bit of a minefield.

[isidorn]

Reopening to investigate

[DanTup]

I'm fairly certain that 0d66fa0 caused this. I mis-read the date on it earlier, but it was actually very recent. So previously, Code's cwd and breakpoints matched before but now they don't.

I have a workaround going out at the end of the month, but I really think this has to be made consistent in Code; it's likely to cause many bugs that could go unnoticed by extension authors and cause missed breakpoints for users (this is actually an ongoing battle for me, because there are many components outside of my control that are treating windows paths case-sensitively; it feels like I'm playing whack-a-mole 😞).

[isidorn]

@DanTup you are correct, I broke it in that commit. However you will see what issue that commit is linked to, so that was the reason for the change.
Will probably have to revert it.

[weinand]

@isidorn If our (new?) strategy is to normalise drive letter casing to upper case, then reverting your change seems to be the wrong approach. Instead we should normalise drive letters everywhere where a path leaves VS Code, e.g. when sending paths to a DA.

[DanTup]

Yeah, my preference would also be that drive letters are just always normalised to uppercase everywhere rather than reverting. And hopefully you can agree that any time something new comes up (like this one) the same fix is applied (I appreciate you shouldn't need to be messing with casing of drive letters; it's as annoying to me as it is to you; but we're all relying on other peoples coed we can't neceesarily easily influence/fix!).

[felixfbecker]

A lot of people are reporting that VS Code 1.20 broke their PHP debugging: xdebug/vscode-php-debug#239
XDebug always uses lowercase drive letters.

[DanTup]

cries

[isidorn]

@felixfbecker can you fix this on the php debug side?
If not, we can consider this as a candidate to revert this change

[DanTup]

If this change is reverted without 0d66fa0 also being reverted (and my fix in Dart Code removed and published), it'll break Dart and Flutter :(

Whatever way you choose to present the drive letters, it really has to be consistent between the path we get given for workspaceRoot and in breakpointRequest (it won't if you just revert this one change).

[isidorn]

@DanTup you are correct.
We just discussed this in the meeting and we should have made an anouncment about this change, however we feel like this should not be reverted and php debug should not rely on drive letters being small case.
So it would be great if @felixfbecker could fix it on the php side. If you do not have time please let us know and we can provide a PR in your extension.

[isidorn]

@felixfbecker just a gentle reminder if we should help with the PR for PHP?

[felixfbecker]

Should be fixed in v1.12.2