[Bug]: Culture used when formatting Date changed in PropertyGroup

[a-ctor]

Issue Description

Starting with MSBuild 17.11 a property set to a date value will use English culture for formatting instead of the current culture. This only happens if the value of the property is the date like in this example:

<PropertyGroup>
   <_dateValue>$([System.DateTime]::UtcNow)</_dateValue>
</PropertyGroup>

My computer uses German date formatting but MSBuild will format the example above using the English date format. This changed between .NET 8 and .NET 9. Previously, the value was formatted using the German date format.

Steps to Reproduce

Sample build file that logs formatted dates in different contexts:

<?xml version="1.0" encoding="utf-8"?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

  <PropertyGroup>
   <_globalDateValue>$([System.DateTime]::UtcNow)</_globalDateValue>
  </PropertyGroup>

  <Target Name="Test">
    <PropertyGroup>
      <_dateValue>$([System.DateTime]::UtcNow)</_dateValue>
    </PropertyGroup>

    <Warning Text="Inline date: '$([System.DateTime]::UtcNow)'" />
    <Warning Text="Local date: '$(_dateValue)'" />
    <Warning Text="Global date: '$(_globalDateValue)'" />
    <Warning Text="MSBuild version: $(MSBuildFileVersion)" />

  </Target>

</Project>

Execute using dotnet msbuild Repro.build /t:Test using a display culture that is not English. In my case I use German date formatting with English system language:

Expected Behavior

When executing with .NET 8, all dates are formatted using the German date format:

Inline date: '23.07.2024 14:34:12'
Local date: '23.07.2024 14:34:12'
Global date: '23.07.2024 14:34:12'
MSBuild version: 17.10.4.21802

Actual Behavior

With .NET 9 the values from <PropertyGroup> use English culture formatting instead of German:

Inline date: '23.07.2024 14:39:45'
Local date: '07/23/2024 14:39:45'
Global date: '07/23/2024 14:39:45'
MSBuild version: 17.11.0.31805

I would expect the values from the <PropertyGroup> to be formatted just like inline strings using the German date format.

Analysis

The formatting bug only happens if the date is the only value in the <PropertyGroup>. If other text is included in the property the correct German format will be used. For example:

• <_dateValue>$([System.DateTime]::UtcNow)</_dateValue> evaluates to 07/23/2024 14:51:59
• <_dateValue> $([System.DateTime]::UtcNow)</_dateValue> evaluates to 23.07.2024 14:52:30 (note the preceding space)

My assumption here is that _dateValue is stored as an object so the DateTime value is directly assigned while any extra characters will do a string.Format using the correct culture. But when _dateValue is used later on in a string.Format the culture is not correctly applied.

Versions & Configurations

Behavior changed between .NET 8 (MSBuild version 17.10.4.21802) and .NET 9 (MSBuild version 17.11.0.31805)

[a-ctor]

Analyzed the problem a bit more. Regression seems to be caused by https://github.com/dotnet/msbuild/issues/9757

Reverting the following changes locally fixes the problem for me:

https://github.com/dotnet/msbuild/commit/1e513b346acdf40dd2586a463e38f89fa72a69e9#diff-e6b332d6a62d7e070dc0b1621e77b7d0146e52a8eba9c2bec2a7cdd3fdb5739fR1480-R1489

There should probably be an exception case for DateTime values that should always be formatted using current culture and not invariant culture

[MichaelKetting]

There should probably be an exception case for DateTime values that should always be formatted using current culture and not invariant culture

Without having tested it, I believe decimal numbers would also be affected due to the difference of using a dot (.) or a comma (,) to denote the decimal place. In German, the dot and comma have inverse semantics, we use the comma for decimals and the dot for thousand-separators.

[f-alizada]

Thank you for reporting that! I believe the better solution would be to use the InvariantCultur only for internal comparison or any other usage (internal I mean using in expressions for example). The mitigation for now would be to use the ChangeWave 17.12 version. Meanwhile we'll take a look at possible quick resolution or revert back.

[MichaelKetting]

@f-alizada thanks for the update. 17.12 is getting released in November 2024, correct? Cool if there's an earlier fix coming out!

[f-alizada]

@a-ctor @MichaelKetting Is there a chance you could share with us the use case of this functionality that were broken by the changes you mentioned. And possibly how impactful this is? Also did the ChangeWave fix the problem for you? (without reverting the code changes :) ) Thank you in advance!

Unfortunately no updates regarding the fix at the moment.

[a-ctor]

Hi @f-alizada, thanks for responding so quickly! Adding the Change Wave env variable for 17.12 fix the problem for us :)

We have a build script that is written in MSBuild. To measure the test duration we save the current time in a property (<_startTime>$([System.DateTime]::Now)</startTime>), execute tests, and later use DateTime::Parse(...) to calculate the time it took the tests to execute.

Because of this bug, _startTime is now formatted using invariant culture, while the parse method still uses german date format for parsing. This silently introduced a bug as _starTime was now MM/dd/yyyy ... instead of dd.MM.yyyy ... but parsing did not fail until day 13 of this month where the day could no longer be parsed as a month. With the 13th, our builds started failing because of a date parsing issue, while previous builds reported invalid test execution times as the dates were parsed incorrectly.

We can fix this issue on our side using explicit date formats when string formatting and parsing. At the same time, it is very unintuative that $([System.DateTime]::Now) and date: $([System.DateTime]::Now) use different date formats. Both should use the same formatting be that current culture or invariant culture and DateTime::Parse should also match that default.

[f-alizada]

@a-ctor Thank you for the detailed explanation! We will investigate the possibility to address the issue and share the updates.

[f-alizada]

Per the recent update the issue is gathering the feedback. FYI: @AR-May