[dotnet] add sourcelink support #20054

haritha-mohan · 2024-02-07T04:40:36Z

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:
dotnet/android#7298
dotnet/roslyn#12625
https://github.com/dotnet/sourcelink/tree/main/docs

Fixes xamarin#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 sourcelink dotnet 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; for example, 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: dotnet/android#7298 dotnet/roslyn#12625 https://github.com/dotnet/sourcelink/tree/main/docs

rolfbjarne

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

Jon Pryor mentioned this takes a while to run: how long does it take for you?

It might make sense to create a test to ensure we don't accidentally forget to embed files in the pdb in the future.

src/generate-sourcelink-json.csharp

src/Makefile

rolfbjarne · 2024-02-07T08:27:23Z

src/Makefile

@@ -1213,6 +1213,12 @@ 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)" "$$@" >/dev/null 2>&1


While I really like quiet build output, piping it all to /dev/null makes diagnosing failures hard... typically I try to not create the output in the first place. Could you explain the exact output you're getting here?

@rolfbjarne it's printing out the contents of the json file, but i'll explore options on how to prevent that some other way.

@rolfbjarne opted to use a stream writer as opposed to a string builder so now the build output is a bit less verbose without manually piping it all away!

rolfbjarne · 2024-02-07T08:42:35Z

src/generate-embed-files.sh

@@ -0,0 +1,28 @@
+#!/bin/bash -e
+
+find_files() {


I have a thought: I believe these two variables will contain/list all the source files:

$$($(2)_DOTNET_SOURCES) @$($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources

xamarin-macios/src/Makefile

Lines 1288 to 1289 in 02947b2

$$($(2)_DOTNET_SOURCES) \

@$($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources \

So maybe use a very different implementation: create a list of all those files, and then exclude any files in git, which I think should be the list you want to pass as -embed:?

You can get the files in git by doing git ls-files.

This seems somewhat more future-proof in that it's less likely for us to forget to update this script if we add more custom-generated files to the build.

Co-authored-by: Rolf Bjarne Kvinge <[email protected]>

haritha-mohan · 2024-02-07T17:55:11Z

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

Jon Pryor mentioned this takes a while to run: how long does it take for you?

It might make sense to create a test to ensure we don't accidentally forget to embed files in the pdb in the future.

it's not bad:
sourcelink test Microsoft.iOS.pdb 4.12s user 0.85s system 122% cpu 4.067 total

sure, i can create a test!

rolfbjarne · 2024-02-23T10:19:35Z

tests/dotnet/UnitTests/Extensions.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Diagnostics;


There are no other changes in this file, so I don't think you need to add a using.

rolfbjarne · 2024-02-23T10:20:49Z

tests/dotnet/UnitTests/ProjectTest.cs

+			var tool = "sourcelink";
+			var toolPath = Directory.GetCurrentDirectory ();
+			DotNet.InstallTool (tool, toolPath);
+			var test = DotNet.RunTool ($"{toolPath}/{tool}", "test", $"{pdbFile}");


Suggested change

var test = DotNet.RunTool ($"{toolPath}/{tool}", "test", $"{pdbFile}");

var test = DotNet.RunTool (Path.Combine (toolPath, tool), "test", pdbFile);

tests/common/DotNet.cs

vs-mobiletools-engineering-service2 · 2024-02-23T19:14:59Z

🔥 Failed to compute test summaries on VSTS: test results 🔥

Failed to compute test summaries: Cannot process command because of one or more missing mandatory parameters: TestPrefix Dependencies..

Pipeline on Agent
Merge b011e65 into 8f868e6

vs-mobiletools-engineering-service2 · 2024-02-24T02:27:38Z

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T02:28:49Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 246cff5bf6a1da85803bf4b74651d7a8e3809db6 [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T02:29:30Z

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T02:30:17Z

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T02:30:44Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T02:31:14Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T03:05:04Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

~~iOS~~ (no change detected)
~~tvOS~~ (no change detected)
~~watchOS~~ (no change detected)
~~macOS~~ (no change detected)

NET (empty diffs)

~~iOS~~: (empty diff detected)
~~tvOS~~: (empty diff detected)
~~MacCatalyst~~: (empty diff detected)
~~macOS~~: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 246cff5bf6a1da85803bf4b74651d7a8e3809db6 [PR build]

vs-mobiletools-engineering-service2 · 2024-02-24T03:47:50Z

🔥 [CI Build] Test results 🔥

Test results

❌ Tests failed on VSTS: test results

0 tests crashed, 1 tests failed, 169 tests passed.

Failures

❌ monotouch tests (MacCatalyst)

1 tests failed, 6 tests passed.

monotouch-test/Mac Catalyst [dotnet]/Release (NativeAOT) [dotnet]: TimedOut (Execution timed out after 1200 seconds.
No test log file was produced)

Html Report (VSDrops) Download

Successes

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ install-source: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 7 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac-binding-project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (watchOS): All 4 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 246cff5bf6a1da85803bf4b74651d7a8e3809db6 [PR build]

Follow up to #20054

haritha-mohan requested a review from rolfbjarne as a code owner February 7, 2024 04:40

rolfbjarne requested changes Feb 7, 2024

View reviewed changes

haritha-mohan and others added 3 commits February 7, 2024 09:17

Apply suggestions from code review

f194d18

Co-authored-by: Rolf Bjarne Kvinge <[email protected]>

Merge branch 'main' into sourcelink

540435c

use global Args var for c# script

82ef09d

This comment has been minimized.

Sign in to view

haritha-mohan and others added 2 commits February 9, 2024 19:34

change script to be more maintainable

a148589

Merge branch 'main' into sourcelink

4e6990b

This comment has been minimized.

Sign in to view

haritha-mohan and others added 4 commits February 20, 2024 22:37

use streamwriter for quieter build output

42fa8f1

Merge branch 'main' into sourcelink

517e842

add test

a27d5f2

ignore test locally/in PRs

471c03b

This comment has been minimized.

Sign in to view

rolfbjarne requested changes Feb 23, 2024

View reviewed changes

address review

b011e65

haritha-mohan requested a review from rolfbjarne February 23, 2024 15:54

This comment has been minimized.

Sign in to view

Merge branch 'main' into sourcelink

246cff5

rolfbjarne approved these changes Feb 26, 2024

View reviewed changes

haritha-mohan merged commit 297f12d into xamarin:main Feb 26, 2024
61 of 63 checks passed

haritha-mohan deleted the sourcelink branch February 26, 2024 16:10

haritha-mohan mentioned this pull request Feb 27, 2024

[dotnet] Fix sourcelink test failure #20205

Merged

rolfbjarne pushed a commit that referenced this pull request Feb 27, 2024

[dotnet] Fix sourcelink test failure (#20205)

4409a06

Follow up to #20054

	$$($(2)_DOTNET_SOURCES) \
	@$($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources \

	var test = DotNet.RunTool ($"{toolPath}/{tool}", "test", $"{pdbFile}");
	var test = DotNet.RunTool (Path.Combine (toolPath, tool), "test", pdbFile);

		@@ -0,0 +1,28 @@
		#!/bin/bash -e

		find_files() {

[dotnet] add sourcelink support #20054

[dotnet] add sourcelink support #20054

Conversation

haritha-mohan commented Feb 7, 2024

rolfbjarne left a comment

Choose a reason for hiding this comment

rolfbjarne Feb 7, 2024

Choose a reason for hiding this comment

haritha-mohan Feb 8, 2024

Choose a reason for hiding this comment

haritha-mohan Feb 21, 2024

Choose a reason for hiding this comment

rolfbjarne Feb 7, 2024

Choose a reason for hiding this comment

haritha-mohan commented Feb 7, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne Feb 23, 2024

Choose a reason for hiding this comment

rolfbjarne Feb 23, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Feb 23, 2024

🔥 Failed to compute test summaries on VSTS: test results 🔥

vs-mobiletools-engineering-service2 commented Feb 24, 2024

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

vs-mobiletools-engineering-service2 commented Feb 24, 2024

💻 [CI Build] Windows Integration Tests passed 💻

vs-mobiletools-engineering-service2 commented Feb 24, 2024

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

vs-mobiletools-engineering-service2 commented Feb 24, 2024

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

vs-mobiletools-engineering-service2 commented Feb 24, 2024

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11) passed 💻

vs-mobiletools-engineering-service2 commented Feb 24, 2024

📚 [PR Build] Artifacts 📚

Packages generated

vs-mobiletools-engineering-service2 commented Feb 24, 2024

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

vs-mobiletools-engineering-service2 commented Feb 24, 2024

🔥 [CI Build] Test results 🔥

Test results

Failures

❌ monotouch tests (MacCatalyst)

Successes