Add sourcelink support to SDK projects #1800
Conversation
Adding package reference to Microsoft.SourceLink.GitHub
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
linux builds are failing. my bust guess is that the new package isn't netstandard compatible. we only need this for signed builds, so I'm testing a Windows condition.
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Hey @eddynaka,
All of this is good but I need you to make one additional change.
The Sourcelink package isn't compatible with our Linux builds.
I need you ta change the <ItemGroup> to <ItemGroup Condition=" $(OS) == 'Windows_NT'">.
This will unblock Linux builds but will allow our signing to incorporate the SourceLink.
I verified this change already on the Microsoft.ApplicationInsights.csproj. Can you please make this same change on the other csproj?
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Hey @eddynaka
the build nupkgs do not have the Source Link.
I built locally and my local builds don't have it either.
Please investigate when you get a chance. Thanks
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
I would not recommend adding SourceLink to each project individually. It's a pain to then update. Put the reference into a Directory.Build.props file. It works on all OS's. |
| <!-- indicate that the build executes on a build/CI server --> | ||
| <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild> | ||
| <!-- mapping all source paths included in the project outputs to deterministic values --> | ||
| <DeterministicSourcePaths>true</DeterministicSourcePaths> |
There was a problem hiding this comment.
No need to set this manually, this should be set by the targets
| <!--Build Infrastructure--> | ||
| <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0"> |
There was a problem hiding this comment.
The SourceLink reference should be added on all OS's
| <!--Build Infrastructure--> | ||
| <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0"> |
There was a problem hiding this comment.
The SourceLink reference should be added on all OS's
| <EmbedUntrackedSources>true</EmbedUntrackedSources> | ||
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup Condition="'$(BuildServer)' == 'true'"> |
There was a problem hiding this comment.
This should be at Directory.Build.props
|
A few things should be added to a Directory.Build.targets as well: |
Fix Issue #1760 .
Changes
Adding common properties to _nupkg.props;
Adding package reference to Microsoft.SourceLink.GitHub
Checklist
For significant contributions please make sure you have completed the following items:
The PR will trigger build, unit tests, and functional tests automatically. Please follow these instructions to build and test locally.
Notes for authors:
Notes for reviewers:
/AzurePipelines runwill queue all builds/AzurePipelines run <pipeline-name>will queue a specific build