-
Notifications
You must be signed in to change notification settings - Fork 520
Add attachDotnetDebugger debug option #3903
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
Conversation
This PR is still active, and I likely will try to fix it before PSConfEU as it will dovetail well into our binary module presentation.
2dd742a to
613f208
Compare
@andschwa picking this back up again. Would you prefer me to add the functionality in PR increments since it requires an explicit debug option to activate for easier review, or do you want this to be a big ol' PR with all batteries included?
Uhh just how big do you think one big PR will be? Certainly less overhead to do it in one go.
Uhh just how big do you think one big PR will be? Certainly less overhead to do it in one go.
Not super huge, see the WIP items. Maybe a couple hundred lines at most.
961fcb4 to
ccc2783
Compare
@andschwa I may need some help with getting the tests up and running, I can't for the life of me get it to work either in codespaces or locally.
EDIT: Not sure what I did but I completely blew away all of node_modules, etc. and did an npm ci and things seem to be OK again.
@JustinGrote ping me when you want a review! I should be on Discord too.
@andschwa implementation code is good for review, we'll just need to dream up some tests.
Here is an extension build for anyone wanting to play with it. You will need to rename .zip to .vsix and then use "Install Extension from VSIX" in the VSCode command palette.
powershell-2023年3月2日-binaryModuleDebug.zip
Or specify the extension on commandline: code --install-extension (Resolve-Path "~\Downloads\powershell-2023年3月2日-binaryModuleDebug.vsix")
f5cd8aa to
0fcf727
Compare
Rebased to latest main
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.
Looks really good, just some changes!
@andschwa this is ready for review. End to End tests for the binary module debugging were added and it tests both that it starts correctly and that a C# breakpoint is hit in a hybrid module.
Extract processId Fetch Cleanup legacy bad rebase Revert breakout config Add dotnetDebuggerConfigName info TODO the workspace folder support Fix typo Cleanup flow and breakout temporary console to dedicated functions Add handlers to cleanup dotnet attach Fix linting issues Refactor one-time event disposal using variable scope demonry Clarify the handlers Remove Disposable (lint fix) Add initial test Lint fix Fix description to use correct and more specific terminology. Co-authored-by: Andy Jordan <2226434+andschwa@users.noreply.github.com> Apply suggestions from @andschwa review Fixed E2E Debug Testing Remove Only Add initial Debug test scaffolding Remove "only" Move validation logic to resolve functions and implement first resolve test Rearrange methods with public first and implementation details last, in order of implemented interfaces. Add more tests Fixed positioning of resolvesubstitutedvariables, now the public methods happen in the order they are called Populate more tests Some more scaffoldng and a broken test Add optional attach port to runTests and remove only *again* Fix wrong argv for port Add check for PSES Binaries Adjust StubInterface to use Partial to preserve intellisense More test fixes Fix test names to be consistent with other tests More specific error checks and implement structuredClone Add structuredclone to dev dependencies Revert accidental commit of new build tasks Update package.json Co-authored-by: Andy Jordan <2226434+andschwa@users.noreply.github.com> Enable ESModuleInterop and fix related issues Updated structuredclone import to ES6 syntax Add ESModuleInterop to tsconfig.json More test adds and add newlines between Arrange Act Assert More test scaffolding ESModuleInterop fixes Bump package-lock More esModuleInterop fixes Tests TESTS TESTS TESTS TESTS Flaky Test Check Move testing default folder to examples Add binary module example and C# extension fetching Add precompiled binarymodule so that it doesnt have to be built for testing Fix pid for ProcessID Add first binary module E2E test Suppress first run messages in examples folder Remove Only Add createDebugAdapterDescriptor test Remove Only Move working dir back to test (was breaking settings tests) and add dummy csproj for C# activation Move dummy to mocks folder Reload window for CI Fix assertion order in binary module test Skip Mac on binary module script test for now Fix bug where stdout is null if CLI install fails Extract CSharp extension install to separate function for DAMP clarity Add test if C# extension was found Remove imports More MacOS troubleshooting (7 minute cycles between tests is hell) More troubleshooting lines Add hack to change cwd to make resolveCliArgs happy Found out the electron extensions run in a whole separate process on MacOS, handle that Re-enable resolveCliArgs detection problem fix Remove error reference Try without window reload Remove Async from InstallCSharpExtension Remove more async Wait for C# extension to load rather than just reloading the window More binary module test scaffolding End to End Tests Done! 🎉🎉🎉 Spoke too soon, find out why CI breakpoint test fails (probably because it might be headless) Add undefined checks Build test binary module locally to avoid absolute PDB path issues MacOS why you gotta be such a PIA Remove dummy csproj, maybe MacOS search was a flake Try reload window on mac Move MacOS C# install to before tests to avoid test duplications Fix reload loop on mac Remove reload from initial Add some debugging to try to find macos issue Move event registration to before startDebugging Try increasing timeout for MacOS Add WaitEvent helper function Try MacOS Hot Install Again Retry: RunCode test seems to be flaky Change exec to spawn to make CodeQL happy Revert "Change exec to spawn to make CodeQL happy" This reverts commit d3aea33.
0ce15de to
15f65cf
Compare
3f16de2 to
43da008
Compare
Co-authored-by: Andy Jordan <2226434+andschwa@users.noreply.github.com>
Doing the "hot install" of the extension seems to be flaky and locking up the extension host on 2022 PS7 for some reason on occasion, I may just move the C# initialization to the beginning prior to starting the tests in the first place.
image
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.
👏
Fixes dotnetDebuggerConfigName-option in launch config when using new attachDotnetDebugger feature introduced in #3903. Always threw config not found-error when dotnetDebuggerConfigName was set. Co-authored-by: Justin Grote <JustinGrote@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Latest VSIX for dotnet debugger
PR Summary
Adds an option to attach the omnisharp C# debugger for binary module projects, enabling mixed debugging for Powershell Binary Modules.
Demo.mp4
The attach task runs as a child task to the PowerShell debugging session and is managed via its lifecycle.
image
Related Changes
As part of this PR I also refactored/rearranged the debug config resolution/validation/mutation steps, to occur at the proper stages of functions that were called. I added tests for what I changed.
WIP Checklist
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready