-
Notifications
You must be signed in to change notification settings - Fork 645
Conversation
Processes were not being killed due to the path to the external scripts being incorrect. Fix this by relying on the tree-kill package rather than shelling out to an external script to kill a process and its children. Also add sinon as a dev dependency for testing purposes. Updates #3044
That sounds good. Feel free to close this pr. |
src/debugAdapter/goDebug.ts
Outdated
@@ -36,6 +36,7 @@ import { | |||
getInferredGopath, | |||
parseEnvFile | |||
} from '../goPath'; | |||
import { killTree } from '../util'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot have the debug adapter take a dependency on util which depends on the vscode
module. This is because the debug adapter is run in a process different from the rest of the extension. The process in which the rest of the extension runs has access to vscode
, but the process running the debug adapter doesnt. That's why previously, the kill code is copied here in this file instead of re-using from util
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can however move the killTree out of the util into a separate file and then re-use it from there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, got it.
We cannot take a dependency on util, which depends on the vscode module because the debug adapter runs in a process that doesn't have access to the vscode module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Zac!
Processes were not being killed due to the path to the external scripts being incorrect. Fix this by relying on the tree-kill package rather than shelling out to an external script to kill a process and its children.
Also add sinon as a dev dependency for testing purposes.
Updates #3044