Skip to content

Conversation

@bjornhellander
Copy link
Contributor

Attempted fix for #3402. A bit of a hack :-), but should hopefully work.

Note: I haven't been able to verify this fix on my machine, due to not understanding how, but the problematic calls to the CultureInfo constructor with a culture name are gone. I can try again if someone can explain what to do.

@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #3411 (0dcb1da) into master (aacf8a5) will decrease coverage by 0.01%.
The diff coverage is 45.16%.

@@            Coverage Diff             @@
##           master    #3411      +/-   ##
==========================================
- Coverage   93.09%   93.08%   -0.02%     
==========================================
  Files        1038     1039       +1     
  Lines      111809   111834      +25     
  Branches     3969     3969              
==========================================
+ Hits       104089   104097       +8     
- Misses       6702     6719      +17     
  Partials     1018     1018              

@bjornhellander
Copy link
Contributor Author

A test for SA1134 failed on the release build. I have seen this sometimes before this pr and have now run the tests locally a couple of times again without any problems. Pushing again to trigger a new build.

@bjornhellander bjornhellander force-pushed the feature/Issue3402InvariantMode branch from af1ca47 to f458507 Compare December 4, 2021 08:46
…trary names, to make analyzers work also in globalization invariant mode
@bjornhellander
Copy link
Contributor Author

Oookey. No code changes relative to the first push and now lots of tests fail with one of the following exceptions:

  • System.Runtime.InteropServices.COMException : Signature specified is zero-sized
  • System.BadImageFormatException : Bad binary signature

Stilll don't think this is related to my change, so pushing again to trigger a new build.

@bjornhellander bjornhellander force-pushed the feature/Issue3402InvariantMode branch from f458507 to 0dcb1da Compare December 5, 2021 11:24
Copy link
Member

@sharwell sharwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on an internal discussion, this change does not appear to adhere to the design/intent of DOTNET_SYSTEM_GLOBALIZATION_INVARIANT. The documented approach to handling this situation is setting one of the following environment variables:

  • Set DOTNET_SYSTEM_GLOBALIZATION_INVARIANT to false
  • Set DOTNET_SYSTEM_GLOBALIZATION_PREDEFINED_CULTURES_ONLY to false

If I get a new suggestion from the team that implemented this option, we may reconsider the approach used by this tool.

Separately, we would accept a change to make the default culture work in this configuration (i.e. cases where DocumentationCulture has not been set). The solution in this case is to use the invariant culture directly instead of trying to create a new instance of a different culture.

@sharwell
Copy link
Member

sharwell commented Jan 6, 2022

I submitted #3426 separately to make the default value "en-US" work with DOTNET_SYSTEM_GLOBALIZATION_INVARIANT.

@sharwell
Copy link
Member

sharwell commented Jan 7, 2022

Superseded by #3426

@sharwell sharwell closed this Jan 7, 2022
@bjornhellander bjornhellander deleted the feature/Issue3402InvariantMode branch May 2, 2022 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants