-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Couple fixes for UseSystemResourceKeys #103463
Changes from all commits
a596f31
b9f620f
b2ee176
cd16de6
d7bb029
703d871
92332d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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"> | ||
<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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could be changed to use If possible I'd prefer to move away from the XML for feature switches. If we can do so here, I wonder if we still need #96539 at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also realized that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Much simpler. Surprising nobody thought of this in the issue, we already had the facilities. FeatureSwitchDefinition didn't look necessary so I didn't add it - it wasn't clear to me what it would buy here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
// 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I synchronized SR.vb with SR.cs that we apparently neglected for years. |
||
End Function | ||
|
||
Friend Shared Function GetResourceString(ByVal resourceKey As String, Optional ByVal defaultString As String = Nothing) As String | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is #95948 as a proof that it works.