Switch Autofac build to use .snupkg instead of embedding the .pdb directly

[jeanluc33]

Autofac version affected: 4.9.x

Description:
The .net 4.5 folder of the nuget package appears to erroneously have the portable PDB, not the full pdb. The result is a build error on legacy TFS servers.

Please consider that Json.net / Newtonsoft had a similar issue when they went to portable PDBs, and they reverted to the full PDB for the .Net 4.5 dll.

It would be great if the full debug PDB could be used again for the .Net 4.5+ targeting dll, as that would relieve this build error for 4.9.x and higher.

(Obviously the portable one is fine for the .Net Standard 1.1/2.0 versions).

[tillig]

Happy to take a PR on this. We're using whatever the dotnet tooling is generating, which means it'll be fudging around a bit with the .csproj, then doing the dotnet pack steps and opening up the package manually to ensure things got in the right spots.

For other folks running into this, can you clarify what "legacy TFS" is? Which version are you on such that this is an issue?

[jeanluc33]

In my case, legacy TFS is on premises TFS, e.g. TFS 2012. Likely affects other similar versions as well (e.g. TFS 2013 etc.)

[tillig]

Sounds good. I will say on TFS 2017 and 2019 there are no errors of this sort, so at least we know what the limitations are. Thanks!

[nevadawilliford]

I'm seeing this issue as well using a new version of TFS on prem.

About Microsoft Visual Studio Team Foundation Server
Microsoft Visual Studio Team Foundation Server
Version 15.117.27024.0

Based on this blog post that's Team Foundation Server 2017 Update 3.

The Publish symbols task fails with this logged...

2019-08-28T12:36:57.8959036Z ##[error]Indexed source information could not be retrieved from 'D:\Builds\Agent\_work\5\s\MyProject\obj\Dev\Package\PackageTmp\bin\Autofac.pdb'. Symbol indexes could not be retrieved.

[nevadawilliford]

I submitted pull request #1032 to address this. I don't know very much about comparing the guts of PDB files but the size of the Autofac.pdb file in the bin/Debug/net45 folder increased from 406kb to 1616kb.

Also ran dotnet pack before and after to generate .nupkg files. Compared those files using NuGet Package Explorer to ensure the file was still in the correct location and there were no obvious problems.

[tillig]

Merged, and thanks! This will be included in Autofac 5.0.0 when it's released.

[tillig]

Based on autofac/Autofac.Extensions.DependencyInjection#58 it appears this doesn't actually fix it all the way. It fixes it for net45 but not the other targets.

It appears we need to set debug to full for all configurations, not just net45.

[tillig]

Hang on, we need to do some more investigation here.

DebugType: full appears to have some impact on the JIT optimized code quality and it's recommended specifically that it's not used for release quality code.

PDB is included here to allow SourceLink to work.

Before we add this, let's see if there's some way we can follow up with the SourceLink team to see if they know what's going on here. I'm not against trying to come up with a fix, but I also want to be sure we're not adding a perf hit to everyone just for this situation.

I've closed PR autofac/Autofac.Extensions.DependencyInjection#59 so we can take a step back and verify things are right. I've also moved the discussion on autofac/Autofac.Extensions.DependencyInjection#58 to this issue so we can talk about it in one spot.

This affects every package in which we've enabled SourceLink, not just the two noted packages. It's bigger than just these two issues, which is why it's interesting to figure out what the real solution is (should be?) and solve it the right way.

For example, looking at the readme over in the SourceLink repo I see:

[start]

---

Prior availability of NuGet.org symbol server the recommendation used to be to include the PDB in the main NuGet package by setting the following property in your project:

  <PropertyGroup>
    <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
  </PropertyGroup>  

Including PDBs in the .nupkg is generally no longer recommended as it increases the size of the package and thus restore time for projects that consume your package, regardless of whether the user needs to debug through the source code of your library or not. That said, .snupkg symbol packages have some limitations:

• They do not currently support Windows PDBs (generated by VC++, or for managed projects that set build property DebugType to full)
• They require the library to be built by newer C#/VB compiler (Visual Studio 2017 Update 9).
• The consumer of the package also needs Visual Studio 2017 Update 9 debugger.
• Not supported by Azure DevOps Artifacts service.

Consider including PDBs in the main package only if it is not possible to use .snupkg for the above reasons.
For managed projects, consider switching to Portable PDBs by setting DebugType property to portable. This is the default for .NET SDK projects, but not classic .NET projects.

---

[end]

So there's a new way to distribute PDB that doesn't involve putting them in the NuGet package. That's a change since we implemented SourceLink.

Can someone take a bit of a dive into this and see what it'll take to change how we deal with SourceLink? This may be how the Microsoft packages do things - it'd mean we don't have the PDB in the NuGet package at all and would solve the problem whilst still enabling SourceLink and not sacrificing performance.

[tillig]

I'm going to update the issue title here to be more about what needs to happen so it might get more traction. Basically, we need to get .snupkg into the builds and stop embedding the .pdb directly. I don't think that'll be much work, but it's not obvious from the title and might not be getting much traction because of that.