Make it possible for featuredefault substitutions to only apply at publish time

Fixes dotnet#96539.
Fixes dotnet#102303.

Two possible approaches were discussed in dotnet#96539 but doing it in illinker feels more straightforward.

Author: MichalStrehovsky

eng/ILLink.Substitutions.Resources.template

@@ -1,10 +1,16 @@
 <linker>
   <!-- System.Resources.UseSystemResourceKeys removes resource strings and instead uses the resource key as the exception message -->
   <assembly fullname="{AssemblyName}" feature="System.Resources.UseSystemResourceKeys" featurevalue="true">
-    <!-- System.Resources.UseSystemResourceKeys removes resource strings and instead uses the resource key as the exception message -->
     <resource name="{StringResourcesName}.resources" action="remove" />
     <type fullname="System.SR">
       <method signature="System.Boolean UsingResourceKeys()" body="stub" value="true" />
+      <method signature="System.Boolean GetUsingResourceKeysSwitchValue()" body="stub" value="true" />
+    </type>
+  </assembly>
+  <assembly fullname="{AssemblyName}" feature="System.Resources.UseSystemResourceKeys" featurevalue="false" featuredefault="true">
+    <type fullname="System.SR">
+      <method signature="System.Boolean UsingResourceKeys()" body="stub" value="false" />
+      <method signature="System.Boolean GetUsingResourceKeysSwitchValue()" body="stub" value="false" />
     </type>
   </assembly>
 </linker>

eng/illink.targets

@@ -37,7 +37,7 @@
     <ILLinkRewritePDBs Condition="'$(ILLinkRewritePDBs)' == ''">true</ILLinkRewritePDBs>
 
     <ILLinkResourcesSubstitutionIntermediatePath>$(IntermediateOutputPath)ILLink.Resources.Substitutions.xml</ILLinkResourcesSubstitutionIntermediatePath>
-    <GenerateResourcesSubstitutions Condition="'$(GenerateResourcesSubstitutions)' == '' and '$(StringResourcesPath)' != ''">true</GenerateResourcesSubstitutions>
+    <GenerateResourcesSubstitutions Condition="'$(GenerateResourcesSubstitutions)' == '' and '$(StringResourcesPath)' != '' and '$(OmitResources)' != 'true'">true</GenerateResourcesSubstitutions>
   </PropertyGroup>
 
    <ItemGroup>
@@ -222,6 +222,8 @@
       <LinkerNoWarn>$(LinkerNoWarn);IL2008;IL2009;IL2121;IL2025;IL2035</LinkerNoWarn>
       <ILLinkArgs>$(ILLinkArgs) --nowarn $(LinkerNoWarn)</ILLinkArgs>
       <ILLinkArgs Condition="'$(ILLinkDisableIPConstProp)' == 'true'">$(ILLinkArgs) --disable-opt ipconstprop</ILLinkArgs>
+      <!-- Featuredefault for System.Resources.UseSystemResourceKeys would hardcode all libraries as not using resource keys, don't apply it -->
+      <ILLinkArgs Condition="'$(GenerateResourcesSubstitutions)' == 'true'">$(ILLinkArgs) --ignore-featuredefault System.Resources.UseSystemResourceKeys</ILLinkArgs>
     </PropertyGroup>
 
     <!-- Move the assembly into a subdirectory for ILLink -->

src/libraries/Common/src/System/SR.cs

@@ -7,7 +7,10 @@ namespace System
 {
     internal static partial class SR
     {
-        private static readonly bool s_usingResourceKeys = AppContext.TryGetSwitch("System.Resources.UseSystemResourceKeys", out bool usingResourceKeys) ? usingResourceKeys : false;
+        private static readonly bool s_usingResourceKeys = GetUsingResourceKeysSwitchValue();
+
+        // This method is a target of ILLink substitution.
+        private static bool GetUsingResourceKeysSwitchValue() => AppContext.TryGetSwitch("System.Resources.UseSystemResourceKeys", out bool usingResourceKeys) ? usingResourceKeys : false;
 
         // This method is used to decide if we need to append the exception message parameters to the message when calling SR.Format.
         // by default it returns the value of System.Resources.UseSystemResourceKeys AppContext switch or false if not specified.

src/libraries/Common/src/System/SR.vb

@@ -11,15 +11,23 @@ Imports System.Resources
 Namespace System
 
     Friend NotInheritable Class SR
-        ' This method is used to decide if we need to append the exception message parameters to the message when calling SR.Format. 
-        ' by default it returns false.
+        Private Shared ReadOnly s_usingResourceKeys As Boolean = GetUsingResourceKeysSwitchValue()
+
+        Private Shared Function GetUsingResourceKeysSwitchValue() As Boolean
+            Dim usingResourceKeys As Boolean
+            If (AppContext.TryGetSwitch("System.Resources.UseSystemResourceKeys", usingResourceKeys)) Then
+                Return usingResourceKeys
+            End If
+
+            Return False
+        End Function
+
+        ' This method Is used to decide if we need to append the exception message parameters to the message when calling SR.Format.
+        ' by default it returns the value of System.Resources.UseSystemResourceKeys AppContext switch Or false if Not specified.
         ' Native code generators can replace the value this returns based on user input at the time of native code generation.
-        ' Marked as NoInlining because if this is used in an AoT compiled app that is not compiled into a single file, the user
-        ' could compile each module with a different setting for this. We want to make sure there's a consistent behavior
-        ' that doesn't depend on which native module this method got inlined into.
-        <Global.System.Runtime.CompilerServices.MethodImpl(Global.System.Runtime.CompilerServices.MethodImplOptions.NoInlining)>
+        ' The trimming tools are also capable of replacing the value of this method when the application Is being trimmed.
         Public Shared Function UsingResourceKeys() As Boolean
-            Return False
+            Return s_usingResourceKeys
         End Function
 
         Friend Shared Function GetResourceString(ByVal resourceKey As String, Optional ByVal defaultString As String = Nothing) As String

src/libraries/System.Diagnostics.FileVersionInfo/src/System.Diagnostics.FileVersionInfo.csproj

@@ -9,7 +9,8 @@
   <!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
   <PropertyGroup>
     <TargetPlatformIdentifier>$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)'))</TargetPlatformIdentifier>
-    <GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetPlatformIdentifier)' == ''">SR.DiagnosticsFileVersionInfo_PlatformNotSupported</GeneratePlatformNotSupportedAssemblyMessage> 
+    <GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetPlatformIdentifier)' == ''">SR.DiagnosticsFileVersionInfo_PlatformNotSupported</GeneratePlatformNotSupportedAssemblyMessage>
+    <OmitResources Condition="'$(GeneratePlatformNotSupportedAssemblyMessage)' == ''">true</OmitResources>
   </PropertyGroup>
 
   <ItemGroup Condition="'$(TargetPlatformIdentifier)' != ''">

src/tools/illink/src/linker/Linker/Driver.cs

@@ -470,6 +470,14 @@ protected int SetupContext (ILogger? customLogger = null)
 
 							continue;
 						}
+					case "--ignore-featuredefault": {
+						if (!GetStringParam (token, out string? featureName))
+							return -1;
+
+						context.SetIgnoredFeatureDefault (featureName);
+
+						continue;
+					}
 					case "--new-mvid":
 						//
 						// This is not same as --deterministic which calculates MVID

src/tools/illink/src/linker/Linker/FeatureSettings.cs

@@ -33,6 +33,10 @@ public static bool ShouldProcessElement (XPathNavigator nav, LinkContext context
 				return false;
 			}
 
+			if (bIsDefault && context.IgnoredFeatureDefaultSettings.Contains (feature)) {
+				bIsDefault = false;
+			}
+
 			if (!context.FeatureSettings.TryGetValue (feature, out bool featureSetting))
 				return bIsDefault;
 

src/tools/illink/src/linker/Linker/LinkContext.cs

@@ -136,6 +136,8 @@ public Pipeline Pipeline {
 
 		public Dictionary<string, bool> FeatureSettings { get; init; }
 
+		public HashSet<string> IgnoredFeatureDefaultSettings { get; init; }
+
 		public List<PInvokeInfo> PInvokes { get; private set; }
 
 		public string? PInvokesListFile;
@@ -218,6 +220,7 @@ protected LinkContext (Pipeline pipeline, ILogger logger, string outputDirectory
 			_isTrimmable = new Dictionary<AssemblyDefinition, bool> ();
 			OutputDirectory = outputDirectory;
 			FeatureSettings = new Dictionary<string, bool> (StringComparer.Ordinal);
+			IgnoredFeatureDefaultSettings = new HashSet<string> (StringComparer.Ordinal);
 
 			SymbolReaderProvider = new DefaultSymbolReaderProvider (false);
 
@@ -264,6 +267,12 @@ public void SetFeatureValue (string feature, bool value)
 			FeatureSettings[feature] = value;
 		}
 
+		public void SetIgnoredFeatureDefault (string feature)
+		{
+			Debug.Assert (!string.IsNullOrEmpty (feature));
+			IgnoredFeatureDefaultSettings.Add (feature);
+		}
+
 		public bool HasFeatureValue (string feature, bool value)
 		{
 			return FeatureSettings.TryGetValue (feature, out bool fvalue) && value == fvalue;

src/tools/illink/test/Mono.Linker.Tests.Cases/FeatureSettings/FeatureDescriptors.cs

@@ -15,6 +15,7 @@ namespace Mono.Linker.Tests.Cases.FeatureSettings
 	[SetupLinkerArgument ("--feature", "FieldCondition", "true")]
 	[SetupLinkerArgument ("--feature", "PropertyCondition", "false")]
 	[SetupLinkerArgument ("--feature", "EventCondition", "true")]
+	[SetupLinkerArgument ("--ignore-featuredefault", "IgnoredDefaultCondition")]
 	public class FeatureDescriptors
 	{
 		public static void Main ()
@@ -25,6 +26,9 @@ public static void Main ()
 		static bool DefaultConditionTrue;
 		static bool DefaultConditionFalse;
 
+                static bool IgnoredDefaultConditionTrue;
+                static bool IgnoredDefaultConditionFalse;
+
 		[Kept]
 		static bool GlobalConditionTrue;
 		static bool GlobalConditionFalse;

src/tools/illink/test/Mono.Linker.Tests.Cases/FeatureSettings/FeatureDescriptors.xml

@@ -47,4 +47,16 @@
       <field name="DefaultConditionFalse" />
     </type>
   </assembly>
+  <!-- Check that a default feature condition can be ignored -->
+  <assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null" feature="IgnoredDefaultCondition" featurevalue="true" featuredefault="true">
+    <type fullname="Mono.Linker.Tests.Cases.FeatureSettings.FeatureDescriptors">
+      <field name="IgnoredDefaultConditionTrue" />
+    </type>
+  </assembly>
+  <!-- Else case for the ignored default condition -->
+  <assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null" feature="IgnoredDefaultCondition" featurevalue="false">
+    <type fullname="Mono.Linker.Tests.Cases.FeatureSettings.FeatureDescriptors">
+      <field name="IgnoredDefaultConditionFalse" />
+    </type>
+  </assembly>
 </linker>