Spawn Flutter debug adapter directly with dart.exe #5204
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VS Code v1.92 will ship with a breaking change (from a new version of NodeJS) that prevents spawning `.bat` files without using `shell: true`. Although Dart-Code uses `shell: true`, the debug adapter is spawned by VS Code and we don't currently have the ability to ask it to use a shell, which means launching the Flutter debugger on Windows is broken. I've filed microsoft/vscode#224184 and open a PR at microsoft/vscode#224204 but this will not be included in the 1.92 release, so to keep debugging working on Windows, we need a workaround. This change switches from asking VS Code to run `flutter.bat debug_adapter` to instead running `dart.exe` and using the same args that Flutter would use. This unfortunately bypasses some code (like ensuring everything is up-to-date), however since we'll have already started things like `flutter daemon` at the start of the session, it's very unlikely everything won't already be set up. Once a better fix is available in VS Code, we should use that and change the scope of the `isUnableToRunBatDebugAdapters` VS Code capability to _only_ the version(s) that require this workaround, because it makes assumptions about flags passed to `dart` that may or may not remain accurate across Flutter SDK versions. Fixes #5183
Member
Author
|
Thanks! As it turns out, a fix did get merged into the VS Code release candidate and I've just verified it without this workaround locally. This is the best outcome, because it doesn't rely on people updating to the latest extension release to not be broken. So, I will revert this workaround and push an updated pre-release today, then do the stable release tomorrow. I'm also going to work on changing the CI runs a bit so that the results from the dev versions of VS Code (and Dart) are more visible (they're not run on every commit because running all combinations of the bots takes a very long time). |
Contributor
|
Good news 👍 Thanks for the update! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a fix (workaround) for a potentially serious issue debugging Flutter on Windows with the incoming VS Code (editor) release. I plan to ship it a Dart-Code pre-release shortly (today) to get some testing and may ship in a stable release tomorrow or Thursday.
@andrewkolos @jwren @helin24 I would appreciate additional eyes over this change before the stable release if possible. It's not a particularly nice workaround but it's currently all I have.
A summary of the issue:
A recent breaking change in NodeJS means spawning a
.batshell script on Windows without explicitly settingshell: truewill fail with anEINVALerror. The upcoming release of the VS Code editor (being prepared and likely to ship this or next week) updates to a version of NodeJS that includes that change. Our extension code has always usedshell: truewhere necessary, however the Debug Adapter (flutter debug_adapter) is not started directly by us in the extension - instead, we pass aexecutable: string, args: string[]to the VS Code editor and it spawns the process. It does not provide the ability for us to tell it to useshell: true. I have filed microsoft/vscode#224184 and (open a PR at microsoft/vscode#224204 with a fix) however because it's close to release and my proposed fix was an API change, it will not be merged before the stable VS Code release.If we do nothing, Windows users will be unable to launch Flutter debug sessions with the upcoming VS Code release (and this is already the case for those using Insider builds of VS Code).
As a workaround, when using the affected version of VS Code (on Windows), I am substituting
"sdk-path\bin\flutter.bat debug_adapter"with the equivalent command that the shell script runs"(sdk-path)\bin\cache\dart-sdk\bin\dart --packages="..." (tool-snapshot-path) debug_adapter. This works for me locally in Insiders, and is passing all of the (previously failing) CI tests run against insiders.As I was writing this, VS Code devs agreed to look at another fix (microsoft/vscode#224184 (comment)) so we might not need this - however I'm going to continue with putting this into a pre-release extension for additional testing so if they are unable to ship a fix, we have this to fall back on (and it isn't completely untested).
(I'm merging it so it can go into the pre-release so this PR will be closed, but if you have any feedback or concerns, please still leave them here and I will address).
Fixes #5183