You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Running in Swedish regional settings, certain expressions that involve dividing and rounding values from msbuild properties return different results in msbuild 17 compared to msbuild 16. I suspect it's related to #8710 from 17.8.
Steps to Reproduce
In summary, I take TotalSeconds (double) from a TimeSpan and pit that in an msbuild property. Depending on how I then combined dividing and rounding in separate or combined expressions, the output is different.
The follow msbuild file show the differences:
<?xml version="1.0" encoding="utf-8"?>
<Projectxmlns="http://schemas.microsoft.com/developer/msbuild/2003"ToolsVersion="Current"DefaultTargets="Build">
<TargetName="Build">
<PropertyGroup>
<TotalSeconds>$([System.DateTime]::UtcNow.Subtract($([System.DateTime]::UtcNow.Date)).TotalSeconds)</TotalSeconds>
<Divided>$([MSBuild]::Divide($(TotalSeconds), 2))</Divided>
<ThenRounded>$([System.Math]::Round($(Divided)))</ThenRounded>
<CombinedDivideAndRound>$([System.Math]::Round($([MSBuild]::Divide($(TotalSeconds), 2))))</CombinedDivideAndRound>
<AllInOne>$([System.Math]::Round($([MSBuild]::Divide($([System.DateTime]::UtcNow.Subtract($([System.DateTime]::UtcNow.Date)).TotalSeconds), 2))))</AllInOne>
</PropertyGroup>
<!-- This will output something like 31161,3224946. -->
<MessageText="TotalSeconds: $(TotalSeconds)"Importance="High"/>
<!-- Sending the msbuild (string?) property $(TotalSeconds) to the msbuild Divide function. Seems msbuild 16 will understand the decimal comma, while msbuild 17 will ignore it (thousand sep?) --><!-- MSBUILD 16 output: 15580,66112473 --><!-- MSBUILD 17 output: 1558066112473 -->
<MessageText="Divided: $(Divided)"Importance="High"/>
<!-- Sending the msbuild (string?) property $(Divided) to the Math Round function. In this case also msbuild 16 will ignore the decimal comma that we got in $(Divided) on that version. --><!-- MSBUILD 16 output: 1558066112473 --><!-- MSBUILD 17 output: 1558066112473 -->
<MessageText="ThenRounded: $(ThenRounded)"Importance="High"/>
<!-- Sending the output from divide directly to round in a single expression. --><!-- MSBUILD 16 output: 15581 --><!-- MSBUILD 17 output: 1558066112473 -->
<MessageText="CombinedDivideAndRound: $(CombinedDivideAndRound)"Importance="High"/>
<!-- Everything in a single expression. Correct output on both versions. But the expression is long and difficult to read... --><!-- MSBUILD 16 output: 15581 --><!-- MSBUILD 17 output: 15581 -->
<MessageText="AllInOne: $(AllInOne)"Importance="High"/>
</Target>
</Project>
Expected Behavior
I don't want to claim this is really the expected output, but it is the actual output on msbuild 16.11:
It seems that on v16, msbuild will honour the decimal comma when it's present in a string msbuild property passed to Divide, but not when passed to Round. For Round I assume it is instead perceived as a thousands separator.
In v17, it is perceived as a thousands separator also for the Divide method.
When the different methods are combined into a single expression it seems to work and produce the correct result - I suppose in this case it never round-trips as a string msbuild properter. But the expression gets long and difficult to read.
I suspect this is related to the application of InvariantCulture for double TryParse as part of #8710.
I do hesitate to claim this is a bug... Perhaps it is as intended, but it does raise an interesting dilemma:
When a floating point value is put in string msbuild property, a decimal comma will be used. When later trying to use this property as an argument to something that takes a double, the decimal comma will be silently treated as a thousands separator instead. It is inconsistent. And to make matters worse, I suppose the same syntax will work fine in regions using a decimal point, but then produce incorrect results if run by someone in e.g. Swedish regional format settings.
If double parsing will insist on requiring decimal point, should msbuild not take care to use that also when converting doubles to strings?
So far I haven't found any docs talking about this subject in terms of "best practice" or similar.
Versions & Configurations
No response
The text was updated successfully, but these errors were encountered:
Issue Description
Running in Swedish regional settings, certain expressions that involve dividing and rounding values from msbuild properties return different results in msbuild 17 compared to msbuild 16. I suspect it's related to #8710 from 17.8.
Steps to Reproduce
In summary, I take TotalSeconds (double) from a TimeSpan and pit that in an msbuild property. Depending on how I then combined dividing and rounding in separate or combined expressions, the output is different.
The follow msbuild file show the differences:
Expected Behavior
I don't want to claim this is really the expected output, but it is the actual output on msbuild 16.11:
Actual Behavior
This is the actual output from msbuild 17.8:
Analysis
It seems that on v16, msbuild will honour the decimal comma when it's present in a string msbuild property passed to
Divide
, but not when passed toRound
. ForRound
I assume it is instead perceived as a thousands separator.In v17, it is perceived as a thousands separator also for the
Divide
method.When the different methods are combined into a single expression it seems to work and produce the correct result - I suppose in this case it never round-trips as a string msbuild properter. But the expression gets long and difficult to read.
I suspect this is related to the application of InvariantCulture for double TryParse as part of #8710.
I do hesitate to claim this is a bug... Perhaps it is as intended, but it does raise an interesting dilemma:
When a floating point value is put in string msbuild property, a decimal comma will be used. When later trying to use this property as an argument to something that takes a double, the decimal comma will be silently treated as a thousands separator instead. It is inconsistent. And to make matters worse, I suppose the same syntax will work fine in regions using a decimal point, but then produce incorrect results if run by someone in e.g. Swedish regional format settings.
If double parsing will insist on requiring decimal point, should msbuild not take care to use that also when converting doubles to strings?
So far I haven't found any docs talking about this subject in terms of "best practice" or similar.
Versions & Configurations
No response
The text was updated successfully, but these errors were encountered: