Home: msbuild /t:Pack always creates seperate symbols package

[clairernovotny]

When using the SDK builds to create a package, if you have <IncludeSymbols> set to true, then NuGet always creates a separate .symbols.nupkg. This needs to be configurable. Sometimes we want to put our symbols in the main NuPkg as it makes it far easier to have source debugging "just work" without extra symbol server configuration.

Even if the current behavior is the default, there needs to be an option not to create a separate .symbols.nupkg and just leave the symbols in the main package.

[rohit21agrawal]

@rrelyea : this seems like a feature request, do we want it for rc3 ?

To the best of my knowledge , dotnet pack never had this ability

[rrelyea]

We'll consider this work in the future. Yes, we shouldn't take time before RTM to land this one way or the other.

[clairernovotny]

What needs to happen to enable this? A property and then something in the build task? If you can point me in the right direction, I might be able to do this sooner.

Sent from my Windows 10 phone

From: Rob [e-mail]
Sent: Thursday, December 29, 2016 7:20 PM
To: NuGet/[e-mail]
Cc: Oren [e-mail]; [e-mail]
Subject: Re: [NuGet/Home] msbuild /t:Pack always creates seperate symbols package (#4142)

We'll consider this work in the future. Yes, we shouldn't take time before RTM to land this one way or the other.

-
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com/NuGet/Home/issues/4142#issuecomment-269714160, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABXHVO6iu-VQGFOedJFYH-5oHHVKCEWGks5rNE46gaJpZM4LTbuR.

[rohit21agrawal]

@onovotny as a workaround , can't you just add a custom target that runs after pack, deletes the .nupkg package and renames the .symbols.nupkg package to .nupkg ? I can help you write the target if you think this will unblock you.

[clairernovotny]

I could probably do that, but what would it take to do out the right way if I can tackle it?

Sent from my Windows 10 phone

From: Rohit [e-mail]
Sent: Thursday, December 29, 2016 8:02 PM
To: NuGet/[e-mail]
Cc: Oren [e-mail]; [e-mail]
Subject: Re: [NuGet/Home] msbuild /t:Pack always creates seperate symbols package (#4142)

@onovotnyhttps://github.com/onovotny as a workaround , can't you just add a custom target that runs after pack, deletes the .nupkg package and renames the .symbols.nupkg package to .nupkg ? I can help you write the target if you think this will unblock you.

-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/NuGet/Home/issues/4142#issuecomment-269717538, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABXHVOyPLHnOi_jc_pEo3WqDc9UD6dt1ks5rNFgQgaJpZM4LTbuR.

[rohit21agrawal]

we'll have to design it appropriately, but thinking off the top of my head - we would have a msbuild property like $(CreateSeparateSymbolsPackage) which would default to true in Pack.targets . We will then use this property in MSBuildProjectFactory.cs (passed to it via PackTargetArgs) and write the logic to determine the extension of the symbols package and stop it from writing out the non-symbols package to disk.

One thing to consider here is that having too many properties/command-line switches is not always a good thing, especially for scenarios which are not blocking.

[clairernovotny]

I think this ties into https://github.com/NuGet/Home/issues/1696 and https://github.com/dotnet/roslyn/issues/3. By default, especially with PortablePdb's being much smaller, symbol files should not always be going to their own packages by default.

I think there's a clear difference between the flag to include symbols in the package at all, IncludeSymbols and whether it should be in a separate file or not.

In addition, FWIW @yishaigalatzer said here https://github.com/NuGet/Home/issues/1696#issuecomment-260168615 that including symbols would be on by default in the new Pack targets. I didn't take that to mean it'd be a separate file as that's mostly useless with today's tooling. Having it in the main NuPkg is useful because then the debugger will easily find and load it.

[rohit21agrawal]

@yishaigalatzer, @rrelyea I wasn't aware of the plan to include symbols by default in the nupkg . Did i miss anything?

[ctaggart]

@rohit21agrawal, yes, that plan that I hashed out with @tmat & @yishaigalatzer at the MS MVP conference in 2015-11 is finally coming together. I added some comments in https://github.com/NuGet/Home/issues/1696#issuecomment-282535229, but to expand on that, here is what Visual Studio 2017 now offers:

source server support

This was done with source indexing large Windows PDB files. There was valid concern for including them in nupkg files, especially when the nuget client didn't share them. Source indexing was done by modifying the pdb file after the compile.

source link support

The new source link support is done with Portable PDB files which are cross platform and many times more compact. The source link json is built before the the compile and the compiler embeds it in the pdb.

Essentially, things have changed. Including Portable PDB files in order to get source link support is important and doable now.

[ctaggart]

I think the new dotnet pack option should be --include-portable-pdbs to clearly indicate that this is for Portable PDB files and the new source link support.

[ctaggart]

@onovotny The solution that we are going forward with to enable source link is to embed the portable pdb files in the .NET Core dll files. That achieves getting them in the nupkg without people having to change anything else about their build process. https://github.com/ctaggart/SourceLink#dotnet-sourcelink

[clairernovotny]

@ctaggart that's a really, really, really bad idea....sorry. It's bad for mobile devices where the physical image size and memory is tight. We don't want pdb's to be loaded into memory because they're embedded in the dll. This is a complete non-starter.

It's not just mobile, but other constrained devices like IoT and embedded (think Tizen).

[ctaggart]

Ugh. @tmat help

[tmat]

@onovotny It's not a bad idea. It might not be feasible for all libraries (e.g. as you mention those potentially deployed on small devices), but many customers are fine with 10-20% dll size increase. Note that the PDB data pages are actually not committed into memory until the PDB data is accessed.

We are currently also figuring out whether the debugger could fetch Portable PDBs from packages published on NuGet.org. Stay tuned.

[tmat]

In any case I'd recommend either embed PDBs into dll/exe, if size is not an issue, or include it in a separate file in the nuget package next to the dll/exe.

[clairernovotny]

@tmat my concern, and why I called it out as "bad", is that the creators of the libraries are often not the end-users and are not likely going to be thinking about where their libraries may be used.

So people will think they're doing the right thing, being helpful, providing symbols, etc, but then adversely affecting their users by generating larger libraries.

We are currently also figuring out whether the debugger could fetch Portable PDBs from packages published on NuGet.org. Stay tuned.

That would be a much nicer idea, IMO.

In any case I'd recommend either embed PDBs into dll/exe, if size is not an issue, or include it in a separate file in the nuget package next to the dll/exe.

Key issue is that as mentioned, the producer of the library cannot know if size is an issue for the end user. And for the end user, the choice would have been prematurely made for them. I'd venture to say that in this case, size of dll relates to perf (loading, memory, size) and thus should strive to be as small as possible under all circumstances.

[tmat]

@onovotny If you find such a library that you absolutely need to be smaller you could let the producer know to change the settings and publish PDB separately. In case the author doesn't respond we can provide a tool that removes the PDB data from the dll. And you could then publish such modified library + pdb in your own feed. The dll would still link to the PDB so the debugger would be able to find it.

I'm still not convinced that 10-20% extra size on disk is a deal breaker. The load time should not be impacted since the loader creates a memory mapping for the file. It doesn't read each byte of the dll.

[tmat]

How about changing the default behavior of <IncludeSymbols> to be:

• if building Portable PDBs include them into .nupkg, do not create .symbols.nupkg
• if building Windows PDBs create .symbols.nupkg, do not include them in the main nupkg

In other words symbol packages are for legacy PDBs. Portable PDBs are small and go directly to the main package where nuget server can index them.

Also, the value of <IncludeSymbols> could be true by default for Portable PDBs.

[ctaggart]

Here is a working example of adding the pdb using the current tooling that I did today for https://github.com/fsprojects/Paket/pull/2313:

  <ItemGroup Label="dotnet pack instructions">
    <Content Include="$(OutputPath)Paket.Core.pdb">
      <Pack>true</Pack>
      <PackagePath>lib/netstandard1.6</PackagePath>
    </Content>
  </ItemGroup>

This is based on the dotnet extensibility article. This works, but it should be easier.

[kzu]

If you install the NuGetizer package (install-package nuget.build.packaging) you should get this for free as a drop-in replacement. (NOTE: this should work for VS2017 15.2+)

I just added support for $(IncludeSymbols) for compatibility (right now you need to set the more explicit $(IncludeSymbolsInPackage))

[rohit21agrawal]

@onovotny considering adding a project level property here called CreateSymbolsPackageOnPacking which will default to true in the targets. If set to false, it will only create one package:

1) If only IncludeSymbols is specified, the only created package will include everything that the normal nupkg would have + the pdb files.
2) If IncludeSource is specified, the only created package will include everything that the normal nupkg would have + the pdb files + the source files.

[tmat]

@rohit21agrawal I don't think we should include source files in a "normal" package. Instead embed the sources into the PDBs that are included in the package via /embed compiler option. The benefit is that these sources are compressed in the PDB, so the local nuget cache won't get bloated. Also the debugger will be able to find them in the PDB (it wouldn't if they were in the nuget cache).
/cc @RichLander

[rohit21agrawal]

@tmat we are not including source files in the nupkg by default. we are not even including symbols in the nupkg by default.
i am aiming to provide a switch that will enable the package author to have full control over where to have their sources and pdbs.

[rohit21agrawal]

as far as embedding the sources into the pdb is concerned, that is not something that NuGet is aiming to do in the future.

[tmat]

@rohit21agrawal I agree embedding sources shouldn't be responsibility of NuGet pack. I was thinking in more general terms - the SDK should do that (we are currently working out details).

Before adding any more properties/options into NuGet pack we need to consider the end-to-end scenario. The debugger won't be able to find the sources if they are in src directory in a package. If the debugger isn't able to find the sources then what is the option good for? Do you have another scenario on mind?

[rohit21agrawal]

i get your point, and i agree that the source discovery within a package is broken - it's broken even for symbols.nupkg right now. i am trying to keep the option open for the future if this scenario ever gets fixed (or debuggers and pdbs become smarter and are able to figure out sources relatively etc etc..)

I don't really see any harm in providing an option to gain complete control over which package the sources/symbols should go to, but i am open to listening to more feedback :)

[tmat]

@rohit21agrawal Keeping the possibility of introducing the option is definitely good. No disagreement on that. All I'm asking for is not to actually introduce it before we finalize the design of the end-to-end scenario (the "smarter debugger" part). Our current thinking is that with sources embedded into PDBs we already have an efficient mechanism how to deliver sources in a package in a way that the debugger can find them without adding extra smarts and features.

[rohit21agrawal]

@rrelyea any thoughts on this?

@tmat Do you have a timeline in mind about when you can finalize the end-to-end scenario? I don't want to keep everyone waiting for too long.

[tmat]

@rohit21agrawal

Re including PDBs: I don't think we need to delay adding the option to include PDBs in the package next to the corresponding .dlls.
The only question re PDBs is what the default value is. I think it would be best if Portable PDBs were included by default, so that DLLs from packages are debuggable by default. DLL dependencies of .NET Core Apps are loaded directly from NuGet cache during development, so once there is a PDB next to the DLL the debugger will be able to automatically load it and everything will just work. In case of .NET Framework Apps, we'd probably need to modify the SDK to copy the PDB to the output directory along with the DLL.

Re including sources: I don't think we should include sources in nuget packages at this point. We will provide solution in the dotnet SDK that will enable the debugger to find sources via Source Link and source embedding.

@nguerrera @richlander @gregg-miskelly for opinions.

[davidfowl]

So the proposal is to add a new field IncludeSymbolsInPackage?

[kzu]

Yes please. That would make it compatible with NuGetizer property without "compat mode" required: https://github.com/NuGet/NuGet.Build.Packaging/blob/dev/src/Build/NuGet.Build.Packaging.Tasks/NuGet.Build.Packaging.Inference.targets#L30

[tmat]

@kzu Any idea why there is '$(Configuration)' == 'Debug' on the line you pointed out?

[kzu]

Because I thought it was more intuitive to enable it by default only for debug builds? I guess I should probably check for DebugType instead? Or maybe not even that and just rely on the DebugSymbolsProjectOutputGroupOutput having something (or not)?

[kzu]

Makes sense to completely remove that from the condition. PR sent: https://github.com/NuGet/NuGet.Build.Packaging/pull/147

[jnm2]

In the meantime this is my workaround:

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

[thomaslevesque]

@jnm2 nice, thanks for the tip!

[ladeak]

@jnm2
Your suggestion works perfectly in VS 2017, but when I deploy to VSTS, it does not seem to have any effects.
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>

Do you know what the reason could be for this?

[jnm2]

@ladeak I don't know, sorry. To diagnose you'd have to get the VSTS deploy task to pass CLI arguments to MSBuild which make it give you back a log file. See http://msbuildlog.com/.

[bording]

@ladeak AllowedOutputExtensionsInPackageBuildOutputFolder requires VS 2017 15.4 / .NET Core 2.0.2 SDK to work. Perhaps VSTS hasn't updated yet?

[ladeak]

It say .Net Core 2.* (preview), log says Version 2.1.8 for dotnet pack.
The build step uses VS 2017 with MSBuild version 1.125.0. It seems to generate the pdb, just not included to the package.

[bording]

The build step uses VS 2017 with MSBuild version 1.125.0. It seems to generate the pdb, just not included to the package.

That seems to be pretty good indication that it's not using the version of the Pack targets included with the 2.0.2 SDK then.

If you want to get it working without waiting for the 2.0.2 SDK to be available, you can use this workaround instead:

<PropertyGroup>
  <TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);IncludePDBsInPackage</TargetsForTfmSpecificContentInPackage>
</PropertyGroup>
<Target Name="IncludePDBsInPackage" Condition="'$(IncludeBuildOutput)' != 'false'">
  <ItemGroup>
    <TfmSpecificPackageFile Include="$(OutputPath)\$(AssemblyName).pdb" PackagePath="lib\$(TargetFramework)" />
  </ItemGroup>
</Target>

[KellyR-STCU]

What is the currently best solution for this? 2.0.3

[rohit21agrawal]

@KellyR-STCU
use : <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>

[ctaggart]

As of SourceLink 2.7, it sets that property so that pdb files are included by default. I consider this issues resolved by AllowedOutputExtensionsInPackageBuildOutputFolder.

[jnm2]

Couldn't we have a nicer bool property like @davidfowl suggested?

This is a lot to type/paste and it shows its implementation details.

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

Also, it's missing .mdb. AllowedOutputExtensionsInSymbolsPackageBuildOutputFolder takes AllowedOutputExtensionsInPackageBuildOutputFolder and adds .pdb and .mdb.

What we really need is for the target to do this after it sets the defaults:

<AllowedOutputExtensionsInPackageBuildOutputFolder Condition="'$(IncludeSymbolsInPackage)' == 'true'">$(AllowedOutputExtensionsInSymbolsPackageBuildOutputFolder)</AllowedOutputExtensionsInPackageBuildOutputFolder>

[jnm2]

Since this issue was closed without action, I moved my proposal to https://github.com/NuGet/NuGet.Client/pull/1971.

[txwizard]

$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb

Where in the .csproj file does the above go?

[eltjo-k]

$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb

Where in the .csproj file does the above go?

Example of a complete project file:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netstandard1.1</TargetFramework>
    <VersionPrefix>1.0.2</VersionPrefix>
    <Description>Example</Description>
    <LangVersion>latest</LangVersion>
    <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.ComponentModel.Annotations" Version="4.5.0" />
  </ItemGroup>
</Project>