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

Embed PDBs in assemblies so source-stepping works #1751

Closed
thomaslevesque opened this issue Jan 22, 2020 · 4 comments · Fixed by #1752
Closed

Embed PDBs in assemblies so source-stepping works #1751

thomaslevesque opened this issue Jan 22, 2020 · 4 comments · Fixed by #1752
Milestone

Comments

@thomaslevesque
Copy link
Member

Currently, source-stepping with SourceLink doesn't work in some scenarios, due to differences in the SDK behavior. For instance, the .NET Core 3 SDK copies the assemblies from referenced packages to the output folder, but not the PDB (see dotnet/sdk#1458). So the debugger doesn't find the PDB, so it can't find the source.

A simple workaround would be to use the embedded PDB format, which is basically the same as the portable format we're currently using, except that it's embedded in the DLL.

Benefits

  • The PDB will always be available, it will no longer depends on whether the SDK copied it or not, leading to a better debugging experience for users.
  • Packaging will become simpler, since we don't have to worry about packaging the PDB

Downsides

  • The assemblies will be larger.

I don't think the increased assembly size is a major issue ; the PDB is already downloaded with the package anyway. And size itself doesn't really matter, since FakeItEasy is mostly a dev dependency (not shipped with apps). Anyway, the PDB files aren't that big now (about 80KB, when the assembly is around 250KB)

@mikebeaton
Copy link

mikebeaton commented Jan 23, 2020

If on a public repo .snupkg is better, and doesn't suffer from the PDB copying problem. It's better because with .snupkg the PDB isn't downloaded at all unless needed, so you're not giving the download overhead to many people who won't want or need it. For private or small audience size repos however, the settings you suggest are fine and much easier to use.

Also note that embedding the PDB and embedding the sources in the PDB are two different things, which don't both have to be done to get sensible output (the alternative to embedding the sources in the PDB is SourceLink).

@thomaslevesque
Copy link
Member Author

Hi @mikebeaton,

Thanks for your input!

If on a public repo .snupkg is better, and doesn't suffer from the PDB copying problem.

We considered it, but we're not sure how well it works outside of Visual Studio. Do other tools support it? VS Code, Rider? Do they know how to download the snupkgs?

It's better because with .snupkg the PDB isn't downloaded at all unless needed, so you're not giving the download overhead to many people who won't want or need it.

True, but:

  • We already have that overhead, as we currently have the PDBs in the same package as the assemblies. Nobody complained so far.
  • As I mentioned above, the PDBs are pretty small (80KB). This is pretty small compared to the XML documentation files, which are about 360KB each. So the overhead isn't much of an issue.

Also note that embedding the PDB and embedding the sources in the PDB are two different things, which don't both have to be done to get sensible output (the alternative to embedding the sources in the PDB is SourceLink).

We're using SourceLink already. We have no intention of embedding the source code 😉

@mikebeaton
Copy link

mikebeaton commented Jan 23, 2020 via email

@blairconrad blairconrad changed the title Embed PDBs in assemblies Embed PDBs in assemblies so source-stepping works Jan 23, 2020
@blairconrad blairconrad added this to the vNext milestone Jan 23, 2020
@afakebot
Copy link

This change has been released as part of FakeItEasy 6.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants