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

Add source link #158

Merged
merged 3 commits into from Mar 14, 2019

Conversation

Projects
None yet
3 participants
@jtattermusch
Copy link
Contributor

commented Mar 13, 2019

Fixes #131

After applying this change, the resulting .nupkg file contains the PDB.

https://pantheon.corp.google.com/storage/browser/grpc-testing-kokoro-prod/test_result_public/prod/grpc/dotnet/master/linux/767/20190313-012403/github/grpc-dotnet/artifacts/

According the https://github.com/dotnet/sourcelink/, the sourcelink support should now be included in the dotnet SDK - we can experiment with it, but changes in this PR should work too (and it's what many other packages do).

@JamesNK
Copy link
Contributor

left a comment

I couldn't get Source Link to work with Grpc.Core.Api which uses the old Source Link package.

You should use the new Source Link package - https://github.com/dotnet/sourcelink/#using-source-link-in-net-projects

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <PublishRepositoryUrl>true</PublishRepositoryUrl>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" PrivateAssets="All"/>
  </ItemGroup>
</Project>
@jtattermusch

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

I couldn't get Source Link to work with Grpc.Core.Api which uses the old Source Link package.

Which nuget package version did you use for testing? The fix for Grpc.* sourceLink grpc/grpc#18333 was only merged very recently.

You should use the new Source Link package - https://github.com/dotnet/sourcelink/#using-source-link-in-net-projects

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <PublishRepositoryUrl>true</PublishRepositoryUrl>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" PrivateAssets="All"/>
  </ItemGroup>
</Project>

jtattermusch added some commits Mar 13, 2019

@jtattermusch jtattermusch force-pushed the jtattermusch:add_sourcelink branch from f1f807c to 09e4581 Mar 14, 2019

@jtattermusch

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

@jtattermusch

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

@JamesNK PTAL

@JunTaoLuo
Copy link
Contributor

left a comment

LGTM, this is the same version we use in our repos.

@JamesNK
Copy link
Contributor

left a comment

It looks good to me BUT I wasn't able to successfully test it in .NET Core 3.0. It looks like .NET Core 3.0 doesn't copy the PDBs embedded in the package to the build directory.

I know the .NET team is encouraging the use of source packages instead of embedded PDBs but it seems odd that they have stopped working altogether. I've internally reached out to the Source Link expert for some advice.

@jtattermusch

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

I'm going to merge for now and we can followup once we know more.

@jtattermusch jtattermusch merged commit d5bfbf3 into grpc:master Mar 14, 2019

2 checks passed

cla/linuxfoundation jtattermusch authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@JamesNK JamesNK referenced this pull request Mar 14, 2019

Closed

Enable Source Link #132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.