Skip to content

Conversation

dsplaisted
Copy link
Member

For "funky" reasons related to dotnet/msbuild#3460, the version normalization that we added in #50264 is causing exceptions to be thrown. This isn't a functional issue, the fact that the exceptions are thrown and then caught are part of how MSBuild works.

However, the additional exceptions are showing up on VS perf tests as a regression in the total number of exceptions thrown.

This PR rewrites the logic to avoid calling Version.Parse on the %(Identity), which should avoid the exceptions.

<_NormalizedWindowsSdkSupportedTargetPlatformVersion Include="@(WindowsSdkSupportedTargetPlatformVersion)">
<NormalizedSupportedTargetPlatformVersion
Condition="$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').Length) == 4 And
$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(3)) == '1'"
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately in the speculative-evaluation path GetValue({}) throws an IndexOutOfRangeException. Thinking of alternatives.

Comment on lines +29 to +31
Condition="$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').Length) == 4 And
$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(3)) == '1'"
>$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(0)).$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(1)).$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(2)).0</NormalizedSupportedTargetPlatformVersion>
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK in the debugger this works with no internal exceptions:

Suggested change
Condition="$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').Length) == 4 And
$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(3)) == '1'"
>$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(0)).$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(1)).$([MSBuild]::ValueOrDefault('%(Identity)', '').Split('.').GetValue(2)).0</NormalizedSupportedTargetPlatformVersion>
Condition="$([System.Text.RegularExpressions.Regex]::IsMatch(%(Identity), '^((\d+\.){3})1$'))"
>$([System.Text.RegularExpressions.Regex]::Replace(%(Identity), '^((\d+\.){3})1$', '${1}0'))</NormalizedSupportedTargetPlatformVersion>

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