Skip to content
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

Fix LangVersion computation on non-English locales #76070

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

jjonescz
Copy link
Member

@jjonescz jjonescz commented Nov 25, 2024

Fixes failing Spanish leg in roslyn-CI after #75795.

Given $(_TargetFrameworkVersionWithoutV) = 5.0, the subtraction $([MSBuild]::Subtract($(_TargetFrameworkVersionWithoutV), 5)) evaluates to 45 (I guess because the period in 5.0 is not interpreted as decimal separator but as thousands separator instead). Adding .Split('.')[0] makes sure only the major version is taken and the subtraction works.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 25, 2024
@jjonescz jjonescz requested a review from jcouv November 25, 2024 11:50
@jjonescz jjonescz marked this pull request as ready for review November 25, 2024 12:56
@jjonescz jjonescz requested a review from a team as a code owner November 25, 2024 12:56
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Isn't this related to the bug on MSBuild side?

dotnet/msbuild#9757

@jjonescz
Copy link
Member Author

jjonescz commented Nov 25, 2024

Isn't this related to the bug on MSBuild side?

Yes, that looks related. Thanks for the link.

@@ -20,7 +20,7 @@
to determine the correct language version.
-->
<_MaxSupportedLangVersion Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND
'$(_MaxSupportedLangVersion)' == ''">$([MSBuild]::Add(9, $([MSBuild]::Subtract($(_TargetFrameworkVersionWithoutV), 5)))).0</_MaxSupportedLangVersion>
'$(_MaxSupportedLangVersion)' == ''">$([MSBuild]::Add(9, $([MSBuild]::Subtract($(_TargetFrameworkVersionWithoutV.Split('.')[0]), 5)))).0</_MaxSupportedLangVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment that points to the msbuild issue about localization?

@jjonescz jjonescz merged commit 079c507 into dotnet:main Nov 25, 2024
24 checks passed
@jjonescz jjonescz deleted the 75795-SpanishFailing branch November 25, 2024 16:07
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants