Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

dotnet pack pdb files by default #291

Merged
merged 4 commits into from
Dec 27, 2017
Merged

dotnet pack pdb files by default #291

merged 4 commits into from
Dec 27, 2017

Conversation

ctaggart
Copy link
Owner

@ctaggart ctaggart commented Dec 26, 2017

by adding .pdb to AllowedOutputExtensionsInPackageBuildOutputFolder #282

Related links:

dotnet pack and msbuild /t:pack do not include the pdb files by default. As of Visual Studio 2017 15.4 or .NET Core 2.0.2 SDK and above, you can fix this by modifying this property:

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

We may automatically set that property in the next version of SourceLink, see issue #282.

The previous recommended way of including them was to use the extension point designed for including content that is different for each target framework:

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

by adding .pdb to AllowedOutputExtensionsInPackageBuildOutputFolder
@ctaggart ctaggart added this to the 2.7.0 milestone Dec 26, 2017
@ctaggart
Copy link
Owner Author

ctaggart commented Dec 26, 2017

This isn't working for me with my tests just now here:
https://github.com/ctaggart/sourcelink-test

Some notes:
PS C:\Users\camer\cs\sourcelink-test\VS2017CSharpLibraryCore> dotnet nuget --version
NuGet Command Line
4.5.0.4

<AllowedOutputExtensionsInPackageBuildOutputFolder>.dll; .exe; .winmd; .json; .pri; .xml; $(AllowedOutputExtensionsInPackageBuildOutputFolder)</AllowedOutputExtensionsInPackageBuildOutputFolder>

https://github.com/NuGet/NuGet.Client/blob/release-4.5.0-rtm/src/NuGet.Core/NuGet.Build.Tasks.Pack/Pack.targets

@jnm2
Copy link
Contributor

jnm2 commented Dec 26, 2017

Using the structured log viewer, I see that nothing I do in the NuGet-imported .props and .targets seems to have any effect on the Pack MSBuild invocation. It affects all the invocations that Pack depends on, but not Pack itself.

@nguerrera Are you able to give us some pointers on how to manipulate the Pack target from .props imported from NuGet packages? Thanks!

@jnm2
Copy link
Contributor

jnm2 commented Dec 26, 2017

I see this if I preprocess the .csproj:

  <ImportGroup Condition=" '$(TargetFramework)' == 'net461' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b626\build\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b626\build\SourceLink.Embed.PaketFiles.props')" />
  </ImportGroup>
  <ImportGroup Condition=" '$(TargetFramework)' == 'netstandard1.4' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b626\build\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b626\build\SourceLink.Embed.PaketFiles.props')" />
  </ImportGroup>

That means the .props is not imported from the NuGet package unless there is a TargetFramework property equal to net461. But at pack time, there is no single target framework and so of course there is no TargetFramework property. There is only TargetFrameworks equal to netstandard1.4;net461. So the .props isn't even getting included. I don't see any recourse we could take. It's not like your .props is in /build/net461; it's in /build.

ctaggart added a commit to ctaggart/sourcelink-test that referenced this pull request Dec 27, 2017
@ctaggart ctaggart mentioned this pull request Dec 27, 2017
@ctaggart
Copy link
Owner Author

Thanks @jnm2,

I troubleshot this today with ctaggart/sourcelink-test@3a8feea and you are indeed correct. It is working with TargetFramework, but not TargetFrameworks. :-(

It currently produces:

  <ImportGroup Condition=" '$(TargetFramework)' == 'net461' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props')" />
    <Import Project="$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props')" />
  </ImportGroup>
  <ImportGroup Condition=" '$(TargetFramework)' == 'netstandard1.4' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props')" />
    <Import Project="$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props')" />
  </ImportGroup>

If it also added a condition for when TargetFramework is empty, it would work:

  <ImportGroup Condition=" '$(TargetFramework)' == '' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props')" />
    <Import Project="$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props')" />
  </ImportGroup>

@jnm2
Copy link
Contributor

jnm2 commented Dec 27, 2017

Yes. Even better:

  <ImportGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b631\build\SourceLink.Embed.PaketFiles.props')" />
    <Import Project="$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b631\build\SourceLink.Create.CommandLine.props')" />
  </ImportGroup>

And only use '$(TargetFramework)' == 'net461' when the .props is under /build/net461.

@jnm2
Copy link
Contributor

jnm2 commented Dec 27, 2017

Could you open an issue at https://github.com/dotnet/sdk, maybe?

@ctaggart
Copy link
Owner Author

ctaggart commented Dec 27, 2017

I'll open up an issue in https://github.com/NuGet/Home. I think that is the right spot.

@ctaggart
Copy link
Owner Author

Yay, buildCrossTargeting works for TargetFrameworks. It adds:

  <ImportGroup Condition=" '$(TargetFramework)' == '' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b637\buildCrossTargeting\SourceLink.Embed.PaketFiles.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.embed.paketfiles\2.7.0-b637\buildCrossTargeting\SourceLink.Embed.PaketFiles.props')" />
    <Import Project="$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b637\buildCrossTargeting\SourceLink.Create.CommandLine.props" Condition="Exists('$(NuGetPackageRoot)sourcelink.create.commandline\2.7.0-b637\buildCrossTargeting\SourceLink.Create.CommandLine.props')" />
  </ImportGroup>

@ctaggart ctaggart merged commit ab1fad1 into master Dec 27, 2017
@ctaggart ctaggart deleted the pack-pdb branch December 27, 2017 19:35
@jnm2
Copy link
Contributor

jnm2 commented Dec 27, 2017

Nice to know! I'm embarrassed that I forgot that existed. 😆

stakx added a commit to stakx/moq that referenced this pull request Mar 10, 2019
ctaggart's SourceLink has at some point started packing `.pdb` files
into the main Nuget package, which is not ideal if we want to publish
a separate `.snupkg` to NuGet.

For that reason, let's make the switch to `dotnet/SourceLink` even
though it is *still* only in beta.

References:
 * ctaggart/SourceLink#291
stakx added a commit to stakx/moq that referenced this pull request May 27, 2019
ctaggart's SourceLink has at some point started packing `.pdb` files
into the main Nuget package, which is not ideal if we want to publish
a separate `.snupkg` to NuGet.

For that reason, let's make the switch to `dotnet/SourceLink` even
though it is *still* only in beta.

References:
 * ctaggart/SourceLink#291
ishimko pushed a commit to ishimko/moq4 that referenced this pull request Sep 2, 2019
ctaggart's SourceLink has at some point started packing `.pdb` files
into the main Nuget package, which is not ideal if we want to publish
a separate `.snupkg` to NuGet.

For that reason, let's make the switch to `dotnet/SourceLink` even
though it is *still* only in beta.

References:
 * ctaggart/SourceLink#291
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants