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

Investigate whether GitLink can be integrated with SourceLink #178

Closed
GeertvanHorrik opened this issue Mar 22, 2017 · 24 comments
Closed

Investigate whether GitLink can be integrated with SourceLink #178

GeertvanHorrik opened this issue Mar 22, 2017 · 24 comments

Comments

@GeertvanHorrik
Copy link
Contributor

I've recently noticed the updates and hard work you are putting into SourceLink, which is great. I also read the request to work more together, and I believe that's a good point. I would like to investigate the requirements / steps to merge GitLink and SourceLink into a single package so there will be one "go-to" package for devs.

A few things that I believe should be supported are:

Command line tooling

The tools should be executable as a command line tool.

Dynamic repositories (not all build servers have access to a local .git)

Some build servers (and the trend is moving towards this) don't check out the .git directory (too much overhead, and it's usually not required). However, this makes it a bit harder for the tooling to find out the remote repository. I think we can either pass this in as command line parameters or via msbuild property definitions. What are you thoughts?

Integration with MSBuild

@AArnott has been doing a lot of work on GitLink v3 (currently in alpha), and it would be nice if we can also merge his ideas into play (since it's a great idea to integrate with msbuild & work directly with pdb files). The package that is currently available simply requires the NuGet to be installed and the rest is done out of the box (it knows the pdb location, etc).

@ctaggart What are your thoughts, do you think it's feasible to migrate this or is SL2 very different from SL1 and should GL just be "as-is"? Do you still plan to keep working on SL1 or will SL2 completely replace SL1?

@ctaggart
Copy link
Owner

ctaggart commented Mar 22, 2017

I've recently noticed the updates and hard work you are putting into SourceLink, which is great. I also read the request to work more together, and I believe that's a good point. I would like to investigate the requirements / steps to merge GitLink and SourceLink into a single package so there will be one "go-to" package for devs.

Thank you @GeertvanHorrik! I would like this. I mentioned so on the Contributing wiki page.

What are your thoughts, do you think it's feasible to migrate this or is SL2 very different from SL1 and should GL just be "as-is"? Do you still plan to keep working on SL1 or will SL2 completely replace SL1?

I encourage you to try SourceLink v2 out. Hopefully, you've seen my blog post announcing v2. SourceLink v1, GitLink, and PdbGit are all tools to enable source server support. SourceLink v2 is the only one to enable source link support. I see source link support replace source server support. They both are for providing the source code, but source server link support is a better design baked into the compilers. With that and new APIs, this tool is much simpler. I ported the needed functionality to C# in part because I am hoping to add you and @AArnott as contributors to this project.

Please try out SourceLink v2. dotnet-sourcelink-git is a command line tool. SourceLink.Create.GitHub is a simply MSBuild integration that wraps it and makes it really easy to use. Adding those two to a new Directory.Build.props:

<Project>
  <ItemGroup>
    <PackageReference Include="SourceLink.Create.GitHub" Version="2.0.2" PrivateAssets="all" />
    <DotNetCliToolReference Include="dotnet-sourcelink-git" Version="2.0.2" />
  </ItemGroup>
</Project>

is all it takes for a lot of projects. If test projects need to be excluded, then @AArnott came up with IsTestProject:

<Project>
  <PropertyGroup>
    <IsTestProject>$(MSBuildProjectName.Contains('Test'))</IsTestProject>
    <DebugType>embedded</DebugType>
  </PropertyGroup>
  <ItemGroup Condition="'$(IsTestProject)' != 'true'">
    <PackageReference Include="SourceLink.Create.GitHub" Version="2.0.2" PrivateAssets="all" />
    <DotNetCliToolReference Include="dotnet-sourcelink-git" Version="2.0.2" />
  </ItemGroup>
</Project>

Please try it out and let me know what you think.

@AArnott
Copy link
Contributor

AArnott commented Mar 23, 2017

but source server support is a better design baked into the compilers.

@ctaggart I think you meant "source link support is a better..."

@AArnott
Copy link
Contributor

AArnott commented Mar 23, 2017

I'm in favor of combining efforts as well. 👏

I haven't studied SourceLinkv2's architecture enough to give pro's and con's about it, but from a user standpoint, I see these benefits of each (which our combined solution should retain all of IMO, or at least most):

GitLink goodness:

  1. Supports all C# project types (older csproj and newer)
  2. Supports all NuGet versions (project.json, packages.config, and PackageReference)
  3. Supports classic Windows PDBs
  4. Supports Source Server (SRCSRV) encoding of PDBs that is compatible with most debuggers used on Windows today.
  5. Normalizes capitalization of source files so they can be downloaded from the server even if the local disk uses different capitalization
  6. Support a build that is not within a git checked out folder (by passing in extra parameters).

SourceLinkv2 goodness:

  1. Supports portable PDBs
  2. Supports Source Link which is the future of debugging into remote source code
  3. Embeds source files into the PDB that are not tracked by git
  4. Rewrites line endings in source files before the compiler sees them if they do not conform to what git records so debuggers see the hashes equal.
  5. Offers CLI commands (to whom is this valuable ❓)
  6. Architecture allows for supporting more than just git ( true❓ )

Feel free to comment if I'm missing something and we can edit to fix it up. This might serve as something of our todo list.

@GeertvanHorrik
Copy link
Contributor Author

Sounds good, I will try to play with SL 2 tomorrow. A few questions that might be simple to answer:

  1. I use Fody a lot (which transforms some code after compilation and updates the pdb). In theory this should not affect the behavior of SL 2, but just wanted to make sure. Is my assumption correct that it's save to combine these?
  2. You mention that SL2 will also work with the classic desktop, but @AArnott mentions that it supports all stuff.

Do you think it's worth the effort of merging GL into SL1 or would that be a waste of resources? If we can already start using SL2 for desktop projects as well, then I vote not to combine these. Then we will release GL as is and hopefully we can deprecate that one in the future (once portable pdb stuff is the facto standard).

@AArnott
Copy link
Contributor

AArnott commented Mar 23, 2017

Is my assumption correct that it's save to combine these?

I'm guessing Fody doesn't yet support portable PDBs. So Fody would be broken in the scenarios where SL2 works today. Also it probably is broken if the project chooses to embed the PDB in the assembly itself unless Fody supports that.

You mention that SL2 will also work with the classic desktop, but @AArnott mentions that it supports all stuff.

This confuses me. Who is "you" in this context? SL2 only works on portable PDBs, which is not the default for desktop projects but can be turned on with a project property. SL2 does not support the windows legacy PDBs.

Do you think it's worth the effort of merging GL into SL1 or would that be a waste of resources?

I wouldn't bother. Either one works fine as-is. Merging GL and SL2 (or perhaps SL1 and SL2) in order to get all the scenarios supported that people care about does sound interesting.

@ctaggart
Copy link
Owner

Do you think it's worth the effort of merging GL into SL1 or would that be a waste of resources? If we can already start using SL2 for desktop projects as well, then I vote not to combine these. Then we will release GL as is and hopefully we can deprecate that one in the future (once portable pdb stuff is the facto standard).

I expect the portable pdb format to become de facto standard very soon. There is still a decent list of unsupported scenarios, but @tmat and others are looking to shink that. I would appreciate input from them too about where we should focus our efforts. I don't think it is worth the effort to merge GL and SL1. We should port over GL features/code to SL2 to fill any gaps.

If anyone is looking to work on some SourceLink code, Error with SourceLink with BouncyCastle #175 would be a good one. I just put down steps to reproduce it. Since it is a command line application, it makes it easy to reproduce errors.

@AArnott
Copy link
Contributor

AArnott commented Mar 23, 2017

Assuming Portable PDB takes the world by storm, we still have the issue that (I think) SL2 only works on very new project types. There's a whole world of code out there that hasn't upgraded yet and may not for years. It sounds like @ctaggart is in favor of filling this requirement within SL2, but this would require letting a nuget package stand alone (without a DotNetCliTool item) in these projects. Are you willing to go along with that, @ctaggart? I ask because we've discussed this before and you seemed resistant to the idea.

@ctaggart
Copy link
Owner

@AArnott It works with old project types too. You just need to use MSBuild 15. For example, take an old project, build it with MSBuild 15, then add a Directory.Build.props and build it again. Can you try that out?

@AArnott
Copy link
Contributor

AArnott commented Mar 23, 2017

Really? I guess I took it for granted that DotNetCliTool only works in the .NET SDK project types. If that's not the case, that's great. I'll try this out sometime in the next few days.

@ctaggart
Copy link
Owner

Me too. :-)

