Documents node in embedded PDB different in .NET Core 3

daveaglick commented 4 years ago

This intrigued me and I've been meaning to dig into how SourceLink works anyway, so here we are. I think I know what's going on...or at least why it's failing. I just don't know how to fix it.

For starters, here's the embedded PDB metadata of a build on .NET Core SDK 2.2.401:

2020-01-28_21h45_52

And here's the embedded PDB metadata of a build on .NET Core SDK 3.1.101:

2020-01-28_21h39_40

There's so much more! But most importantly, check out that "Documents" node. It doesn't even exist in the .NET Core 2.2 build.

This can be verified by the sourcelink tool - here it is listing documents for the 2.2.401 build:

>tools\sourcelink print-documents bin\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.dll

>

Well that was anticlimactic. Here it is for the 3.1.101 build:

>dotnet sourcelink print-documents bin\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.dll
8c4deb227a4346af8cf4e320a3d78efdce7dd602bc423d79cd2d448dbb9ffc41 sha256 csharp C:\Code\Experiments\dotnetcore-sourcelink-test-bug\Class1.cs
a529e7a12beb0f9db49ac28387cf2d33b2dc765f7d70b604834a085118757966 sha256 csharp C:\Code\Experiments\dotnetcore-sourcelink-test-bug\obj\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
a6e03ae4df13fe05345e9022d1f1cd24ecae4bfd66db4843697c855d9f9335f4 sha256 csharp C:\Users\dglick\AppData\Local\Temp\.NETStandard,Version=v2.0.AssemblyAttributes.cs

>

I'm pretty sure that confirms the sourcelink tool is using the Documents node in the PDB. Just to be sure, I peeped the sourcelink tool code (which isn't in the DNF repo, but in a repo from @ctaggart directly):

Here's the actual code for the test command: https://github.com/ctaggart/SourceLink/blob/be8dcdaad09a2891bc0e1d04e43af40a1e482158/dotnet-sourcelink/Program.cs#L203

If we go back and look more closely at the output from test when it fails:

>dotnet sourcelink test bin\Release\dotnetcore-sourcelink-test-bug.1.0.0.nupkg
1 Documents without URLs:
a6e03ae4df13fe05345e9022d1f1cd24ecae4bfd66db4843697c855d9f9335f4 sha256 csharp C:\Users\dglick\AppData\Local\Temp\.NETStandard,Version=v2.0.AssemblyAttributes.cs
1 Documents with errors:
a529e7a12beb0f9db49ac28387cf2d33b2dc765f7d70b604834a085118757966 sha256 csharp C:\Code\Experiments\dotnetcore-sourcelink-test-bug\obj\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
https://raw.githubusercontent.com/shiftkey/dotnetcore-sourcelink-test-bug/e707de6428305d6a2bd997a516b5444592421cfd/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
error: url failed NotFound: Not Found
sourcelink test failed
failed for lib/netstandard2.0/dotnetcore-sourcelink-test-bug.dll
1 files did not pass in bin\Release\dotnetcore-sourcelink-test-bug.1.0.0.nupkg

We can see two errors which make sense to me now because we can track them back to entries in the PDB metadata as document nodes:

The file at "C:\Users\dglick\AppData\Local\Temp.NETStandard,Version=v2.0.AssemblyAttributes.cs" doesn't exist - that makes sense given that the file isn't local
The file with a translated URL of "https://raw.githubusercontent.com/shiftkey/dotnetcore-sourcelink-test-bug/e707de6428305d6a2bd997a516b5444592421cfd/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs" can't be found - again makes sense given that it translated a build-time generated file to a GitHub URL and the generated file obviously isn't committed.

TL;DR:

Lots of extra PDF metadata is getting added when using the .NET Core SDK 3.1.101
This extra metadata includes document nodes for generated files
The sourcelink tool reads those nodes from the embedded PDB and has no idea they're not actual source files

I'm left with the following questions:

Why is the 3.1.101 SDK generating these document nodes in the embedded PDB but earlier SDKs are not
Are the document nodes "correct" in the sense that not generating them was actually the wrong behavior?
Does the sourcelink tool need to know how to identify and deal with (or ignore) generated AssemblyAttributes.cs, AssemblyInfo.cs, and other generated source files?

shiftkey commented 4 years ago

@daveaglick thanks for digging into this and pointing out the interesting changes that might be important here. I'm gonna let this sit for a few days and let others chime in about ways to address this.

daveaglick commented 4 years ago

