Remove workaround requiring full pdb's

[clairernovotny]

Verify that dotnet/sdk#955 is fixed in the latest VS and revert the generating the full pdb's

[windowstoolkitbot]

No response from the community. ping @nmetulev @Odonno @IbraheemOsama

[windowstoolkitbot]

This issue seems inactive. Do you need help to complete this issue?

[wstaelens]

I cannot debug .net standard 2.0 application. Not when set to full, or pdb-only. breakpoints are not hit in VS2017 15.9.3

[azchohfi]

@onovotny I'm working on getting snupkg working. Should we be fine changing them all to portable pdbs (that's the only version nuget symbol server supports)?

[clairernovotny]

Is there a reason we can't use embedded pdb's? There's a healthy debate here but there are downsides to snupkg too, namely perf and the need to setup a symbol server. Also, more specific to UWP, the .NET Native tools need the pdbs available to munge during the process, so symbol servers aren't useful there either.

[clairernovotny]

In my opinion, embedded pdb is the easiest since there's no separate pdb artifact to deal with, version, download, or lose.

[azchohfi]

@onovotny Adding the symbol server is one more step, I agree, but do we want people to download the PDBs, even if they don't need them? It does have an impact on the packages size.

Going back to portable PDBs reduces this impact considerably. Just to close this issue, I'm going to send a PR that changes the PDB to portable ones. I've also verified that this also solves the issue of VS not loading the PDB by default, which is another big improvement.

[clairernovotny]

using portable pdb is a reasonable middle ground, if they're in the main nuget. Still no real need for a seperate snupkg IMO.

For mobile size, shouldn't that be the job of a linker to remove extra stuff, including these?

[azchohfi]

I was wrong. VS didn't load the symbols for me, even if they were portable. I've started a side conversation with the VS team to try to solve this. VS is not loading symbols for NuGet dependencies on UWP projects.
I'm also investigating the impact of not having the symbols available for developers when we push this to the store.

Now, back to this issue, I set it back to pdbonly and the PDBs got included. Should we go back to the defaults? pdbonly on Release and full on debug? @onovotny I'm not too familiar with MSBuild.Sdk.Extras. If we just remove that, it will default to something else (other than pdbonly for Debug and Full for Release), right?

[azchohfi]

I think portable PDBs are the best answer here, if there is no extra benefit to snupkg. It'll shrink the PDBs considerably. Is there anything that we need/use that will be lost (for reference: https://github.com/dotnet/core/blob/master/Documentation/diagnostics/portable_pdb.md)?

[clairernovotny]

Portable PDB's are a full replacement, so we won't lose anything.

[michael-hawker]

@azchohfi @onovotny do we have what we need now to fix/close this?

[clairernovotny]

According to the source, it's still generating full pdb's...unless I'm missing something?

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/master/Directory.Build.targets#L8

[azchohfi]

@clairernovotny You are right. We need this fixed. Did anything change?
@michael-hawker any chance we can get this to 7.0?

[clairernovotny]

What are you looking to see was changed?

I still recommend portable PDB's in the main nuget package. Also, while you're at it, ensure that deterministic builds are produced: https://github.com/clairernovotny/DeterministicBuilds

[azchohfi]

I just validated again and VS beautifully loaded the portable pdbs. I'll send the PR fixing this. We can discuss later if snupkg is the way to go or not (#2684).