Azure / Microsoft.Azure.StackExchangeRedis

Azure-specific wrapper for the StackExchange.Redis client library
MIT License
17 stars 14 forks source link

Invalid PDB symbols with SourceLink, non-deterministic build #39

Closed tompazourek closed 5 months ago

tompazourek commented 11 months ago

Issue description

When viewing the NuGet package (2.0.0) via NuGet Package Explorer, I see this:

image

I also get an error when debugging process dumps in Visual Studio:

image

Source Link data for Microsoft.Azure.StackExchangeRedis.dll (embedded) failed to parse or does not conform to the schema: {"documents":{}}

Untracked sources fix

I think the untracked sources (generated AssemblyInfo) warning is only minor, and can be resolved by adding an MSBuild property:

<EmbedUntrackedSources>true</EmbedUntrackedSources>

Non-deterministic paths fix for SourceLink

The bigger issue is the non-deterministic build where the source file paths in the embedded symbols are not normalized, and thus don't work with SourceLink.

As seen via ILSpy:

image

The paths shouldn't be real disk paths like C:\..., the "normalized" paths should start with /_/.

This can be achieved by passing MSBuild property:

<ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>

But this property should only be used for non-local-development builds as it affects debugging. A good way might be to pass it as -p:ContinuousIntegrationBuild=true in a build pipeline, or alternatively make it conditional based on some environment variable etc:

<PropertyGroup Condition="'$(TF_BUILD)'=='true'">
  <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
</PropertyGroup>

I would contribute the fix, but it seems that the build pipelines are unfortunately not open-source, so I cannot confirm it will work.

philon-msft commented 11 months ago

Thanks for reporting this @tompazourek. We'll work on a fix for the next release.

philon-msft commented 6 months ago

Apologies, I forgot to address this in the v3.0.0. We'll keep this issue active to fix in the next release.

philon-msft commented 5 months ago

Should be fixed by #54. Let us know if you still see any issues.