@shiftkey No problem at all - was a nice distraction from the usual even if it's not particularly helpful as a root cause analysis. Looks like @ctaggart has picked it up and has some ideas about what's going on.

rainersigwald commented 4 years ago

Looks like a change in the compiler. I can pass the test using .NET Core SDK 3.1.200-preview-014883 (what I had handy, should be identical to 3.1.1xx in this respect) with this patch:

diff --git a/dotnetcore-sourcelink-test-bug.csproj b/dotnetcore-sourcelink-test-bug.csproj
index f4f552a..5634149 100644
--- a/dotnetcore-sourcelink-test-bug.csproj
+++ b/dotnetcore-sourcelink-test-bug.csproj
@@ -7,6 +7,10 @@
   </PropertyGroup>

   <ItemGroup>
+    <PackageReference Include="Microsoft.Net.Compilers" Version="3.3.0">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All"/>
   </ItemGroup>

All that does is force using an older C# compiler. Maybe the new one has different behavior with respect to generated files?

poke commented 4 years ago

should be identical to 3.1.1xx in this respect

Can confirm that. I was able to get it work with your patch on 3.1.1.

japj commented 4 years ago

@daveaglick related to untracked sources: there is an option to embed untracked sources in the pdb as described at https://github.com/dotnet/sourcelink/blob/master/docs/README.md#embeduntrackedsources

That will allow the debugger to also debug "into" sources that are not in version control (e.g. build time generated code).

shiftkey commented 4 years ago

@rainersigwald @poke is there a way to validate that for non-Windows usage? I tried the same but it errors on macOS and it fails to build while mentioning Win32Exception (lol)

$ git diff
diff --git a/dotnetcore-sourcelink-test-bug.csproj b/dotnetcore-sourcelink-test-bug.csproj
index f4f552a..c52f4f6 100644
--- a/dotnetcore-sourcelink-test-bug.csproj
+++ b/dotnetcore-sourcelink-test-bug.csproj
@@ -7,6 +7,10 @@
   </PropertyGroup>

   <ItemGroup>
+    <PackageReference Include="Microsoft.Net.Compilers" Version="3.4.0">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All"/>
   </ItemGroup>