@ctaggart
Copy link
Owner

ctaggart commented Mar 28, 2017

I put together a Sourcelink.Create.CommandLine, which just uses the git command line to create the sourcelink.json. It does not call out to dotnet sourcelink-git. All of the commands may be replaced, in case you want to use hg or svn. I think this addresses some additional scenarios that you both brought up. It is currently available from the AppVeyor NuGet feed. https://ci.appveyor.com/project/ctaggart/sourcelink/build/2.1.0-b444/artifacts

@GeertvanHorrik
Copy link
Contributor Author

I'll look into it asap, some stuff has come up but I expect that I can get to this this week.

@ctaggart
Copy link
Owner

@AArnott DotNetCli just requires MSBuild 15. I tested it just now with the test plan for 2.1 in #188. I've published 2.1 with Sourcelink.Create.CommandLine, which allows something to work with the F# projects where dotnet doesn't work.

@ctaggart
Copy link
Owner

The thing I could use the most help on right is driving adoption.

@AArnott
Copy link
Contributor

AArnott commented Apr 4, 2017

I think an adoption deterrent is that ILSpy malfunctions for assemblies with portable PDBs next to them. It's a known issue, but one they should at least patch.

@GeertvanHorrik
Copy link
Contributor Author

I finally have time to work on this now. I'll first try if I can make this work on some repositories to get used to the source code. Once I have an understanding how SL2 works, we can analyze what needs to be done.

