Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

Any reason you've removed the global.json? It's there to pin to a specific version of the SDK. Change looks fine apart from that.

Yes, AllowedOutputExtensionsInPackageBuildOutputFolder is used to include the pdb in the nupkg. It only works with SDK > 2.0.2 I believe.

Okay - if we need SDK 2.0.2, I expect we'll need to change a bit more. In particular, rather than removing global.json we should require the appropriate SDK... and then we'll need some changes to the CI configs to make sure we include both the right SDK for building and the right runtime for testing. I suggest we separate that work from the SourceLink part, in terms of PRs. (I can't get to anything this week as I'm at a conference, but I could potentially look next week.)

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

(Sorry, closure was fumbling on my part.)

@jskeet I put back the global.json as requested. You need to ship the source link enabled pdb files in the nupkg for this pull request to be meaningful. You can simply do that by building with a more recent SDK.

For example, I removed the global.json so that it used the default SDK 2.1.3 on AppVeyor from my pdb branch:
https://ci.appveyor.com/project/ctaggart/protobuf/build/8
https://www.myget.org/feed/Packages/sourcelink

@ctaggart: Right - I'll create a new PR to require a specific SDK that will work. I believe Travis would fail at the moment - and I don't want to rely on a default SDK version at all.

@jskeet dotnet 2.1.3 may be a good choice for Travis. That is what I used for netmq:
https://github.com/zeromq/netmq/blob/master/.travis.yml

I've now merged the SDK change (2.0.3 in the end) so if you rebase your change, we can take another look.

@jskeet The build failure on Travis was:

The command "sudo apt-get install -qq dotnet-sdk-2.0.3" failed and exited with 100 during .

Hmm... that's confusing, given that it worked on the PR itself. And that was all the detail you saw?

You can click through to see the build failure details:
https://travis-ci.org/google/protobuf/jobs/333428487

I merged master in again, so let's see if it works this time.

Ah - this is the root cause:

W: Failed to fetch https://packages.microsoft.com/repos/microsoft-ubuntu-trusty-prod/dists/trusty/InRelease Connection timed out after 10001 milliseconds

I suspect this will be a transient error.

Oops. Yeah, I should have noticed that.

@jskeet yes, they pass now. Ready to squash merge?

I'm happy, but we should double check with someone actively on the Protobuf team before merging.

@anandolee Are you comfortable with this change? Not that now we won't get a symbols NuGet package when we pack the release, deliberately.

(@ctaggart: The "can't rebase" is slightly worrying, mind you - could you rebase manually from master down to one commit and force push? That way we won't even need to squash...)

Okay, I redid the same changes and force pushed. You should check that the nupkg has the pdb files in it. If you put it somewhere, I could run dotnet sourcelink test on the nupkg to verify that the source link info is all working.

ping @jskeet

It's fine by me, but I'm not on the team - @anandolee or @xfxyjwf should approve and merge.

Ping @anandolee @xfxyjwf. Anything preventing this from being merged?

@anandolee Jie, can you review and merge this pull request if it seems good to you?

Ping @xfxyjwf @anandolee. Can you review or just merge it?