Return amiga.bin-path with forward slashes even on win32

[timfel]

The win32 APIs accept paths with forward slashes. Making the command amiga.bin-path return the path with forward slashes makes it possible and easy in our VSCode projects to have custom tasks that execute binaries from the extension via e.g. "command": "${command:amiga.bin-path}/exe2adf" for example. This already works on Unix platforms, but on Windows this fails because the backslashes mess up the JSON string (they are not escaped again and just added into the JSON string by VSCode verbatim, making them into escape sequences)

[BartmanAbyss]

Hi, I'm a bit confused. the tasks.json file from the template project contains "command": "${command:amiga.bin-path}\\gnumake.exe", and this works fine on Windows. It even works when I change it to "command": "${command:amiga.bin-path}/gnumake.exe"

[timfel]

Hi, I'm a bit confused. the tasks.json file from the template project contains "command": "${command:amiga.bin-path}\\gnumake.exe", and this works fine on Windows. It even works when I change it to "command": "${command:amiga.bin-path}/gnumake.exe"

Yes, it seems that VSCode does extra escaping in that context, but not in others - sorry for the bad example. I would like to also use this for environment specs and compiler definitions for CMake, and there the escaping is not done. Forward slashes fix that.

[BartmanAbyss]

Can you post a concrete example?

[timfel]

I put this in .vscode/cmake-kits.json

[
  {
    "name": "GCC Bartman m68k Win32",
    "toolchainFile": "${workspaceFolder}/deps/AmigaCMakeCrossToolchains/m68k-bartman.cmake",
    "environmentVariables": {
      "PATH": "${command:amiga.bin-path}/opt/bin;${command:amiga.bin-path};${command:amiga.bin-path}/opt/m68k-amiga-elf/bin;${env:PATH}"
    },
    "compilers": {
      "C": "${command:amiga.bin-path}/opt/bin/m68k-amiga-elf-gcc.exe"
    },
    "preferredGenerator": {
      "name": "Ninja"
    },
    "cmakeSettings": {
      "M68K_CPU": "68000",
      "TOOLCHAIN_PREFIX": "m68k-amiga-elf",
      "TOOLCHAIN_PATH": "${command:amiga.bin-path}/opt",
      "CMAKE_AR": "${command:amiga.bin-path}/opt/m68k-amiga-elf/bin/ar.exe"
    },
    "keep": true
  }
]

Then I run cmake. Without my change, this gives

[proc] Executing command: "C:\Program Files\CMake\bin\cmake.EXE" --no-warn-unused-cli -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=TRUE -DCMAKE_BUILD_TYPE:STRING=Debug "-DCMAKE_C_COMPILER:FILEPATH=c:\Users\Tim Felgentreff\.vscode\extensions\bartmanabyss.amiga-debug-1.6.7\bin\win32/opt/bin/m68k-amiga-elf-gcc.exe" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=c:/Users/Dev/TheGreatFlight/deps/AmigaCMakeCrossToolchains/m68k-bartman.cmake -DM68K_CPU:STRING=68000 -DTOOLCHAIN_PREFIX:STRING=m68k-amiga-elf "-DTOOLCHAIN_PATH:STRING=c:\Users\Tim Felgentreff\.vscode\extensions\bartmanabyss.amiga-debug-1.6.7\bin\win32/opt" "-DCMAKE_AR:STRING=c:\Users\Tim Felgentreff\.vscode\extensions\bartmanabyss.amiga-debug-1.6.7\bin\win32/opt/m68k-amiga-elf/bin/ar.exe" -Sc:/Users/Dev/TheGreatFlight -Bc:/Users/Dev/TheGreatFlight/build -G Ninja
[cmake] Not searching for unused variables given on the command line.
[cmake] -- The C compiler identification is GNU 12.1.0
[cmake] CMake Error at build/CMakeFiles/3.23.1/CMakeCCompiler.cmake:1 (set):
[cmake]   Syntax error in cmake code at
[cmake] 
[cmake]     C:/Users/Dev/TheGreatFlight/build/CMakeFiles/3.23.1/CMakeCCompiler.cmake:1
[cmake] 
[cmake]   when parsing string
[cmake] 
[cmake]     c:\Users\Tim Felgentreff\.vscode\extensions\bartmanabyss.amiga-debug-1.6.7\bin\win32/opt/bin/m68k-amiga-elf-gcc.exe
[cmake] 
[cmake]   Invalid escape sequence \U
[cmake] Call Stack (most recent call first):
[cmake]   CMakeLists.txt:2 (project)

With my change, it works.

[timfel]

Come to think of it, maybe the issue isn't actually VSCode not escaping it right, but simply that some things are not executed from VSCode, but passed as arguments on a commandline, in which case they would need double escaping 🤔

So this could also bite us also in custom tasks that take the bin-path as argument.

In either case, forward slashes would just get around the problem, without having to fix all potential points where these paths are passed along.

[BartmanAbyss]

Yeah, I think it's more likely that the problem is in the cmake extension. The windows shell doesn't really require escaping of backslashes, as you can see in the path of the cmake executable. While I understand that your PR fixes this issue and doesn't seem to create any problems for the default Makefile, I'm still a bit uneasy about implementing this 'workaround' as it really feels like quite the hack.

[timfel]

Yeah, I think it's more likely that the problem is in the cmake extension. The windows shell doesn't really require escaping of backslashes, as you can see in the path of the cmake executable. While I understand that your PR fixes this issue and doesn't seem to create any problems for the default Makefile, I'm still a bit uneasy about implementing this 'workaround' as it really feels like quite the hack.

It may be a bit hacky, sure, but there a no APIs on Win32 that would expect backslashes - forward slashes work just as well. Is there anything I can do to convince you? From what I see, I cannot actually fix this in VSCode at all - we could add a hack to the CMake extension, but then the argument case still won't work correctly. I would need extra facilities from VSCode core to escape this in other places where it can be useful. :(

[timfel]

Also (only slightly related): is there a reason you do the following?

https://github.com/BartmanAbyss/vscode-amiga-debug/blob/master/src/extension.ts#LL124C3-L124C3

this.binPath = path.join(this.extensionPath, "bin", process.platform);

and then in the amiga.bin-path command you do it again? https://github.com/BartmanAbyss/vscode-amiga-debug/blob/master/src/extension.ts#L160

vscode.commands.registerCommand('amiga.bin-path', () => path.join(this.extensionPath, 'bin', process.platform)),

Can the this.extensionPath change during execution?

[timfel]

I was browsing a bit, and afaict the only way for me to get the bin-path with generic directory separators (in the way that e.g. C++17 defines generic directory separators, i.e., forward slashes, not backslashes) is to make another VSCode extension, depend on your extension, and expose a command amiga.generic-bin-path. Since you have much more experience with VSCode extensions and APIs, do you know of another way?

[BartmanAbyss]

Also (only slightly related): is there a reason you do the following?

https://github.com/BartmanAbyss/vscode-amiga-debug/blob/master/src/extension.ts#LL124C3-L124C3

this.binPath = path.join(this.extensionPath, "bin", process.platform);

and then in the amiga.bin-path command you do it again? https://github.com/BartmanAbyss/vscode-amiga-debug/blob/master/src/extension.ts#L160

vscode.commands.registerCommand('amiga.bin-path', () => path.join(this.extensionPath, 'bin', process.platform)),

This is probably an oversight as the this.binPath variable is quite new.

[BartmanAbyss]

I put this in .vscode/cmake-kits.json

[
  {
    "name": "GCC Bartman m68k Win32",
    "toolchainFile": "${workspaceFolder}/deps/AmigaCMakeCrossToolchains/m68k-bartman.cmake",
    "environmentVariables": {
      "PATH": "${command:amiga.bin-path}/opt/bin;${command:amiga.bin-path};${command:amiga.bin-path}/opt/m68k-amiga-elf/bin;${env:PATH}"
    },
    "compilers": {
      "C": "${command:amiga.bin-path}/opt/bin/m68k-amiga-elf-gcc.exe"
    },
    "preferredGenerator": {
      "name": "Ninja"
    },
    "cmakeSettings": {
      "M68K_CPU": "68000",
      "TOOLCHAIN_PREFIX": "m68k-amiga-elf",
      "TOOLCHAIN_PATH": "${command:amiga.bin-path}/opt",
      "CMAKE_AR": "${command:amiga.bin-path}/opt/m68k-amiga-elf/bin/ar.exe"
    },
    "keep": true
  }
]

Have you tried removing the path from "compiler" "C" altogether? You're already specifying ${command:amiga.bin-path} in PATH.

[timfel]

Have you tried removing the path from "compiler" "C" altogether? You're already specifying ${command:amiga.bin-path} in PATH.

I still get an issue then with sysroot. I've played around a bit more and I found a workaround for this issue that I can add to the CMake toolchain file. I still think forward slashes would be nice, but we can close this :)

[BartmanAbyss]

great!