@ctaggart
Copy link
Owner

Cool. Try to use SourceLink.Create.CommandLine first. A good example is Rx.NET #167 (comment). There is new documenation for Source Link here https://github.com/dotnet/core/blob/master/Documentation/diagnostics/source_link.md

@ctaggart
Copy link
Owner

ctaggart commented May 12, 2017

@GeertvanHorrik, thanks for rearranging the solution as part of #209! Any more thought on this topic & becoming a SourceLink collaborator? I just sent you an invite. One thing that is new is that Microsoft is adding source link support to Windows PDB files too #194.

@GeertvanHorrik
Copy link
Contributor Author

Thanks for the invitation. I can't promise hard deadlines since contribute to a lot of OSS projects, but I'll try to help out where possible.

@gep13
Copy link

gep13 commented May 17, 2017

@GeertvanHorrik @ctaggart @AArnott just seeing this thread just now. Really happy to see a combining of efforts on this. Looking forward to checking out what you guys are able to create!

@GeertvanHorrik
Copy link
Contributor Author

@ctaggart there are a lot of up-for-grabs labels, but what are the easiest tasks to get started with?

@ctaggart
Copy link
Owner

@GeertvanHorrik, it would be good to tag and publish a 2.2.0 in a week or two #234. I just cleaned out the issues. The easiest would be in order as I see:

More difficult, reviewing and merging the two other open pull requests.

@ctaggart
Copy link
Owner

I'm still trying to get this into the .NET Foundation. #138
I've talked to Microsoft and I think this will happen. All parties will be involved.

@gep13
Copy link

gep13 commented Sep 25, 2017

@ctaggart said...
I've talked to Microsoft and I think this will happen. All parties will be involved.

That is great news! 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants