Enable source link for all 'src' projects that produce DLLs

[Eilon]

See aspnet/Mvc#5905 for how to use an existing NuGet package by @ctaggart that does this.

We'll want to do this via KoreBuild so that it applies to all 'src' projects that produce DLLs.

[Eilon]

@ryanbrandenburg feel free to start partying on this. We can sync in person if anything isn't clear.

[Eilon]

Please use SourceLink.Create.CommandLine version 2.2.1 for this (I have approval for it).

[davidfowl]

@Eilon when I spoke to @jaredpar turns out there were some performance problems with roslyn when they looked at using that package (something about launching too many processes) such that it slowed the build down too much.

/cc @tmat @pranavkm

[ctaggart]

He then supplied the values to avoid looking them up each time via a command line process for each build.

[davidfowl]

@ctaggart is it all fixed then?

[ctaggart]

I'd say yes. Lookup the Roslyn pull request and sync with @jaredpar again. I'm on vacation in Greece with just my phone, else I'd be more helpful.

To recap, you can look at the targets file to see what values need to set to skip running the processes. https://github.com/ctaggart/SourceLink/blob/master/SourceLink.Create.CommandLine/SourceLink.Create.CommandLine.targets

• SourceLinkRootDirectory to the filesystem path where the Git repo it.
• SourceLinkOriginUrl to the GitHub Git URL.
• SourceLinkCommit to the Git commit being used for the build.

[vancem]

The PR that is being referenced is dotnet/roslyn#20719.
We should clone that here unless there is a good reason not to.

[vancem]

Looking this over a bit, the Roslyn team already had infrastructure for getting the GIT commit ID and repository URL and thus just wired into that. I don't know if the ASP.NET build has that.

The SourceLink code does figure that out on its own if you let it, but it will probably have the performance issue that @jaredpar mentions in dotnet/roslyn#20719.

Note that ultimately what is being done is pretty simple.

1. Run some GIT commands so that you get the commit ID and URL, and from that create a very simple file symbol_link.json in the intermediate directory.
2. Set the SourceLink variable to this file. This causes the C# compiler to do the rest.

At most this is a set of MSBUILD statements (see https://github.com/dotnet/core/blob/master/Documentation/diagnostics/source_link.md) Now this code probably has the problem of executing these commands on every DLL produced, but you definitely could put in something that caused it to skip if the existing file is not that old (say 5 min), this would get rid of most if not all the overhead.

@ryanbrandenburg - ideally we fix this soon (this week), and ideally we get it integrated into the 2.01 release (otherwise we have to hack again).

[Eilon]

@vancem , ah it's a bit trickier to get into the 2.0.x patch but we'll give it a shot. We have other infrastructure stuff to figure out for 2.0.x so this might be able to be rolled into that.

[jaredpar]

I really dont think the extra processes will be noticable on most solutions. Roslyn is really big and hence the problem gets magnified a lot. We are also really sensitive about build speed

[Eilon]

ASP.NET/EF Core is ~200 source projects but we'll see what happens 😄

[pranavkm]

cc @natemcmaster \ @anurse. We already have access to the commit sha in our repo builds and we get to it by reading files in the .git directory. The repo url is stored in gitconfig so it's a bit more obtuse to read it without running git.

[natemcmaster]

The repo url is stored in gitconfig

Also stored as an MSBuild property (example)...though we never verify it or use it.

[jaredpar]

ASP.NET/EF Core is ~200 source projects but we'll see what happens

Yeah you will notice the perf hit. Roslyn is ~160 and it was measurable there.

[vancem]

It sounds like you can do what Roslyn did (set variables for the GIT commit/URL so you don't need to run GIT. It sounds like ASP.NET has enough projects so this is worth it.

Note that we should not be so allergic to invoking GIT commands. It is frankly better to do that than to rummage around in the .git directory (taking dependencies on internal details), to avoid it. The main issue is that you would prefer to only do these commands ONCE per Repo, and share the result for all the projects in the repo (since the commit ID and URL do not change for the whole repo). That is not hard to do (basically save the result in a file / MSBUILD variable and skip if it has already been done and is not too old (say a few mins).

Who wants to take a stab at it? I am willing to do so if others would rather not. It looks like we might already have the necessary information in MSBUILD variables (in which case it is pretty easy).

Here are the necessary lines we used for CoreFX and CoreCLR. In the end what these to is create a source_link.json file from the MSBUILD variables holding the GIT commit ID and URL, and then set the SourceLink variable to point at that file. C# will do the rest. We just need to adapt them to the names that the ASP.NET build uses...

  <PropertyGroup>
    <SourceLinkFilePath>$(BaseIntermediateOutputPath)source_link.json</SourceLinkFilePath>
  </PropertyGroup>

  <PropertyGroup Condition="'$(UseSourceLink)' == 'true' AND Exists('$(SourceLinkFilePath)')">
    <SourceLink>$(SourceLinkFilePath)</SourceLink>
  </PropertyGroup>

<!-- This is under a target that is run before the C# compiler is run (ideally very eary, once for the entire build -->

    <MakeDir Directories="$(BaseIntermediateOutputPath)" />
    <WriteLinesToFile Condition="'$(UseSourceLink)' == 'true' AND '$(GitHubRepositoryUrl)' != '' AND '$(LatestCommit)' != 'N/A'"
      File="$(BaseIntermediateOutputPath)source_link.json" 
      Overwrite="true" 
      Lines='{"documents": { "$(ProjectDir.Replace("\", "\\"))*" : "$(GitHubRepositoryUrl.Replace("github.com", "raw.githubusercontent.com"))/$(LatestCommit)/*" }}' />

Note that if you use the lines above, you don't need to take a dependency on the SourceLink package. That package basically is stuff so you only have to write one line (the package reference), but it does mean running git commands on each DLL (which is not a big deal for most people, but ASP.NET and Roslyn were exceptions).

[Eilon]

Yeah we should do whatever makes the most sense for us and is easy to maintain.

@ryanbrandenburg - please take into account @vancem 's post above when working on this.

[analogrelay]

Yeah, I'm OK with running git if we need it. It's better than diving too deep into .git.

My suggestion would be to have KoreBuild load up the Git and SourceLink data and pass it through to the projects when they build.

[ctaggart]

@ryanbrandenburg, @Eilon what is the new route? The public communication seams to have stagnated and there doesn't appear to have been any progress.

[Eilon]

@ctaggart ah sorry, the guy who was working on this was on vacation for 2.5 weeks and just returned. @ryanbrandenburg - can you share an update when you have a chance?

[ryanbrandenburg]

I discussed it with @natemcmaster and we came to the conclusion that between the build from source effort, the desire to only SourceLink "src" projects, and our simple requirements, it would be simpler to write a SourceLink task ourselves and include it in Internal.AspNetCore.Sdk.

[Eilon]

That sounds fine to me, thanks!

[ctaggart]

Now that aspnet/BuildTools#423 has merged, when will the pdb files be added to the nuget packages? #131

[ryanbrandenburg]

The question of when and where to ship the pdb's is, I feel, a separate issue which is addressed by #131.