Add support for emitting Source Link data #110270
Conversation
Fixes dotnet#81415. We read the source link data from the portable PDB, map file names to URLs, and emit into a JSON file. Then pass the JSON file to link.exe. We make no effort to make the source link data more compact. We could attempt to find common prefixes and emit as `*` but I left that as a future potential extension. The extra data in the expanded form is nothing compared to the size of native PDBs.
| /// <summary> | ||
| /// Source Link URL map. Maps file paths matching Source Link patterns to URLs. | ||
| /// </summary> | ||
| internal readonly struct SourceLinkMap |
There was a problem hiding this comment.
This file is basically https://github.com/dotnet/sourcelink/blob/928f459936292b3128b949957c7b46ae5ed59e4c/src/SourceLink.Tools/SourceLinkMap.cs, but fixed up not to trigger warnings.
We can't use this in the NuGet form because for whatever reason this is a source code nuget (doesn't have an assembly, it contains C# source code) that triggers tons of warnings in our build. So a fork it is.
There was a problem hiding this comment.
@tmat - would it be possible to update the original to comply with the runtime build environment? It would be obviously much cleaner to not have to fork it here.
There was a problem hiding this comment.
It would be preferable if it was not a source code nuget. This will be an ongoing problem in source form.
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
| @@ -87,6 +87,7 @@ The .NET Foundation licenses this file to you under the MIT license. | |||
| </ItemGroup> | |||
|
|
|||
| <ItemGroup> | |||
| <LinkerArg Condition="$(EnableSourceLink) == 'true'" Include="/SOURCELINK:$(NativeIntermediateOutputPath)%(ManagedBinary.Filename).sourcelink" /> | |||
There was a problem hiding this comment.
EnableSourceLink is from the SDK, right?
There was a problem hiding this comment.
Yup, if there's a property that controls this behavior for non-AOT cases, we tend to just reuse that. SDK already defined this.
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
Outdated
Show resolved
Hide resolved
| public override unsafe ReadOnlySpan<byte> GetSourceLinkData() | ||
| { | ||
| // {CC110556-A091-4D38-9FEC-25AB9A351A6A} | ||
| Guid sourceLinkDataGuid = new Guid(0xCC110556, 0xA091, 0x4D38, 0x9F, 0xEC, 0x25, 0xAB, 0x9A, 0x35, 0x1A, 0x6A); |
There was a problem hiding this comment.
Is it better to be a static read-only field?
There was a problem hiding this comment.
RyuJIT might already optimize this to constant data. If it doesn't, it doesn't matter anyway, this is not perf critical and it's only a couple instructions to initialize.
|
/ba-g infra signing issue on apple devices |
Fixes #81415.
We read the source link data from the portable PDB, map file names to URLs, and emit into a JSON file. Then pass the JSON file to link.exe. We make no effort to make the source link data more compact. We could attempt to find common prefixes and emit as
*but I left that as a future potential extension. The extra data in the expanded form is nothing compared to the size of native PDBs.Cc @dotnet/ilc-contrib