$ ./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 27.43 ms for /Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj.
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error MSB3883: Unexpected exception:  [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error : System.ComponentModel.Win32Exception (8): Exec format error [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at System.Diagnostics.Process.ForkAndExecProcess(String filename, String[] argv, String[] envp, String cwd, Boolean redirectStdin, Boolean redirectStdout, Boolean redirectStderr, Boolean setCredentials, UInt32 userId, UInt32 groupId, UInt32[] groups, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean usesTerminal, Boolean throwOnNoExec) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at System.Diagnostics.Process.StartCore(ProcessStartInfo startInfo) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at System.Diagnostics.Process.Start() [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at Microsoft.Build.Utilities.ToolTask.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.4.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at Microsoft.CodeAnalysis.BuildTasks.ManagedCompiler.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
file does not exist: bin/Release/dotnetcore-sourcelink-test-bug.1.0.0.nupkg

daveaglick commented 4 years ago

@japj Cool, TIL

Confirmed that setting <EmbedUntrackedSources>true</EmbedUntrackedSources> gets past the "1 Documents without URLs" due to missing AssemblyAttributes.cs but doesn't resolve the "1 Documents with errors" error from AssemblyInfo.cs.

Also doesn't address the underlying problem of compiler/sourcelink compatibility regarding compiler generated files and embedded PDBs, but at least gets closer to a successful sourcelink test.

poke commented 4 years ago

@shiftkey Could you try the 3.3.0 compiler version? 3.4.0 also didn’t work for me on Windows due to some MSBuild version.

shiftkey commented 4 years ago

@poke same issue on macOS

./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 182.35 ms for /Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj.
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error MSB3883: Unexpected exception:  [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error : System.ComponentModel.Win32Exception (8): Exec format error [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at System.Diagnostics.Process.ForkAndExecProcess(String filename, String[] argv, String[] envp, String cwd, Boolean redirectStdin, Boolean redirectStdout, Boolean redirectStderr, Boolean setCredentials, UInt32 userId, UInt32 groupId, UInt32[] groups, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean usesTerminal, Boolean throwOnNoExec) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at System.Diagnostics.Process.StartCore(ProcessStartInfo startInfo) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at System.Diagnostics.Process.Start() [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at Microsoft.Build.Utilities.ToolTask.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
/Users/shiftkey/.nuget/packages/microsoft.net.compilers/3.3.0/tools/Microsoft.CSharp.Core.targets(59,5): error : at Microsoft.CodeAnalysis.BuildTasks.ManagedCompiler.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands) [/Users/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj]
file does not exist: bin/Release/dotnetcore-sourcelink-test-bug.1.0.0.nupkg

shiftkey commented 4 years ago

@poke @rainersigwald nevermind, I should have spotted this clue from NuGet:

This package can be used to compile code targeting any platform, but can only be run using the desktop .NET 4.7.2+ Full Framework.

I'll dust off my Windows VM

KirillOsenkov commented 4 years ago

Paging @tmat

tmat commented 4 years ago

Can you share the binaries?

shiftkey commented 4 years ago

@tmat the binary is generated from the csproj file in the repository root

You should be able to run ./repro.sh to trigger the issue. I wrote this last night on macOS and re-tested on Linux, but the commands in that file are easy to follow if you're using a different shell.

shiftkey commented 4 years ago

I tested this on Windows with .NET Core SDK 3.0.100 and sourcelink test passes there, so that narrows down the versions to where this was introduced:

$ ./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.3.0+0f4c62fea for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 327.45 ms for C:\Users\shiftkey\src\dotnetcore-sourcelink-test-bug\dotnetcore-sourcelink-test-bug.csproj.
  dotnetcore-sourcelink-test-bug -> C:\Users\shiftkey\src\dotnetcore-sourcelink-test-bug\bin\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:06.18
sourcelink test passed: bin/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.dll

rainersigwald commented 4 years ago

For non-Windows use this package instead

diff --git a/dotnetcore-sourcelink-test-bug.csproj b/dotnetcore-sourcelink-test-bug.csproj
index f4f552a..edaa520 100644
--- a/dotnetcore-sourcelink-test-bug.csproj
+++ b/dotnetcore-sourcelink-test-bug.csproj
@@ -7,6 +7,10 @@
   </PropertyGroup>

   <ItemGroup>
+    <PackageReference Include="Microsoft.NETCore.Compilers" Version="3.3.0">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All"/>
   </ItemGroup>

SDK 3.0.100 shipped with Roslyn 3.3.0 (coordinated with VS 16.3), and SDK 3.1.100 shipped with Roslyn 3.4.0 (coordinated with VS 16.4).

rainersigwald commented 4 years ago

Note: you shouldn't downgrade the compilers for production use. I'm suggesting it here to narrow down the problem.

shiftkey commented 4 years ago

@rainersigwald all good, I walked through the published versions and NuGet and tested whether they passed:

3.3.0 - ✅
3.3.1 - ✅
3.4.0-beta1-final - ✅
3.4.0-beta2-final - ✅
3.4.0-beta3-final - :x:
3.4.0-beta4-final - :x:
3.4.0 - :x:

That seems to narrow the range of changes to somewhere in here:

3.4.0-beta2-final - https://github.com/dotnet/roslyn/commit/154af84a603094b52bd08b3366c4448f7481af52
3.4.0-beta3-final - https://github.com/dotnet/roslyn/commit/b8a5611e3db4f7ac3b5e1404b129304b5baf843e

Comparison Link (sadly too big for GitHub to render 😢 ) - https://github.com/dotnet/roslyn/compare/154af84a603094b52bd08b3366c4448f7481af52...b8a5611e3db4f7ac3b5e1404b129304b5baf843e

shiftkey commented 4 years ago

I setup a script to building Roslyn from source and apply it to the repro, and bisected the resulting commits above and stumbled upon this commit that seems to introduce the problem - Include source files w/o method bodies in the PDB documents: https://github.com/dotnet/roslyn/commit/fce41a72b037ea633e06c763bdcb68e01af9e385

This was merged as part of https://github.com/dotnet/roslyn/pull/39136 which fixed https://github.com/dotnet/roslyn/issues/38954. I'm not familiar with the "sequence points" feature mentioned above, but I'd guess that the impact to Sourcelink is an unexpected side-effect.

KirillOsenkov commented 4 years ago

@ivanbasov

tmat commented 4 years ago

@shiftkey Thanks for root-causing the issue. It's now immediately obvious to me what's going on.

First of all, the root cause of the issue is https://github.com/microsoft/msbuild/issues/1479 and a workaround is mentioned in the description.

dotnet/roslyn#39136 works as designed. Prior to this change the compiler did not include source file entries in the document table of the PDB that did not contain any method bodies. The PR changed the compiler to always include all source files passed to the compiler. There was no good reason for the compiler to behave the way it did before, it was pretty much a result of internal implementation detail leaking through. We have found this behavior problematic in several ways as it prevented us from making other features reliable.

Unfortunately, we did not anticipate dotnet/roslyn#39136 exacerbating the effect of https://github.com/microsoft/msbuild/issues/1479.

tmat commented 4 years ago

Fix for microsoft/msbuild#1479: https://github.com/microsoft/msbuild/pull/5101

tmat commented 4 years ago

Another thing - even when microsoft/msbuild#5101 is fixed you'll need to set EmbedUntrackedSources to true, so that the compiler embeds sources that are not in git repo into the PDB. Otherwise, you'll still get error: url failed NotFound: Not Found errors.

shiftkey commented 4 years ago

@tmat thanks for the tip about EmbedUntrackedSource - happy to test and verify things on my end when there's a build ready to pull in.

KirillOsenkov commented 4 years ago

@shiftkey btw as a workaround you can set the property Tomas is setting in your own codebase (Directory.Build.props). You don't have to wait for the MSBuild fix to go in.

shiftkey commented 4 years ago

@KirillOsenkov the EmbedUntrackedSources value? I tried including it in the csproj file itself but it didn't seem to change the behaviour. I don't have a Directory.Build.props file but I'll test that and confirm I can workaround it. Will report back.

KirillOsenkov commented 4 years ago

no, you also need this:

<PropertyGroup>
  < EmbedUntrackedSources >true</EmbedUntrackedSources >

<TargetFrameworkMonikerAssemblyAttributesPath>$([System.IO.Path]::Combine('$(IntermediateOutputPath)','$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)'))</TargetFrameworkMonikerAssemblyAttributesPath>
</PropertyGroup>

KirillOsenkov commented 4 years ago

this should be visible to all projects, one way to do it is have a Directory.Build.props at the root of your repo

shiftkey commented 4 years ago

@KirillOsenkov is that the entire fix? I tested the change in the project here:

diff --git a/dotnetcore-sourcelink-test-bug.csproj b/dotnetcore-sourcelink-test-bug.csproj
index f4f552a..5752d9c 100644
--- a/dotnetcore-sourcelink-test-bug.csproj
+++ b/dotnetcore-sourcelink-test-bug.csproj
@@ -4,6 +4,8 @@
     <TargetFrameworks>netstandard2.0</TargetFrameworks>
     <RootNamespace>dotnetcore_sourcelink_test_bug</RootNamespace>
     <DebugType>embedded</DebugType>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
+    <TargetFrameworkMonikerAssemblyAttributesPath>$([System.IO.Path]::Combine('$(IntermediateOutputPath)','$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)'))</TargetFrameworkMonikerAssemblyAttributesPath>
   </PropertyGroup>

   <ItemGroup>

And while the behaviour changes, I now see two failing files:

$ ./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 26.52 ms for /home/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj.
  dotnetcore-sourcelink-test-bug -> /home/shiftkey/src/dotnetcore-sourcelink-test-bug/bin/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.89
2 Documents with errors:
cc2dfcd56f5a115cc05c27d10c7c4b1545b3232fbbd18c0064fea0c99a24091f sha256 csharp /home/shiftkey/src/dotnetcore-sourcelink-test-bug/.AssemblyAttributes
https://raw.githubusercontent.com/shiftkey/dotnetcore-sourcelink-test-bug/c49a1900e297a776d414a333862b2179a721fe47/.AssemblyAttributes
error: url failed NotFound: Not Found
fb70b369d74ecda0872ffd29c9b4517a4c7881fb6fe45c1149b54697d6387855 sha256 csharp /home/shiftkey/src/dotnetcore-sourcelink-test-bug/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
https://raw.githubusercontent.com/shiftkey/dotnetcore-sourcelink-test-bug/c49a1900e297a776d414a333862b2179a721fe47/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
error: url failed NotFound: Not Found
sourcelink test failed

I think I also need <EmbedAllSources>true</EmbedAllSources> here:

diff --git a/dotnetcore-sourcelink-test-bug.csproj b/dotnetcore-sourcelink-test-bug.csproj
index f4f552a..8b71d31 100644
--- a/dotnetcore-sourcelink-test-bug.csproj
+++ b/dotnetcore-sourcelink-test-bug.csproj
@@ -4,6 +4,9 @@
     <TargetFrameworks>netstandard2.0</TargetFrameworks>
     <RootNamespace>dotnetcore_sourcelink_test_bug</RootNamespace>
     <DebugType>embedded</DebugType>
+    <EmbedAllSources>true</EmbedAllSources>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
+    <TargetFrameworkMonikerAssemblyAttributesPath>$([System.IO.Path]::Combine('$(IntermediateOutputPath)','$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)'))</TargetFrameworkMonikerAssemblyAttributesPath>
   </PropertyGroup>

   <ItemGroup>

And we have success:

$ ./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 26.65 ms for /home/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj.
  dotnetcore-sourcelink-test-bug -> /home/shiftkey/src/dotnetcore-sourcelink-test-bug/bin/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.75
sourcelink test passed: bin/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.dll

Does that sound right?

shiftkey commented 4 years ago

@KirillOsenkov looks like only <EmbedAllSources>true</EmbedAllSources> is needed here to make sourcelink test happy for me, but I'll keep an eye on the other issues as they move along.

tmat commented 4 years ago

@shiftkey EmbedAllSources makes Source Link unnecessary as it embeds all source files to the PDB. EmbedUntrackedSources only embeds files that are not committed in the repository. The rest of the files are located via Source Link.

tmat commented 4 years ago

Setting TargetFrameworkMonikerAssemblyAttributesPath needs to be done after importing SDK targets because IntermediateOutputPath is set in Microsoft.Common.CurrentVersion.targets and not before that.

So I think the best would be to set it in Directory.Build.targets in the repo root.

You can see from the sourcelink test error message that the path is wrong:

cc2dfcd56f5a115cc05c27d10c7c4b1545b3232fbbd18c0064fea0c99a24091f sha256 csharp /home/shiftkey/src/dotnetcore-sourcelink-test-bug/.AssemblyAttributes

shiftkey commented 4 years ago

@tmat this project pre-dates .NET Core, so it hasn't really caught up to these sorts of habits.

I'll try adding one in and see how that goes.

shiftkey commented 4 years ago

@tmat @KirillOsenkov I realise I've been told differing advice about which file to add with these settings:

one way to do it is have a Directory.Build.props at the root of your repo ... So I think the best would be to set it in Directory.Build.targets in the repo root.

I ended up going with the .targets file because it feels like this needs to be set as early as possible. However it doesn't seem to change the problem with the repro here (I've pushed the commit to master so you can see the same thing):

$ cat Directory.build.targets
<Project>
  <PropertyGroup>
    <EmbedUntrackedSources>true</EmbedUntrackedSources>
    <TargetFrameworkMonikerAssemblyAttributesPath>$([System.IO.Path]::Combine('$(IntermediateOutputPath)','$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)'))</TargetFrameworkMonikerAssemblyAttributesPath>
  </PropertyGroup>
</Project>

$ ./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 42.71 ms for C:\Users\shiftkey\src\dotnetcore-sourcelink-test-bug\dotnetcore-sourcelink-test-bug.csproj.
  dotnetcore-sourcelink-test-bug -> C:\Users\shiftkey\src\dotnetcore-sourcelink-test-bug\bin\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.18
1 Documents with errors:
a4c19432de75e181e356d760bbb28a912db5b9af9dcea512ddafa409dc8a3ce1 sha256 csharp C:\Users\shiftkey\src\dotnetcore-sourcelink-test-bug\obj\Release\netstandard2.0\dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
https://raw.githubusercontent.com/shiftkey/dotnetcore-sourcelink-test-bug/627e024cc009eb652f15898454c666fe2ae9e7e9/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
error: url failed NotFound: Not Found
sourcelink test failed

tmat commented 4 years ago

I think msbuild might not be importing the file. Since this is on Linux casing matters and it needs to be Directory.Build.targets (upper-case B).

shiftkey commented 4 years ago

No change:

$ cat Directory.Build.targets
<Project>
  <PropertyGroup>
    <EmbedUntrackedSources>true</EmbedUntrackedSources>
    <TargetFrameworkMonikerAssemblyAttributesPath>$([System.IO.Path]::Combine('$(IntermediateOutputPath)','$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)'))</TargetFrameworkMonikerAssemblyAttributesPath>
  </PropertyGroup>
</Project>

$ ./repro.sh
Tool 'sourcelink' (version '3.1.1') was restored. Available commands: sourcelink

Restore was successful.
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 27.6 ms for /home/shiftkey/src/dotnetcore-sourcelink-test-bug/dotnetcore-sourcelink-test-bug.csproj.
  dotnetcore-sourcelink-test-bug -> /home/shiftkey/src/dotnetcore-sourcelink-test-bug/bin/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.75
1 Documents with errors:
acc55c21d05bc6b728d166bafefd81e250877e4fd9bd2885ae2167f98bac2d02 sha256 csharp /home/shiftkey/src/dotnetcore-sourcelink-test-bug/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
https://raw.githubusercontent.com/shiftkey/dotnetcore-sourcelink-test-bug/35d5c01831cc78bbaab84ef951f455f3b1ae0943/obj/Release/netstandard2.0/dotnetcore-sourcelink-test-bug.AssemblyInfo.cs
error: url failed NotFound: Not Found
sourcelink test failed

KirillOsenkov commented 4 years ago

Oh, if you're not using the SDK-style project format the Directory.Build.props/targets won't get picked up.

Unfortunately in that case you'll need to resort to manually setting the properties directly in the .csproj file (at the very end, after all imports).

And Tomas was right earlier, these properties should go in Directory.Build.targets file, since it gets imported almost at the end. Directory.Build.props is being imported very early on (if you're using an SDK-style project).

Try using a binlog and http://msbuildlog.com to see if the properties are being set correctly for your project.

bording commented 4 years ago

I've been following this as well, and it's not clear to me why the TargetFrameworkMonikerAssemblyAttributesPath would address all the problems.

With my own minimal project to repro the issue (https://github.com/bording/SourceLinkProblem) the AssemblyAttributes.cs file in the temp folder isn't the problem. It's still being properly embedded:

sourcelink print-urls .\SourceLinkProblem.pdb
0cb850dcfd7c23e8e3acd133161e0788640146c5b60292d3022c345d0e3bd7d7 sha256 csharp C:\Users\Brandon\source\repos\SourceLinkProblem\SourceLinkProblem\Program.cs https://raw.githubusercontent.com/bording/SourceLinkProblem/82b0dc7a9366f1af96f499bf0d6ad0f5a8f3b0c1/SourceLinkProblem/Program.cs
609721b0977b4bb4296f30db82935147eb4a1c0dd665d90d1ca0787ff0f86f1d sha256 csharp C:\Users\Brandon\AppData\Local\Temp\.NETCoreApp,Version=v3.1.AssemblyAttributes.cs embedded
909fb12537feca3c1b9469ac7fde57a6ad05b5d78502c18f9ce37ca62452a3a6 sha256 csharp C:\Users\Brandon\source\repos\SourceLinkProblem\SourceLinkProblem\obj\Debug\netcoreapp3.1\SourceLinkProblem.AssemblyInfo.cs https://raw.githubusercontent.com/bording/SourceLinkProblem/82b0dc7a9366f1af96f499bf0d6ad0f5a8f3b0c1/SourceLinkProblem/obj/Debug/netcoreapp3.1/SourceLinkProblem.AssemblyInfo.cs

The problem is that despite having <EmbedUntrackedSources>true</EmbedUntrackedSources> in the project file, the SourceLinkProblem.AssemblyInfo.cs from the obj folder is not being embedded.

This is the same thing that you can see from @shiftkey's output in https://github.com/shiftkey/dotnetcore-sourcelink-test-bug/issues/1#issuecomment-582075845

It doesn't seem like https://github.com/microsoft/msbuild/pull/5101 fixes this problem.

tmat commented 4 years ago

There is another issue - GenerateAssemblyInfo target is ordered after the Source Link's _SetEmbeddedFilesFromSourceControlManagerUntrackedFiles target:

This means Source Link won't see the file.

The workaround is to specify it manually in Directory.Build.targets file:

<Project>
  <PropertyGroup>
    <TargetFrameworkMonikerAssemblyAttributesPath>$([System.IO.Path]::Combine('$(IntermediateOutputPath)','$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)'))</TargetFrameworkMonikerAssemblyAttributesPath>
  </PropertyGroup>
  <ItemGroup>
    <EmbeddedFiles Include="$(GeneratedAssemblyInfoFile)"/>
  </ItemGroup>
</Project>

tmat commented 4 years ago

Fix: https://github.com/dotnet/sdk/pull/10613

bording commented 4 years ago

@tmat I can confirm that the Directory.Build.targets file does address both issues. The AssemblyAttributes.cs file is in the obj folder and embedded, and the AssemblyInfo.cs is properly embedded as well.

sourcelink test passes now.

KirillOsenkov commented 4 years ago

Note that this line may also be necessary:

<EmbeddedFiles Include="$(TargetFrameworkMonikerAssemblyAttributesPath)"/>

shiftkey / dotnetcore-sourcelink-test-bug

Documents node in embedded PDB different in .NET Core 3 #1