[dotnet] Don't overwrite any existing value for DOTNET_DiagnosticPorts when computed through our build logic. (#23993)

Until now, people set the 'DOTNET_DiagnosticPorts' environment variable at
launch/runtime to profile or trace an app.

When we added support for setting specific MSBuild properties to enable
diagnostics (DiagnosticsAddress, DiagnosticsSuspend, etc.), we end up
unconditionally setting the DOTNET_DiagnosticPorts environment variable,
effectively overwriting any value specified by customers when launching the
app.

This meant that existing (and fully functioning) profiling/tracing workflows
would only work if they set the DOTNET_DiagnosticPorts variable to the
defaults we have.

Fix this by allowing overriding DOTNET_DiagnosticPorts variable at launch/runtime.

Author: rolfbjarne

dotnet/targets/Xamarin.Shared.Sdk.targets

@@ -590,7 +590,7 @@
 				DeploymentTarget=$(_MinimumOSVersion)
 				@(_CustomLinkFlags -> 'CustomLinkFlags=%(Identity)')
 				EnableSGenConc=$(EnableSGenConc)
-				@(_BundlerEnvironmentVariables -> 'EnvironmentVariable=%(Identity)=%(Value)')
+				@(_BundlerEnvironmentVariables -> 'EnvironmentVariable=Overwrite=%(Overwrite)|%(Identity)=%(Value)')
 				@(_XamarinFrameworkAssemblies -> 'FrameworkAssembly=%(Filename)')
 				Interpreter=$(MtouchInterpreter)
 				IntermediateLinkDir=$(IntermediateLinkDir)

msbuild/Xamarin.Shared/Xamarin.Shared.props

@@ -235,9 +235,9 @@ Copyright (C) 2020 Microsoft. All rights reserved.
 		<DiagnosticConfiguration Condition="'$(DiagnosticSuspend)' == 'true'">$(DiagnosticConfiguration),suspend</DiagnosticConfiguration>
 		<DiagnosticConfiguration Condition="'$(DiagnosticSuspend)' != 'true'">$(DiagnosticConfiguration),nosuspend</DiagnosticConfiguration>
 	</PropertyGroup>
-	<PropertyGroup Condition="'$(EnableDiagnostics)' == 'true' And '$(DiagnosticConfiguration)' != '' And '$(DiagnosticConfiguration)' != 'disable'">
-		<AppBundleExtraOptions>$(AppBundleExtraOptions) --setenv=DOTNET_DiagnosticPorts=$(DiagnosticConfiguration)</AppBundleExtraOptions>
-	</PropertyGroup>
+	<ItemGroup Condition="'$(EnableDiagnostics)' == 'true' And '$(DiagnosticConfiguration)' != '' And '$(DiagnosticConfiguration)' != 'disable'">
+		<_BundlerEnvironmentVariables Include="DOTNET_DiagnosticPorts" Value="$(DiagnosticConfiguration)" Overwrite="false" />
+	</ItemGroup>
 
 	<PropertyGroup Condition="'$(IsBindingProject)' == 'true'">
 		<GeneratedSourcesDir Condition="'$(GeneratedSourcesDir)' == ''">$(IntermediateOutputPath)$(_PlatformName)</GeneratedSourcesDir>

tools/common/Application.cs

@@ -151,7 +151,7 @@ public bool AreAnyAssembliesTrimmed {
 		public bool EnableDiagnostics;
 		public bool? DebugTrack;
 
-		public Dictionary<string, string> EnvironmentVariables = new Dictionary<string, string> ();
+		public Dictionary<string, (string Value, bool Overwrite)> EnvironmentVariables = new Dictionary<string, (string Value, bool Overwrite)> ();
 
 		public MarshalObjectiveCExceptionMode MarshalObjectiveCExceptions;
 		public MarshalManagedExceptionMode MarshalManagedExceptions;

tools/common/Target.cs

@@ -777,8 +777,12 @@ void GenerateIOSMain (StringWriter sw, Abi abi)
 			if (!string.IsNullOrEmpty (app.MonoGCParams))
 				sw.WriteLine ("\tsetenv (\"MONO_GC_PARAMS\", \"{0}\", 1);", app.MonoGCParams);
 			// Do this last, so that the app developer can override any other environment variable we set.
-			foreach (var kvp in app.EnvironmentVariables)
-				sw.WriteLine ("\tsetenv (\"{0}\", \"{1}\", 1);", kvp.Key.Replace ("\"", "\\\""), kvp.Value.Replace ("\"", "\\\""));
+			foreach (var kvp in app.EnvironmentVariables) {
+				var name = kvp.Key;
+				var value = kvp.Value.Value;
+				var overwrite = kvp.Value.Overwrite;
+				sw.WriteLine ("\tsetenv (\"{0}\", \"{1}\", {2});", name.Replace ("\"", "\\\""), value.Replace ("\"", "\\\""), overwrite ? 1 : 0);
+			}
 			if (app.XamarinRuntime != XamarinRuntime.NativeAOT)
 				sw.WriteLine ("\txamarin_supports_dynamic_registration = {0};", app.DynamicRegistrationSupported ? "TRUE" : "FALSE");
 #if NET && !LEGACY_TOOLS

tools/dotnet-linker/LinkerConfiguration.cs

@@ -173,11 +173,23 @@ public static LinkerConfiguration GetInstance (LinkContext context)
 					Application.EnableSGenConc = string.Equals (value, "true", StringComparison.OrdinalIgnoreCase);
 					break;
 				case "EnvironmentVariable":
+					var overwrite = true;
+					var needle = "Overwrite=";
+					if (value.StartsWith (needle, StringComparison.Ordinal)) {
+						var pipe = value.IndexOf ('|', needle.Length);
+						if (pipe > 0) {
+							var overwriteString = value [needle.Length..pipe];
+							if (!TryParseOptionalBoolean (overwriteString, out var parsedOverwrite))
+								throw new InvalidOperationException ($"Unable to parse the 'Overwrite' value '{overwriteString}' for the environment variable entry '{value}' in {linker_file}");
+							overwrite = parsedOverwrite.Value;
+							value = value [(pipe + 1)..];
+						}
+					}
 					var separators = new char [] { ':', '=' };
 					var equals = value.IndexOfAny (separators);
 					var name = value.Substring (0, equals);
 					var val = value.Substring (equals + 1);
-					Application.EnvironmentVariables.Add (name, val);
+					Application.EnvironmentVariables.Add (name, new (val, overwrite));
 					break;
 				case "FrameworkAssembly":
 					FrameworkAssemblies.Add (value);