From 55125c245bce60d4c59077e2741769b0058cba64 Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Tue, 6 Feb 2024 20:31:44 -0800 Subject: [PATCH 01/12] [dotnet] Add sourcelink support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/xamarin/xamarin-macios/issues/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: https://github.com/xamarin/xamarin-android/pull/7298 https://github.com/dotnet/roslyn/issues/12625 https://github.com/dotnet/sourcelink/tree/main/docs --- src/Makefile | 10 ++++++++++ src/generate-embed-files.sh | 28 ++++++++++++++++++++++++++++ src/generate-sourcelink-json.csharp | 25 +++++++++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100755 src/generate-embed-files.sh create mode 100755 src/generate-sourcelink-json.csharp diff --git a/src/Makefile b/src/Makefile index 1063650ce25a..81b73d55c5b1 100644 --- a/src/Makefile +++ b/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 + +$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp: $($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources + $$(Q) $(TOP)/src/generate-embed-files.sh $($(2)_DOTNET_BUILD_DIR) $(3) > $$@ + $($(2)_DOTNET_BUILD_DIR)/ILLink.Substitutions.xml: $(TOP)/src/ILLink.Substitutions.$(1).xml | $($(2)_DOTNET_BUILD_DIR) $(Q) $(CP) $$< $$@ @@ -1259,6 +1265,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) \ @@ -1277,6 +1285,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) \ diff --git a/src/generate-embed-files.sh b/src/generate-embed-files.sh new file mode 100755 index 000000000000..45397ce51a66 --- /dev/null +++ b/src/generate-embed-files.sh @@ -0,0 +1,28 @@ +#!/bin/bash -e + +find_files() { + find "$(realpath "$1")" -type f -name "$2" -exec printf "%s," {} + +} + +build_dir="$1" +platform="$2" + +if [ "$platform" = "macos" ]; then + modified_platform="mac" +else + modified_platform="$platform" +fi + +root="$(realpath "$build_dir/../../../..")" + +# find generated/untracked files +# impt to include, otherwise sourcelink test will fail (don't have a valid source file to link to) +files=$(find_files "$build_dir" "*.g.cs") +files+=$(find_files "$root/src/build/common" "*.cs") +files+=$(find_files "$root/src/build/$modified_platform" "AssemblyInfo.cs") +files+=$(find_files "$root/src/build/$modified_platform" "Constants.cs") +files+=$(find_files "$root/src/build/dotnet" "Constants.$platform.generated.cs") +files+=$(find_files "$root/runtime" "*generated.cs") +files+=$(find_files "$root/src" "MinimumVersions.cs") +files="-embed:${files%,}" +echo $files diff --git a/src/generate-sourcelink-json.csharp b/src/generate-sourcelink-json.csharp new file mode 100755 index 000000000000..5581cb6fe353 --- /dev/null +++ b/src/generate-sourcelink-json.csharp @@ -0,0 +1,25 @@ +#!/usr/bin/env /Library/Frameworks/Mono.framework/Commands/csharp + +using System.IO; +using System.Text; + +var args = Environment.GetCommandLineArgs (); + +var latestCommit = args [2]; +var src = args [3]; +var outputPath = args [4]; + +var json = new StringBuilder (); +json.AppendLine ("{"); +json.AppendLine (" \"documents\": {"); + +json.AppendLine ($" \"{src}*\": \"https://raw.githubusercontent.com/xamarin/xamarin-macios/{latestCommit}/src/*\""); +json.AppendLine (" }"); +json.AppendLine ("}"); + +string outputData = json.ToString (); +using (StreamWriter writer = new StreamWriter(outputPath)) +{ + writer.Write(outputData); +} + From f194d18de6cd0cd6b263d812d0ca02d3c27f2ad8 Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Wed, 7 Feb 2024 09:17:56 -0800 Subject: [PATCH 02/12] Apply suggestions from code review Co-authored-by: Rolf Bjarne Kvinge --- src/Makefile | 5 +++-- src/generate-sourcelink-json.csharp | 8 ++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Makefile b/src/Makefile index 81b73d55c5b1..bdaf51b0f53f 100644 --- a/src/Makefile +++ b/src/Makefile @@ -1216,8 +1216,9 @@ $($(2)_DOTNET_BUILD_DIR)/ILLink.LinkAttributes.xml: $(TOP)/src/ILLink.LinkAttrib $($(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 -$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp: $($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources - $$(Q) $(TOP)/src/generate-embed-files.sh $($(2)_DOTNET_BUILD_DIR) $(3) > $$@ +$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp: $($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources $(TOP)/src/generate-embed-files.sh + $$(Q) $(TOP)/src/generate-embed-files.sh $($(2)_DOTNET_BUILD_DIR) $(3) > $$@.tmp + $$(Q) mv $$@.tmp $$@ $($(2)_DOTNET_BUILD_DIR)/ILLink.Substitutions.xml: $(TOP)/src/ILLink.Substitutions.$(1).xml | $($(2)_DOTNET_BUILD_DIR) $(Q) $(CP) $$< $$@ diff --git a/src/generate-sourcelink-json.csharp b/src/generate-sourcelink-json.csharp index 5581cb6fe353..525a77797ce9 100755 --- a/src/generate-sourcelink-json.csharp +++ b/src/generate-sourcelink-json.csharp @@ -9,7 +9,7 @@ var latestCommit = args [2]; var src = args [3]; var outputPath = args [4]; -var json = new StringBuilder (); +var json = new StringBuilder (); json.AppendLine ("{"); json.AppendLine (" \"documents\": {"); @@ -17,9 +17,5 @@ json.AppendLine ($" \"{src}*\": \"https://raw.githubusercontent.com/xamarin/x json.AppendLine (" }"); json.AppendLine ("}"); -string outputData = json.ToString (); -using (StreamWriter writer = new StreamWriter(outputPath)) -{ - writer.Write(outputData); -} +File.WriteAllText (outputPath, json.ToString ()); From 82ef09d4d456b3e41e02116e6f7e7730f075c1c4 Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Wed, 7 Feb 2024 09:47:45 -0800 Subject: [PATCH 03/12] use global Args var for c# script --- src/generate-sourcelink-json.csharp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/generate-sourcelink-json.csharp b/src/generate-sourcelink-json.csharp index 525a77797ce9..36ae5bb683b8 100755 --- a/src/generate-sourcelink-json.csharp +++ b/src/generate-sourcelink-json.csharp @@ -1,13 +1,13 @@ -#!/usr/bin/env /Library/Frameworks/Mono.framework/Commands/csharp +#!/usr/bin/env /Library/Frameworks/Mono.framework/Commands/csharp -s using System.IO; using System.Text; -var args = Environment.GetCommandLineArgs (); - -var latestCommit = args [2]; -var src = args [3]; -var outputPath = args [4]; +var args = Args; +var idx = 0; +var latestCommit = args [idx++]; +var src = args [idx++]; +var outputPath = args [idx++]; var json = new StringBuilder (); json.AppendLine ("{"); From a1485898383c8b42650e2a63452f37b8851a3bae Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Thu, 8 Feb 2024 10:41:37 -0800 Subject: [PATCH 04/12] change script to be more maintainable --- src/Makefile | 4 +-- src/generate-embed-files.sh | 39 ++++++++++------------------- src/generate-sourcelink-json.csharp | 3 +-- 3 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/Makefile b/src/Makefile index bdaf51b0f53f..bc58593147e6 100644 --- a/src/Makefile +++ b/src/Makefile @@ -1216,8 +1216,8 @@ $($(2)_DOTNET_BUILD_DIR)/ILLink.LinkAttributes.xml: $(TOP)/src/ILLink.LinkAttrib $($(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 -$($(2)_DOTNET_BUILD_DIR)/embed-files.rsp: $($(2)_DOTNET_BUILD_DIR)/$(3)-generated-sources $(TOP)/src/generate-embed-files.sh - $$(Q) $(TOP)/src/generate-embed-files.sh $($(2)_DOTNET_BUILD_DIR) $(3) > $$@.tmp +$($(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) diff --git a/src/generate-embed-files.sh b/src/generate-embed-files.sh index 45397ce51a66..34878c2b0bd7 100755 --- a/src/generate-embed-files.sh +++ b/src/generate-embed-files.sh @@ -1,28 +1,15 @@ #!/bin/bash -e -find_files() { - find "$(realpath "$1")" -type f -name "$2" -exec printf "%s," {} + -} - -build_dir="$1" -platform="$2" - -if [ "$platform" = "macos" ]; then - modified_platform="mac" -else - modified_platform="$platform" -fi - -root="$(realpath "$build_dir/../../../..")" - -# find generated/untracked files -# impt to include, otherwise sourcelink test will fail (don't have a valid source file to link to) -files=$(find_files "$build_dir" "*.g.cs") -files+=$(find_files "$root/src/build/common" "*.cs") -files+=$(find_files "$root/src/build/$modified_platform" "AssemblyInfo.cs") -files+=$(find_files "$root/src/build/$modified_platform" "Constants.cs") -files+=$(find_files "$root/src/build/dotnet" "Constants.$platform.generated.cs") -files+=$(find_files "$root/runtime" "*generated.cs") -files+=$(find_files "$root/src" "MinimumVersions.cs") -files="-embed:${files%,}" -echo $files +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 \ No newline at end of file diff --git a/src/generate-sourcelink-json.csharp b/src/generate-sourcelink-json.csharp index 36ae5bb683b8..2e665eefc422 100755 --- a/src/generate-sourcelink-json.csharp +++ b/src/generate-sourcelink-json.csharp @@ -12,8 +12,7 @@ var outputPath = args [idx++]; var json = new StringBuilder (); json.AppendLine ("{"); json.AppendLine (" \"documents\": {"); - -json.AppendLine ($" \"{src}*\": \"https://raw.githubusercontent.com/xamarin/xamarin-macios/{latestCommit}/src/*\""); +json.AppendLine ($" \"{src}*\": \"https://raw.githubusercontent.com/xamarin/xamarin-macios/{latestCommit}/src*\""); json.AppendLine (" }"); json.AppendLine ("}"); From 42fa8f1ee1598f98653f395f5bdada2aed86e5a3 Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Tue, 20 Feb 2024 22:37:36 -0800 Subject: [PATCH 05/12] use streamwriter for quieter build output --- src/Makefile | 2 +- src/generate-sourcelink-json.csharp | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Makefile b/src/Makefile index 6c4cfa5041a6..90134368714a 100644 --- a/src/Makefile +++ b/src/Makefile @@ -1214,7 +1214,7 @@ $($(2)_DOTNET_BUILD_DIR)/ILLink.LinkAttributes.xml: $(TOP)/src/ILLink.LinkAttrib $$(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 + $$(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 diff --git a/src/generate-sourcelink-json.csharp b/src/generate-sourcelink-json.csharp index 2e665eefc422..965628a5278d 100755 --- a/src/generate-sourcelink-json.csharp +++ b/src/generate-sourcelink-json.csharp @@ -9,12 +9,12 @@ var latestCommit = args [idx++]; var src = args [idx++]; var outputPath = args [idx++]; -var json = new StringBuilder (); -json.AppendLine ("{"); -json.AppendLine (" \"documents\": {"); -json.AppendLine ($" \"{src}*\": \"https://raw.githubusercontent.com/xamarin/xamarin-macios/{latestCommit}/src*\""); -json.AppendLine (" }"); -json.AppendLine ("}"); - -File.WriteAllText (outputPath, json.ToString ()); +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) From a27d5f2aa5ff7791f836c1692da734501d50def3 Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Wed, 21 Feb 2024 09:02:48 -0800 Subject: [PATCH 06/12] add test --- tests/dotnet/UnitTests/Extensions.cs | 16 ++++++++++++++++ tests/dotnet/UnitTests/ProjectTest.cs | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/tests/dotnet/UnitTests/Extensions.cs b/tests/dotnet/UnitTests/Extensions.cs index 7a606da98bcc..5cb02347e147 100644 --- a/tests/dotnet/UnitTests/Extensions.cs +++ b/tests/dotnet/UnitTests/Extensions.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.IO; #nullable enable @@ -16,6 +17,21 @@ public static void AssertNoWarnings (this ExecutionResult result, Func v.ToString ()))}"); } + public static void StartWithArgs (this Process process, string args) + { + process.StartInfo = new ProcessStartInfo { + FileName = DotNet.Executable, + Arguments = args, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + }; + + process.Start (); + if (!process.WaitForExit (TimeSpan.FromSeconds (60))) + throw new TimeoutException ($"Process '{args}' timed out"); + } + public static void AssertWarnings (this IEnumerable actualWarnings, IEnumerable expectedWarnings) { // Source paths may be full (and local) paths. So make full paths relative to the root folder of xamarin-macios. diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index 1ac95435bf9b..5e9fc17269b4 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1666,5 +1666,32 @@ 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;")] + [TestCase (ApplePlatform.MacOSX, "osx-arm64;osx-x64")] + [TestCase (ApplePlatform.MacCatalyst, "maccatalyst-x64")] + [TestCase (ApplePlatform.TVOS, "tvos-arm64;")] + public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) + { + var project = "MySimpleApp"; + + var project_path = GetProjectPath (project, runtimeIdentifiers: runtimeIdentifiers, platform: platform, out var appPath); + Clean (project_path); + var properties = GetDefaultProperties (runtimeIdentifiers); + var rv = DotNet.AssertBuild (project_path, properties); + + var pdbFile = Directory + .GetFiles (Path.GetDirectoryName (project_path)!, "*.pdb", SearchOption.AllDirectories) + .FirstOrDefault (); + Assert.NotNull (pdbFile, "No PDB file found"); + + using Process install = new(); + install.StartWithArgs ("tool install sourcelink"); + + using Process test = new (); + test.StartWithArgs ($"sourcelink test {pdbFile}"); + + Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ReadToEnd ()); + } } } From 471c03bf0fd6a7b18c17a554ae95143767815258 Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Wed, 21 Feb 2024 09:08:45 -0800 Subject: [PATCH 07/12] ignore test locally/in PRs --- tests/common/TestRuntime.cs | 11 +++++++++++ tests/dotnet/UnitTests/ProjectTest.cs | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/tests/common/TestRuntime.cs b/tests/common/TestRuntime.cs index ae64edb68acb..83975c59f5f1 100644 --- a/tests/common/TestRuntime.cs +++ b/tests/common/TestRuntime.cs @@ -136,6 +136,17 @@ public static bool IsInCI { } } + 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) { diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index 5e9fc17269b4..a818d6944837 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1673,6 +1673,13 @@ public void StrippedRuntimeIdentifiers (ApplePlatform platform, string runtimeId [TestCase (ApplePlatform.TVOS, "tvos-arm64;")] public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) { + // 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 (!TestRuntime.IsCI || TestRuntime.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); @@ -1683,6 +1690,7 @@ public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) var pdbFile = Directory .GetFiles (Path.GetDirectoryName (project_path)!, "*.pdb", SearchOption.AllDirectories) .FirstOrDefault (); + Assert.NotNull (pdbFile, "No PDB file found"); using Process install = new(); From 269c7119ad670169f7420b71cd2b7e5545caa80c Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Wed, 21 Feb 2024 16:24:52 -0800 Subject: [PATCH 08/12] implement isPR and isCI logic in dotnet test suite tried reusing logic directly from the TestRuntime class but it is not as simple as just linking the file (as done here, for ex: https://github.com/xamarin/xamarin-macios/pull/6802) there are objc dependencies that require additional references, so to keep the changes minimal here i just reimplemented the 2 methods that were necessary.. --- tests/common/TestRuntime.cs | 2 +- tests/dotnet/UnitTests/ProjectTest.cs | 5 +---- tests/dotnet/UnitTests/TestBaseClass.cs | 23 +++++++++++++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/common/TestRuntime.cs b/tests/common/TestRuntime.cs index 83975c59f5f1..25ec4fa977c7 100644 --- a/tests/common/TestRuntime.cs +++ b/tests/common/TestRuntime.cs @@ -143,8 +143,8 @@ public static bool IsPullRequest { var pr = string.Equals(Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal); is_pull_request = pr; } + return is_pull_request.Value; } - return is_pull_request.Value; } public static void IgnoreInCI (string message) diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index a818d6944837..0f21fc97302c 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/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 { @@ -1677,7 +1674,7 @@ public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) // it is valid which will not pass until the commit has // been merged in and actually exists on github. - if (!TestRuntime.IsCI || TestRuntime.IsPullRequest) + if (!IsInCI || IsPullRequest) Assert.Ignore ("This test is disabled for local runs and Pull Requests."); var project = "MySimpleApp"; diff --git a/tests/dotnet/UnitTests/TestBaseClass.cs b/tests/dotnet/UnitTests/TestBaseClass.cs index a37cfe1d991c..3a0b91443eec 100644 --- a/tests/dotnet/UnitTests/TestBaseClass.cs +++ b/tests/dotnet/UnitTests/TestBaseClass.cs @@ -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; + } + } + } } From 49ea70e6bd9773d4d04e6cc62667c573f6c2533b Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Thu, 22 Feb 2024 00:30:34 +0000 Subject: [PATCH 09/12] Auto-format source code --- tests/common/TestRuntime.cs | 2 +- tests/dotnet/UnitTests/ProjectTest.cs | 2 +- tests/dotnet/UnitTests/TestBaseClass.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/common/TestRuntime.cs b/tests/common/TestRuntime.cs index 25ec4fa977c7..f3e8b6275619 100644 --- a/tests/common/TestRuntime.cs +++ b/tests/common/TestRuntime.cs @@ -140,7 +140,7 @@ public static bool IsInCI { public static bool IsPullRequest { get { if (!is_pull_request.HasValue) { - var pr = string.Equals(Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal); + var pr = string.Equals (Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal); is_pull_request = pr; } return is_pull_request.Value; diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index 0f21fc97302c..e68ec0344619 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1690,7 +1690,7 @@ public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) Assert.NotNull (pdbFile, "No PDB file found"); - using Process install = new(); + using Process install = new (); install.StartWithArgs ("tool install sourcelink"); using Process test = new (); diff --git a/tests/dotnet/UnitTests/TestBaseClass.cs b/tests/dotnet/UnitTests/TestBaseClass.cs index 3a0b91443eec..e4ecb7b5e13b 100644 --- a/tests/dotnet/UnitTests/TestBaseClass.cs +++ b/tests/dotnet/UnitTests/TestBaseClass.cs @@ -481,7 +481,7 @@ public static bool IsInCI { public static bool IsPullRequest { get { if (!is_pull_request.HasValue) { - var pr = string.Equals(Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal); + var pr = string.Equals (Environment.GetEnvironmentVariable ("BUILD_REASON"), "PullRequest", StringComparison.Ordinal); is_pull_request = pr; } return is_pull_request.Value; From bb6910485e0e2062fbc9421ac6d2411dfb96776b Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Thu, 22 Feb 2024 18:03:37 -0800 Subject: [PATCH 10/12] address review, update method format --- src/generate-embed-files.sh | 2 +- tests/common/DotNet.cs | 29 +++++++++++++++++++++++++++ tests/dotnet/UnitTests/Extensions.cs | 15 -------------- tests/dotnet/UnitTests/ProjectTest.cs | 25 +++++++++++------------ 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/generate-embed-files.sh b/src/generate-embed-files.sh index 34878c2b0bd7..1b9929f92d8e 100755 --- a/src/generate-embed-files.sh +++ b/src/generate-embed-files.sh @@ -12,4 +12,4 @@ IFS=$'\n' filtered_files=($(printf "%s\n" "${arr[@]}" | grep -vF "${git_files[*]}")) IFS=',' echo "-embed:${filtered_files[*]}" -unset IFS \ No newline at end of file +unset IFS diff --git a/tests/common/DotNet.cs b/tests/common/DotNet.cs index af1c0bb47698..929ef4bf022c 100644 --- a/tests/common/DotNet.cs +++ b/tests/common/DotNet.cs @@ -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)) + 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 (); + 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? properties, bool assert_success = true, string? target = null, bool? msbuildParallelism = null, TimeSpan? timeout = null) { if (!File.Exists (project)) diff --git a/tests/dotnet/UnitTests/Extensions.cs b/tests/dotnet/UnitTests/Extensions.cs index 5cb02347e147..e9c113ba1c7a 100644 --- a/tests/dotnet/UnitTests/Extensions.cs +++ b/tests/dotnet/UnitTests/Extensions.cs @@ -17,21 +17,6 @@ public static void AssertNoWarnings (this ExecutionResult result, Func v.ToString ()))}"); } - public static void StartWithArgs (this Process process, string args) - { - process.StartInfo = new ProcessStartInfo { - FileName = DotNet.Executable, - Arguments = args, - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - }; - - process.Start (); - if (!process.WaitForExit (TimeSpan.FromSeconds (60))) - throw new TimeoutException ($"Process '{args}' timed out"); - } - public static void AssertWarnings (this IEnumerable actualWarnings, IEnumerable expectedWarnings) { // Source paths may be full (and local) paths. So make full paths relative to the root folder of xamarin-macios. diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index e68ec0344619..8acc9be26fad 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1664,11 +1664,11 @@ public void StrippedRuntimeIdentifiers (ApplePlatform platform, string runtimeId } [Test] - [TestCase (ApplePlatform.iOS, "ios-arm64;")] - [TestCase (ApplePlatform.MacOSX, "osx-arm64;osx-x64")] - [TestCase (ApplePlatform.MacCatalyst, "maccatalyst-x64")] - [TestCase (ApplePlatform.TVOS, "tvos-arm64;")] - public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) + [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 @@ -1682,21 +1682,20 @@ public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers) var project_path = GetProjectPath (project, runtimeIdentifiers: runtimeIdentifiers, platform: platform, out var appPath); Clean (project_path); var properties = GetDefaultProperties (runtimeIdentifiers); - var rv = DotNet.AssertBuild (project_path, properties); + DotNet.AssertBuild (project_path, properties); var pdbFile = Directory - .GetFiles (Path.GetDirectoryName (project_path)!, "*.pdb", SearchOption.AllDirectories) + .GetFiles (Path.GetDirectoryName (project_path)!, $"Microsoft.{platformName}.pdb", SearchOption.AllDirectories) .FirstOrDefault (); Assert.NotNull (pdbFile, "No PDB file found"); - using Process install = new (); - install.StartWithArgs ("tool install sourcelink"); - - using Process test = new (); - test.StartWithArgs ($"sourcelink test {pdbFile}"); + var tool = "sourcelink"; + var toolPath = Directory.GetCurrentDirectory (); + DotNet.InstallTool (tool, toolPath); + var test = DotNet.RunTool ($"{toolPath}/{tool}", "test", $"{pdbFile}"); - Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ReadToEnd ()); + Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ToString()); } } } From 7d2f8cb971cc9570503de596d758ccd38c782b4a Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Fri, 23 Feb 2024 02:06:47 +0000 Subject: [PATCH 11/12] Auto-format source code --- tests/dotnet/UnitTests/ProjectTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index 8acc9be26fad..70f8abe4cb36 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1695,7 +1695,7 @@ public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers, s DotNet.InstallTool (tool, toolPath); var test = DotNet.RunTool ($"{toolPath}/{tool}", "test", $"{pdbFile}"); - Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ToString()); + Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ToString ()); } } } From b011e65aa736bab229b7d7b86a652bd7579a5a6b Mon Sep 17 00:00:00 2001 From: Haritha Mohan Date: Fri, 23 Feb 2024 07:49:18 -0800 Subject: [PATCH 12/12] address review --- tests/common/DotNet.cs | 2 +- tests/dotnet/UnitTests/Extensions.cs | 1 - tests/dotnet/UnitTests/ProjectTest.cs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/common/DotNet.cs b/tests/common/DotNet.cs index 929ef4bf022c..1e7afae875ae 100644 --- a/tests/common/DotNet.cs +++ b/tests/common/DotNet.cs @@ -117,7 +117,7 @@ public static ExecutionResult InstallTool (string tool, string path) { var installed = ExecuteCommand (Executable, "tool", "list", "--tool-path", path); if (!installed.StandardOutput.ToString ().Contains (tool)) - ExecuteCommand (Executable, "tool", "install", tool, "--tool-path", path); + installed = ExecuteCommand (Executable, "tool", "install", tool, "--tool-path", path); return installed; } diff --git a/tests/dotnet/UnitTests/Extensions.cs b/tests/dotnet/UnitTests/Extensions.cs index e9c113ba1c7a..7a606da98bcc 100644 --- a/tests/dotnet/UnitTests/Extensions.cs +++ b/tests/dotnet/UnitTests/Extensions.cs @@ -1,5 +1,4 @@ using System; -using System.Diagnostics; using System.IO; #nullable enable diff --git a/tests/dotnet/UnitTests/ProjectTest.cs b/tests/dotnet/UnitTests/ProjectTest.cs index 70f8abe4cb36..5f8686313c77 100644 --- a/tests/dotnet/UnitTests/ProjectTest.cs +++ b/tests/dotnet/UnitTests/ProjectTest.cs @@ -1693,7 +1693,7 @@ public void SourcelinkTest (ApplePlatform platform, string runtimeIdentifiers, s var tool = "sourcelink"; var toolPath = Directory.GetCurrentDirectory (); DotNet.InstallTool (tool, toolPath); - var test = DotNet.RunTool ($"{toolPath}/{tool}", "test", $"{pdbFile}"); + var test = DotNet.RunTool (Path.Combine (toolPath, tool), "test", pdbFile!); Assert.AreEqual ($"sourcelink test passed: {pdbFile}", test.StandardOutput.ToString ()); }