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

Reference new debugger bits #58051

Conversation

davidwengier
Copy link
Contributor

@davidwengier davidwengier commented Dec 1, 2021

Part of #55834
Follow up to #57978 which implements the actual service and calls the actual debugger.

For now this includes the merge to -vs-deps unfortunately, and there is only one commit to review: 00920ea (#58051)

Unfortunately with infrastructure issues, this might not be a clean PR for a little while, so sorry for doing this again, but I would like to get in front of any issues.

@davidwengier
Copy link
Contributor Author

davidwengier commented Dec 1, 2021

Waiting for #58062 to merge, then this will be shorter

@davidwengier davidwengier marked this pull request as ready for review December 1, 2021 23:42
@davidwengier davidwengier requested review from a team as code owners December 1, 2021 23:42
@davidwengier
Copy link
Contributor Author

Okay this has been rebased and is now a sensible PR @jasonmalinowski @ryzngard

@jinujoseph this is the last part of the 17.1 P2 work to enable source link

var (project, symbol) = await CompileAndFindSymbolAsync(path, Location.OnDisk, Location.OnDisk, metadataSource, c => c.GetMember("C.E"));
// Ideally we don't want to pass in true for windowsPdb here, and this is supposed to test that the service ignores non-portable PDBs when the debugger
// tells us they're not portable, but the debugger has a bug at the moment.
var (project, symbol) = await CompileAndFindSymbolAsync(path, Location.OnDisk, Location.OnDisk, metadataSource, c => c.GetMember("C.E"), windowsPdb: true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a tracking bug for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, waiting to hear back from the debugger team. I might just be interpreting the API incorrectly.

Copy link
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signing off in that this looks like a good step forward and most places are clearly marked with TODO.

@davidwengier davidwengier merged commit 9af4b1e into dotnet:release/dev17.1-vs-deps Dec 2, 2021
@davidwengier davidwengier deleted the ReferenceNewDebuggerBits branch December 2, 2021 21:43
allisonchou added a commit to allisonchou/roslyn that referenced this pull request Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants