Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer possible to use cmd as clangd executable #683

Closed
JVApen opened this issue Sep 12, 2024 · 5 comments · Fixed by #730 or #758
Closed

No longer possible to use cmd as clangd executable #683

JVApen opened this issue Sep 12, 2024 · 5 comments · Fixed by #730 or #758
Labels
bug Something isn't working

Comments

@JVApen
Copy link
Contributor

JVApen commented Sep 12, 2024

The VS Code upgrade to 1.92 contains a new node version (20)
This version seems to have as a side effect that scripts can no longer be used as 'executable' on windows
(see similar issue for cmake: microsoft/vscode-cmake-tools#4037 (comment))

When using VS Code with 1.91, I can use a clangd.cmd that figures out the clangd version to use (the same as we use to compile). However from VS Code 1.92 this is no longer possible.

I'm no expert in typescript, though I believe this behavior can be re-enabled in ClangdContext::active by passing "shell: true" as part of the options of vscodelc.Executable.

Logs

[Error - 11:31:01 AM] Clang Language Server client: couldn't create connection to server.
Error: spawn EINVAL
	at ChildProcess.spawn (node:internal/child_process:421:11)
	at Object.spawn (node:child_process:800:9)
	at c:\Users\JVA\.vscode\extensions\llvm-vs-code-extensions.vscode-clangd-0.1.29\out\bundle.js:39:56906
	at async oc.createConnection (c:\Users\JVA\.vscode\extensions\llvm-vs-code-extensions.vscode-clangd-0.1.29\out\bundle.js:39:12067)
	at async oc.start (c:\Users\JVA\.vscode\extensions\llvm-vs-code-extensions.vscode-clangd-0.1.29\out\bundle.js:39:2940)

System information

Operating system: Windows

@JVApen JVApen added the bug Something isn't working label Sep 12, 2024
JVApen pushed a commit to JVApen/vscode-clangd that referenced this issue Oct 25, 2024
…s upgrade, it was possible to execute cmd/bat scripts as executable.

     After this update this was suddenly broken.
     Our use-case for using such a script has to do with the consistency of our clang-tooling.
     We build clang-format, clang-tidy, clangd ... all together and put this in a package manager.
     In the script, we first download these executables (when needed) and than start the downloaded clangd.
     As such, when fixing an issue in clang-tidy, the same issue will get fixed in the clangd exe.
     Without this script, it is impossible to automatically trigger this download and it introduces the risk that we do not update all the tooling to the latest version.
@JVApen JVApen closed this as completed Nov 10, 2024
@HighCommander4
Copy link
Contributor

Released in 0.1.30.

@HighCommander4
Copy link
Contributor

Sorry, had to back out the fix for this, together with #715, for causing #722. Reopening.

@JVApen
Copy link
Contributor Author

JVApen commented Nov 14, 2024

Thanks @HighCommander4, I will be able to look at this on Friday. Thanks for reverting and as such reducing the pressure

JVApen added a commit to JVApen/vscode-clangd that referenced this issue Nov 18, 2024
…t was possible to execute cmd/bat scripts as executable.

     After this update this was suddenly broken.
     Our use-case for using such a script has to do with the consistency of our clang-tooling.
     We build clang-format, clang-tidy, clangd ... all together and put this in a package manager.
     In the script, we first download these executables (when needed) and than start the downloaded clangd.
     As such, when fixing an issue in clang-tidy, the same issue will get fixed in the clangd exe.
     Without this script, it is impossible to automatically trigger this download and it introduces the risk that we do not update all the tooling to the latest version.

Fixes clangd#683

In this reapply, we quote both the command line and the arguments, such that both can contain spaces.
We also introduce the option useScriptAsExecutable, such that this is only enabled when the user wants it.
JVApen added a commit to JVApen/vscode-clangd that referenced this issue Nov 18, 2024
…t was possible to execute cmd/bat scripts as executable.

     After this update this was suddenly broken.
     Our use-case for using such a script has to do with the consistency of our clang-tooling.
     We build clang-format, clang-tidy, clangd ... all together and put this in a package manager.
     In the script, we first download these executables (when needed) and than start the downloaded clangd.
     As such, when fixing an issue in clang-tidy, the same issue will get fixed in the clangd exe.
     Without this script, it is impossible to automatically trigger this download and it introduces the risk that we do not update all the tooling to the latest version.

Fixes clangd#683

In this reapply, we quote both the command line and the arguments, such that both can contain spaces.
We also introduce the option useScriptAsExecutable, such that this is only enabled when the user wants it.
@HighCommander4
Copy link
Contributor

Reverted again in #736 for causing #734.

Discussion in #734 suggests that the issue is related to the default value of "clangd.useScriptAsExecutable" not being handled correctly in some environments, as e.g. this comment reports the issue being resolved if the user puts an explicit "clangd.useScriptAsExecutable": false into their settings.

Regardless of the specific fix, I'd also like to make a process change before re-landing a fix here again: I'd like to add a way to create an "unstable" / "nightly" release that shows up in https://github.com/clangd/vscode-clangd/releases/ and allows users to download and test a .vsix package, without pushing the new version to the VSCode Marketplace or OpenVSX yet. Then, we can ask users (e.g. the ones who reported experiencing #722 or #734) to test that version and verify that it works for them, before publishing the version to the marketplaces. I'll look at setting up a CI action for that in the coming days.

@HighCommander4
Copy link
Contributor

I'd also like to make a process change before re-landing a fix here again: I'd like to add a way to create an "unstable" / "nightly" release that shows up in https://github.com/clangd/vscode-clangd/releases/ and allows users to download and test a .vsix package, without pushing the new version to the VSCode Marketplace or OpenVSX yet. Then, we can ask users (e.g. the ones who reported experiencing #722 or #734) to test that version and verify that it works for them, before publishing the version to the marketplaces. I'll look at setting up a CI action for that in the coming days.

I put up a PR adding such a CI action at #748.

JVApen added a commit to JVApen/vscode-clangd that referenced this issue Dec 16, 2024
…t was possible to execute cmd/bat scripts as executable.

     After this update this was suddenly broken.
     Our use-case for using such a script has to do with the consistency of our clang-tooling.
     We build clang-format, clang-tidy, clangd ... all together and put this in a package manager.
     In the script, we first download these executables (when needed) and than start the downloaded clangd.
     As such, when fixing an issue in clang-tidy, the same issue will get fixed in the clangd exe.
     Without this script, it is impossible to automatically trigger this download and it introduces the risk that we do not update all the tooling to the latest version.

Fixes clangd#683

In this reapply, we quote both the command line and the arguments, such that both can contain spaces.
We also introduce the option useScriptAsExecutable, such that this is only enabled when the user wants it.
We also provide the default value as boolean instead of string
JVApen added a commit to JVApen/vscode-clangd that referenced this issue Dec 16, 2024
…t was possible to execute cmd/bat scripts as executable.

     After this update this was suddenly broken.
     Our use-case for using such a script has to do with the consistency of our clang-tooling.
     We build clang-format, clang-tidy, clangd ... all together and put this in a package manager.
     In the script, we first download these executables (when needed) and than start the downloaded clangd.
     As such, when fixing an issue in clang-tidy, the same issue will get fixed in the clangd exe.
     Without this script, it is impossible to automatically trigger this download and it introduces the risk that we do not update all the tooling to the latest version.

Fixes clangd#683

In this reapply, we quote both the command line and the arguments, such that both can contain spaces.
We also introduce the option useScriptAsExecutable, such that this is only enabled when the user wants it.
We also provide the default value as boolean instead of string
HighCommander4 pushed a commit that referenced this issue Dec 22, 2024
In VSCode 1.92 and later (which uses node 20), this requires passing `shell: true` in the executable options.

However, using `shell: true` changes the behavior in some other ways, e.g. the path and arguments now have to be quoted in case they contain spaces.

To avoid the potential for regressions from using `shell: true`, its use is made conditional on a new clangd option, "clangd.useScriptAsExecutable".

Fixes #683
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants