Skip to content
This repository has been archived by the owner on Oct 18, 2018. It is now read-only.
This repository has been archived by the owner on Oct 18, 2018. It is now read-only.

Source Index and pack .pdb files into regular .nupkg #131

Closed
4 tasks
pranavkm opened this issue Nov 24, 2014 · 114 comments
Closed
4 tasks

Source Index and pack .pdb files into regular .nupkg #131

pranavkm opened this issue Nov 24, 2014 · 114 comments

Comments

@pranavkm
Copy link
Contributor

pranavkm commented Nov 24, 2014

Wherever we ship DLLs that are used in customer apps, we need to include the portable PDB along side it.

We need to include portable PDBs in:

  • The shared framework
  • The NUPKGs

And we need to do this in:

  • 2.1.0-preview1
  • dev

Original text:

From aspnet/dnx#380

The plan would be to create a single nupkg rather than a *.nupkg and *.symbols.nupkg as part of our build and use SourceLink to support having pdbs sit in the package.

@davidfowl
Copy link
Member

So I think the steps would be to take the output of the build and generate a source linked pdb using the pdb from the .symbols package. When we unzip and rezip the package to tweak extra stuff, we should include the pdb source linked pdb in there pointing to the github sha.

@AndriySvyryd
Copy link
Member

Note that including the pdbs in the .nupkg where VS can find them turns the corresponding dlls into “My Code”.

By default VS has Just My Code and break on user-unhandled exceptions enabled. This will cause the debugger to break on exceptions that cross "My Code" boundaries.

We found this out because we were including the symbols in the System.Data.SqlClient.nupkg, see dotnet/efcore#987

@ctaggart
Copy link

I just published a working version of SourceLink.exe. It should do what you need. Details:
ctaggart/SourceLink#53 (comment)

@ctaggart
Copy link

Hoping to provide better examples of using SourceLink.exe, I submitted an issue that shows how to index Sake.pdb and Sake.Engine.pdb sakeproject/sake#17.

Is there an API that I can access to get a list of files to compile in a project.json? If yes, I may be able to incorporate in SourceLink.exe. If not, use these two args instead:

        --pdb <string>: pdb file to add the index to, supports multiple and globs
        --file [-f] <string>: source file to put in the index, supports multiple and globs

For example --pdb the\path\to\the.pdb -f *\*.cs.

@ctaggart
Copy link

I just announced a stable SourceLink.exe and added documentation on how to use it.
http://blog.ctaggart.com/2015/03/announcing-sourcelinkexe.html
http://ctaggart.github.io/SourceLink/exe.html

Also be sure to check out the demo that demonstrates taking advantage of the source index at design time.
http://ctaggart.github.io/SourceLink/index.html#Demo-Go-To-Source-during-Design-Time

I would love to make progress on this ticket.

/cc @pranavkm @davidfowl

@muratg muratg added this to the backlog milestone Sep 8, 2015
@ctaggart
Copy link

ctaggart commented Nov 5, 2015

At the Microsoft MVP Summit, I spoke with several key individuals today and we've come up with a great plan for source indexing. The primary push right now is to get debugging to work cross-platform. The portable pdb format recently became 1.0 and Roslyn csc.exe can create them with a debug switch shipping with Visual Studio 2015 Update 1. The pdb's are much much more compact and cross platform (6 to 10 times). It would be great to included them in the nupkg files.

Open issues:

  • I am working with @tmat to get the source index text file into the portable pdb file via Roslyn csc.exe
  • the dnx build tool would use that Roslyn support, cc @anurse
  • @yishaigalatzer and the NuGet team is going to look at using satellite nupkg files like they did for native packages for CoreCLR
  • the Roslyn team will have a design discussion about identifying generated source files in a couple of weeks. (may be *.g.cs like Xamarin). The generated ones could be embedded in the pdb.

Simply including the portable pdb files with the nuget packages is a great first step.

@pranavkm
Copy link
Contributor Author

pranavkm commented Nov 6, 2015

cc @rynowak for symbro.

@ctaggart
Copy link

ctaggart commented Feb 27, 2017

We can change the title of this to source link instead of source index. I rewrote SourceLink as a dotnet tool for version 2. It supports Portable PDB files and its new source link support.

I've added pull requests that should get you guys started for both aspnet.mvc aspnet/Mvc#5860 and EntityFramework dotnet/efcore#7726.

It was been agreed upon to include Portable PDB files in the nupkgs for source link support. Check out the fresh new documentation here: https://github.com/ctaggart/SourceLink

cc @davidfowl

@ctaggart
Copy link

Just to get @DamianEdwards @Eilon @shanselman & others up to speed:

  • Visual Studio 2017 is shipping with "Enable source link support"
  • I worked weekends and evenings this last month to rewrite SourceLink for version 2 as a dotnet command line tool to support this.
  • We've agreed to ship the Portable PDB files in the nupkg files for source link support.
  • I've provided examples for adding it to both asp.net mvc & EntityFramework core.

It should be easy to add. It only needs to run on the build server. Happy to help whoever does the work. I'm also wanting to onboard the SourceLink project to the .NET Foundation & have submitted to do so.

@tmat
Copy link

tmat commented Feb 27, 2017

Let me also point out that VS 2017 supports Embedded Portable PDBs. This feature (/debug:embedded) instructs the compiler to embed a compressed Portable PDB into the dll/exe itself and the debugger finds it there. I'd recommend using that for release builds for all libraries where the extra size overhead doesn't matter. Source link feature is also available for Embedded PDBs.

@ctaggart
Copy link

/debug:embedded sounds like a great option that would make it even easier for projects to adopt source linking. I'll add support in SourceLink soon.

@Eilon
Copy link
Member

Eilon commented Feb 28, 2017

We don't currently ship PDBs anywhere at all - I think that's probably required for this suggestion to be useful, is that correct?

@ctaggart
Copy link

@Eilon That is correct. Embedding in the dll sounds like a good option. It is simple to test and to switch to.

The objections about shipping pdb files before were:

  • Windows PDB files are too big and require more network
  • Windows PDB files take up too much space on the client computer

With Portable PDB files and the new dotnet/nuget tooling, these issues have been addressed. The files are much smaller & the dotnet/nuget tooling shares the downloads in ~/.nuget. It should be simple to test this change to see how much bigger the nupkg files get. Simply switch to embedded PDB files and try it with <DebugType>embedded</DebugType>.

I think the increased file size is justified to enable source link support, but please discuss with the decision makers. @yishaigalatzer is who I've talked to before about this.

@analogrelay
Copy link
Contributor

Do we know the approximate increase in file size? Even if it's fairly significant, I'd probably still advocate for embedding PDBs because having to manage PDBs is JUST THE WORST. :)

I suppose I could just try it, but then I'd have to do work and mumble mumble I'm lazy mumble

@analogrelay
Copy link
Contributor

A simple test with Microsoft.AspNetCore.Authentication showed an increase from 62 KB to 73 KB for embedding the PDB. An increase of 17% in size. This seems completely reasonable to me given the benefits :).

@dougbu
Copy link
Member

dougbu commented Mar 1, 2017

Is @AndriySvyryd's comment above still correct?

including the pdbs in the .nupkg where VS can find them turns the corresponding dlls into “My Code”

If yes, how do we help users focus on what they're developing when that's their preference?

@tmat
Copy link

tmat commented Mar 1, 2017

@gregg-miskelly For opinion on JMC.

I wonder if it would make sense to distinguish Debug (My Code) vs Release (not My Code) bits.

@gregg-miskelly
Copy link

As long as dll/exe is optimized compiled, and 'Tools->Options->Debugging->Suppress JIT Optimizations on Module Loads' is in its default (unchecked) state -- no, since the code is JIT optimized it would still be considered non-user code.

@dougbu
Copy link
Member

dougbu commented Mar 1, 2017

dotnet/efcore#987 happened at the end of October 2014. I know we had a short (let's hope) period where we accidentally shipped non-optimized bits. Anyone remember if the time frames overlap?

If the System.Data.SqlClient bits were optimized, we need to figure out why VS treated them as My Code. Or @gregg-miskelly, are you describing a different behaviour than we would have seen way back then?

@gregg-miskelly
Copy link

@dougbu versions of VS before 2015 had Suppress JIT optimizations on by default.

@dougbu
Copy link
Member

dougbu commented Mar 1, 2017

Excellent 🥇

@Eilon
Copy link
Member

Eilon commented Mar 1, 2017

@anurse does your experiment just embed regular PDBs in the DLL, or does it include source links and/or embedded source?

@Eilon
Copy link
Member

Eilon commented Feb 5, 2018

O, change of plans, sorry for the confusion. We (ASP.NET team) discussed this further with the .NET team and decided to not include the PDBs in the NUPKGs or shared runtime, but to instead include them on a symbol server that will have everything needed for debugging. We're tracking that work item here: #850

Ultimately, our goal is to have a great debugging experience for everyone, and I think this new approach will achieve that. This is a very fluid space with a lot of ever-changing variables, but I think we're getting closer to a great solution.

@Eilon Eilon added wontfix and removed 2 - Working labels Feb 5, 2018
@Eilon Eilon closed this as completed Feb 5, 2018
@ctaggart
Copy link

ctaggart commented Feb 5, 2018

We are doing this as an experiment for 2.1.0-preview1 to see how well this works.

You didn't even do the experiment to see how well it works. You decided against it without giving reasons why. You chose a solution that does not work currently for the rest of the .NET community who can't publish to symbol servers.

@Eilon
Copy link
Member

Eilon commented Feb 5, 2018

Sorry about that, let me elaborate. We are working on getting symbol server support for additional debugging scenarios in the .NET Core 2.1 timeframe. So, by publishing PDBs to those servers, debuggers will be able to pull down the PDBs and you get a great debugger experience. The PDBs already have source linking enabled, so you can step straight into GitHub sources (wherever that specific feature is supported).

Can you clarify the remark about "the rest of the .NET community"? This issue applies only to the PDBs for ASP.NET/EF Core, unless I've misunderstood some requirements.

@ctaggart
Copy link

ctaggart commented Feb 5, 2018

So, by publishing PDBs to those servers, debuggers will be able to pull down the PDBs and you get a great debugger experience.

Symbol Servers 👎

My experience with Symbol Servers is usually not a great experience. What makes using Symbol Servers a better experience then including the pdb files in the nupkg? If you put the pdb files in the nupkg, you don't need Symbol Servers. It is one less system to deal with (host, configure, manage).

host

There are lots of options to host nuget feeds, including services like MyGet and AppVeyor in addition to NuGet Gallery. I am not aware of Symbol Server services.

configure

Using nuget clients like nuget.exe and paket you can specify a list of sources. I am not aware of any solution like that for symbol servers.

manage

To clean up disk space, it is pretty easy to delete nupkg files that are versioned. You can easily delete all of the prerelease versions before the last stable release. If you use Symbol Servers, how do you go about removing all of the corresponding pdb files?

@Eilon
Copy link
Member

Eilon commented Feb 5, 2018

Because this is just a question for the .NET/ASP.NET/EF Core packages, there's just one symbol server, the Microsoft one. At least when using VS, the symbols are cached in a folder of your choice, so it's easy to clear them all out (but perhaps not as easy to clear out just a subset):

image

One of the downsides of including the PDBs in the NUPKGs is the increased size and that most of the time they aren't used. Keep in mind that most DLLs in the .NET Core release are themselves doubled (one MSIL DLL, and one crossgen'ed DLL), so there are also two PDBs (one for each DLL).

@vancem
Copy link

vancem commented Feb 5, 2018

Note that we are not saying that people should not put PDBs into their main Nuget packages. Indeed we suggest that they do precisely because we do NOT have a story for 3rd parties to use a symbol server (we have plans, but they are not ready).

However this particular work item is of much narrower scope. It is whether ASP.NET (and certainly the .NET Framework) should package its PDBS into their nuget packages.

Now the ASP.NET and .NET Framework team are a bit different than most other in the .NET community because

  1. There is already a well established symbol server (namely http://msdl.microsoft.com/download/symbols) that can be used to host PDBS created by Microsoft, and this symbol server is already trusted (that is it would be considered a very serious security bug if hackers could exploit this server to violate our customers).

  2. We generate MUCH More of the code of a typical app. Between ASP.NET and the .NET Framework itself, typically 80% or more of the application comes from these Nuget packages. That makes any size impact greater. We know from internal experiments that the size WILL be a factor (we expect a 20% to 40$ increase in package size (probably toward the bigger end). This translates directly into download times, which are already known to be a pain point in some scenarios (again because apps contain so much of our code).

  3. While ASP.NET does not have native code, the .NET Framework does. Native PDBs are much larger, and thus the problem is bigger. Within Microsoft at least, a solution that was not different between native and managed code would be useful.

Note that other package managers (e.g. for Linux etc), have come to the same conclusion (that having separate packages for symbols is good), for undoubtedly the same reason (only dev scenarios need the symbols, but the are large, so why make everyone pay for something that few need (pay for play).

@ctaggart
Copy link

ctaggart commented Feb 5, 2018

Note that we are not saying that people should not put PDBs into their main Nuget packages. Indeed we suggest that they do precisely because we do NOT have a story for 3rd parties to use a symbol server (we have plans, but they are not ready).

Can we agree that the .NET community should put source linked PDBs into their main NuGet packages until there is a better solution? I think you are already saying this, but I just want to confirm. I think with these projects finally enabling source link, it will be a major win and other major .NET projects will finally adopt it too. These Microsoft projects lead by example. With your clear explanation of why Microsoft is different and is using a symbol server, that should be good enough to help spur adoption.

@clairernovotny
Copy link

@ctaggart I think that's the direction NuGet is going, either there or with the snupkg files. The snupkg files just being a delivery mechanism to the NuGet symbol server. I think they were looking at also indexing pdb's in the main package so either would work.

@vancem
Copy link

vancem commented Feb 6, 2018

I agree with @ctaggart point that people besides Microsoft should be

  1. Enabling their build to use SourceLink (assuming their source code is hosted on the web in some way (e.g. GitHub). Most Microsoft code is SourceLinked now and if it is not it is a bug.
  2. They should include their (portable) PDBs in their nuget packages. Microsoft does not do this but only because of special Microsoft circumstances (given in my previous comment).

We strongly encourage people to take these steps. Ultimately we want to have a secure symbol server story that everyone can use, but that that is far enough away that people should NOT wait. They should be including PDBs now.

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

No branches or pull requests