Skip to content

Commit

Permalink
[dotnet] add sourcelink support (#20054)
Browse files Browse the repository at this point in the history
Fixes #18968

We provide a mapping to the checked in source files via SourceLink.json
and the rest of the generated/untracked sources are embedded into the
PDB to provide a more comprehensive debugging experience. Since we
invoke CSC directly, there were a few workarounds that had to be
implemented (ex: implementing a helper script to account for untracked
sources instead of simply using the EmbedUntrackedSources MSBuild
property).

As for testing, the newly added support was validated via the dotnet
sourcelink tool which confirmed all the sources in the PDB either had
valid urls or were embedded.

`sourcelink test Microsoft.MacCatalyst.pdb` —> `sourcelink test passed:
Microsoft.MacCatalyst.pdb`

The PDB size does increase in size after embedding;
Microsoft.MacCatalyst.pdb went from 5 MB to 15.7 MB.

But considering it would significantly help improve the debugging
experience, be consistent with Android’s offerings, and it’s a
highlighted attribute on the NuGet package explorer I think it’s a
worthy size increase.

Refs:
xamarin/xamarin-android#7298 
dotnet/roslyn#12625
https://github.com/dotnet/sourcelink/tree/main/docs

---------

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
Co-authored-by: Alex Soto <alex@alexsoto.me>
Co-authored-by: Michael Cummings (MSFT) <mcumming@microsoft.com>
Co-authored-by: GitHub Actions Autoformatter <github-actions-autoformatter@xamarin.com>
  • Loading branch information
5 people committed Feb 26, 2024
1 parent caf8517 commit 297f12d
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 3 deletions.
11 changes: 11 additions & 0 deletions src/Makefile
Expand Up @@ -1213,6 +1213,13 @@ dotnet-gen:: dotnet-gen-$(3)
$($(2)_DOTNET_BUILD_DIR)/ILLink.LinkAttributes.xml: $(TOP)/src/ILLink.LinkAttributes.xml.in | $($(2)_DOTNET_BUILD_DIR)
$$(call Q_PROF_GEN,$(3)) sed < $$< > $$@ 's|@PRODUCT_NAME@|Microsoft.$(1)|g;'

$($(2)_DOTNET_BUILD_DIR)/SourceLink.json: $($(2)_DOTNET_BUILD_DIR)
$$(Q) $(TOP)/src/generate-sourcelink-json.csharp "$(PACKAGE_HEAD_REV)" "$(abspath $(TOP)/src)" "$$@"

$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp: $($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources $($(2)_DOTNET_SOURCES) $(TOP)/src/generate-embed-files.sh
$$(Q) $(TOP)/src/generate-embed-files.sh $($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources "$($(2)_DOTNET_SOURCES)" > $$@.tmp
$$(Q) mv $$@.tmp $$@

$($(2)_DOTNET_BUILD_DIR)/ILLink.Substitutions.xml: $(TOP)/src/ILLink.Substitutions.$(1).xml | $($(2)_DOTNET_BUILD_DIR)
$(Q) $(CP) $$< $$@

Expand Down Expand Up @@ -1259,6 +1266,8 @@ endif
$(2)_DOTNET_PLATFORM_ASSEMBLY_DEPENDENCIES = \
$($(2)_DOTNET_SOURCES) \
$($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources \
$($(2)_DOTNET_BUILD_DIR)/SourceLink.json \
$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp \
$($(2)_DOTNET_BUILD_DIR)/ILLink.LinkAttributes.xml \
$($(2)_DOTNET_BUILD_DIR)/ILLink.Substitutions.xml \
$(PRODUCT_KEY_PATH) \
Expand All @@ -1277,6 +1286,8 @@ $($(2)_DOTNET_BUILD_DIR)/$(4)/Microsoft.$(1)%dll $($(2)_DOTNET_BUILD_DIR)/$(4)/M
-publicsign -keyfile:$(PRODUCT_KEY_PATH) \
$$($(2)_$(4)_REFOUT_ARG) \
$$($(2)_$(4)_DOC_ARG) \
-sourcelink:$($(2)_DOTNET_BUILD_DIR)/SourceLink.json \
@$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp \
$$($(2)_DEFINES) \
$(ARGS_$(4)) \
$$(DOTNET_WARNINGS_TO_FIX) \
Expand Down
15 changes: 15 additions & 0 deletions src/generate-embed-files.sh
@@ -0,0 +1,15 @@
#!/bin/bash -e

gen_sources="$1"
dotnet_sources="$2"
IFS=$'\n'
arr=($(<$gen_sources))
IFS=' '
read -ra sources_array <<< "$dotnet_sources"
arr+=("${sources_array[@]}")
git_files=($(git ls-files))
IFS=$'\n'
filtered_files=($(printf "%s\n" "${arr[@]}" | grep -vF "${git_files[*]}"))
IFS=','
echo "-embed:${filtered_files[*]}"
unset IFS
20 changes: 20 additions & 0 deletions src/generate-sourcelink-json.csharp
@@ -0,0 +1,20 @@
#!/usr/bin/env /Library/Frameworks/Mono.framework/Commands/csharp -s

using System.IO;
using System.Text;

var args = Args;
var idx = 0;
var latestCommit = args [idx++];
var src = args [idx++];
var outputPath = args [idx++];

using (var writer = new StreamWriter (outputPath)) {
writer.WriteLine ("{");
writer.WriteLine (" \"documents\": {");
writer.WriteLine ($" \"{src}*\": \"https://raw.githubusercontent.com/xamarin/xamarin-macios/{latestCommit}/src*\"");
writer.WriteLine (" }");
writer.WriteLine ("}");
}

Environment.Exit(0)
29 changes: 29 additions & 0 deletions tests/common/DotNet.cs
Expand Up @@ -113,6 +113,35 @@ public static ExecutionResult AssertNew (string outputDirectory, string template
return new ExecutionResult (output, output, rv.ExitCode);
}

public static ExecutionResult InstallTool (string tool, string path)
{
var installed = ExecuteCommand (Executable, "tool", "list", "--tool-path", path);
if (!installed.StandardOutput.ToString ().Contains (tool))
installed = ExecuteCommand (Executable, "tool", "install", tool, "--tool-path", path);
return installed;
}

public static ExecutionResult RunTool (string tool, params string [] args) => ExecuteCommand (tool, args);

public static ExecutionResult ExecuteCommand (string exe, params string [] args)
{
var env = new Dictionary<string, string?> ();
env ["MSBuildSDKsPath"] = null;
env ["MSBUILD_EXE_PATH"] = null;

var output = new StringBuilder ();
var rv = Execution.RunWithStringBuildersAsync (exe, args, env, output, output, Console.Out, workingDirectory: Configuration.SourceRoot, timeout: TimeSpan.FromMinutes (10)).Result;
if (rv.ExitCode != 0) {
var msg = new StringBuilder ();
msg.AppendLine ($"'{exe}' failed with exit code {rv.ExitCode}");
msg.AppendLine ($"Full command: {Executable} {StringUtils.FormatArguments (args)}");
msg.AppendLine (output.ToString ());
Console.WriteLine (msg);
Assert.Fail (msg.ToString ());
}
return new ExecutionResult (output, output, rv.ExitCode);
}

public static ExecutionResult Execute (string verb, string project, Dictionary<string, string>? properties, bool assert_success = true, string? target = null, bool? msbuildParallelism = null, TimeSpan? timeout = null)
{
if (!File.Exists (project))
Expand Down
11 changes: 11 additions & 0 deletions tests/common/TestRuntime.cs
Expand Up @@ -136,6 +136,17 @@ public static Version GetSDKVersion ()
}
}

static bool? is_pull_request;
public static bool IsPullRequest {
get {
if (!is_pull_request.HasValue) {
var pr = string.Equals (Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal);
is_pull_request = pr;
}
return is_pull_request.Value;
}
}

public static void IgnoreInCI (string message)
{
if (!IsInCI) {
Expand Down
37 changes: 34 additions & 3 deletions tests/dotnet/UnitTests/ProjectTest.cs
@@ -1,11 +1,8 @@
using System.Runtime.InteropServices;
using System.Diagnostics;
using System.Xml;

using Mono.Cecil;

using Xamarin.Tests;

#nullable enable

namespace Xamarin.Tests {
Expand Down Expand Up @@ -1666,5 +1663,39 @@ public void StrippedRuntimeIdentifiers (ApplePlatform platform, string runtimeId
Assert.That (symbols, Does.Contain ("_xamarin_release_managed_ref"), "_xamarin_release_managed_ref");
}

[Test]
[TestCase (ApplePlatform.iOS, "ios-arm64;", "iOS")]
[TestCase (ApplePlatform.MacOSX, "osx-arm64;osx-x64", "macOS")]
[TestCase (ApplePlatform.MacCatalyst, "maccatalyst-x64", "MacCatalyst")]
[TestCase (ApplePlatform.TVOS, "tvos-arm64;", "tvOS")]
public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers, string platformName)
{
// Sourcelink uses the latest commit and tests to see if
// it is valid which will not pass until the commit has
// been merged in and actually exists on github.

if (!IsInCI || IsPullRequest)
Assert.Ignore ("This test is disabled for local runs and Pull Requests.");

var project = "MySimpleApp";

var project_path = GetProjectPath (project, runtimeIdentifiers: runtimeIdentifiers, platform: platform, out var appPath);
Clean (project_path);
var properties = GetDefaultProperties (runtimeIdentifiers);
DotNet.AssertBuild (project_path, properties);

var pdbFile = Directory
.GetFiles (Path.GetDirectoryName (project_path)!, $"Microsoft.{platformName}.pdb", SearchOption.AllDirectories)
.FirstOrDefault ();

Assert.NotNull (pdbFile, "No PDB file found");

var tool = "sourcelink";
var toolPath = Directory.GetCurrentDirectory ();
DotNet.InstallTool (tool, toolPath);
var test = DotNet.RunTool (Path.Combine (toolPath, tool), "test", pdbFile!);

Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ToString ());
}
}
}
23 changes: 23 additions & 0 deletions tests/dotnet/UnitTests/TestBaseClass.cs
Expand Up @@ -465,5 +465,28 @@ public void AssertThatLinkerDidNotExecute (ExecutionResult result)
Assert.That (output, Does.Not.Contain ("LinkerConfiguration:"), "Custom steps did not run as expected.");
}

static bool? is_in_ci;
public static bool IsInCI {
get {
if (!is_in_ci.HasValue) {
var in_ci = !string.IsNullOrEmpty (Environment.GetEnvironmentVariable ("BUILD_REVISION"));
in_ci |= !string.IsNullOrEmpty (Environment.GetEnvironmentVariable ("BUILD_SOURCEVERSION")); // set by Azure DevOps
is_in_ci = in_ci;
}
return is_in_ci.Value;
}
}

static bool? is_pull_request;
public static bool IsPullRequest {
get {
if (!is_pull_request.HasValue) {
var pr = string.Equals (Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal);
is_pull_request = pr;
}
return is_pull_request.Value;
}
}

}
}

8 comments on commit 297f12d

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Please sign in to comment.