Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable source link support #923

Closed
ctaggart opened this issue Mar 21, 2017 · 9 comments
Closed

Enable source link support #923

ctaggart opened this issue Mar 21, 2017 · 9 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: process A process-related concern. May include testing, release, or the like.

Comments

@ctaggart
Copy link
Contributor

ctaggart commented Mar 21, 2017

It would be awesome if these libraries enabled source link support. As we use these libraries, we run into exceptions where being able to debug into the source code would be really helpful. Enabling source link support is very easy once you upgrade to MSBuild 15.

An easy way is just adding a Directory.Build.props in the right directory.

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

I'll happily submit a pull request after ya'll have upgraded to MSBuild 15. For details see:
http://blog.ctaggart.com/2017/03/enable-source-link-support-announcing.html
https://github.com/ctaggart/SourceLink/

@jskeet
Copy link
Collaborator

jskeet commented Mar 21, 2017

Thanks - when we've updated, I'd certainly be interested in that. We do push the source packages to Symbol Source already, so I'd hope that would already work for debugging. Will read your blog post for more details :)

(And two minutes later, I see why symbol source isn't enough :)

@jskeet
Copy link
Collaborator

jskeet commented Mar 28, 2017

Out of interest, is there going to be any official Microsoft documentation around this? I'm somewhat nervous about starting to depend on a package provided only via a personal github repo rather than any official Microsoft support. The fact that the option is in VS is a good indicator that this does have MS support, but I suspect you'll find it's easier to get adoption if it were found in the dotnet org or similar. (.NET Foundation perhaps?)

@ctaggart
Copy link
Contributor Author

Yes, I'm interested in joining the .NET Foundation. I could use some help with this. The success of source linking is general depends on Microsoft adoption that is happening in the background right now of the aspnet MVC #5905. I've received feedback on 2.0 from Microsoft & GitLink devs that I'm trying to address for 2.1.

@jskeet
Copy link
Collaborator

jskeet commented Mar 28, 2017

Okay, I'm probably not the right person to help in general with this aspect, but if you need me to nudge anyone, just let me know.

@jskeet jskeet added priority: p2 Moderately-important priority. Fix may not be included in next release. type: process A process-related concern. May include testing, release, or the like. labels Jun 21, 2017
@jskeet
Copy link
Collaborator

jskeet commented Jul 5, 2017

@ctaggart: I've just started looking into this in a bit more detail, and I'm still pretty confused about:

  • Where the pdbs should end up
  • Whether this works for a net45 target as well as netstandard
  • What the current state of play is vs what's coming up

(I'd rather not change things multiple times.)

I'm doing experimentation in Noda Time rather than in this repo, as there's a smaller set of projects to consider there, but any clarifications you can give would be helpful.

@ctaggart
Copy link
Contributor Author

ctaggart commented Jul 5, 2017

@jskeet The adoption of source link should increase after Visual Studio 15.3 is released. Microsoft is making it work with the old Windows PDB format too, which will unblock them adopting it everywhere like ASP.NET. They have internal legacy systems that only work with the older format. They also now have a tool to convert between the two.

You should build Portable PDB files and package them in the NuGet packages. They are cross platform and work with net45 as well as netstandard. Use SourceLink.Create.CommandLine by following the quick start. There are several examples available linked from the issues.

@jskeet
Copy link
Collaborator

jskeet commented Jul 5, 2017

@ctaggart: Is that where things are headed, in a fairly definite way? Should we stop pushing symbol packages as well? (And are there plans to make dotnet pack include pdb files rather than having to manually craft the build file? I've done that for Noda Time, but it's ugly...)

@jskeet
Copy link
Collaborator

jskeet commented Jul 5, 2017

@ctaggart: Oh, and do I still need <IncludeSymbols>True</IncludeSymbols> in the build configuration? It doesn't look like I do (file sizes stay the same) but you know more about this than I do...

@ctaggart
Copy link
Contributor Author

ctaggart commented Jul 5, 2017

@jskeet Correct, <IncludeSymbols> isn't needed. http://blog.ctaggart.com/2017/03/enable-source-link-support-announcing.html

Yes, with portable pdb files, they are small enough that they can be packaged in the nuget packages. Yes, you can stop publishing symbol packages separately after including them in the nuget packages.

Including pdb files is a known issue. Follow the link to the issue for a couple of decent solutions.

jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 17, 2017
Fixes googleapis#923.

(Generated project files in next commit.)
@jskeet jskeet closed this as completed in 50b8dea Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
Development

No branches or pull requests

2 participants