dotnet / dotnet/msbuild

Avoid exceptions when calling [System.Version]::Parse

Open
#12,590 1 comment 2 reactions 1 assignee Assigned to @Copilot View on GitHub
10.0 Area: Performance
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 8h
Merged PRs (30d)
141

Description

We're observing a regression in exception count in some VS scenarios when inserting builds of .NET SDK 10.0.100-rc.2. Specifically, there's a `System.ArgumentException` thrown with a stack like this:

```
mscorlib.dll!System.Version.VersionResult.SetFailure(System.Version.ParseFailureKind failure, string argument) Unknown
mscorlib.dll!System.Version.TryParseVersion(string version, ref System.Version.VersionResult result) Unknown
mscorlib.dll!System.Version.Parse(string input) Unknown
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.WellKnownFunctions.TryExecuteWellKnownFunction(string methodName, System.Type receiverType, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem, out object returnVal, object objectInstance, object[] args) Line 862 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.Function.Execute(object objectInstance, Microsoft.Build.Evaluation.IPropertyProvider properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation) Line 4061 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.PropertyExpander.ExpandPropertyBody(string propertyBody, object propertyValue, Microsoft.Build.Evaluation.IPropertyProvider properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation, Microsoft.Build.Evaluation.PropertiesUseTracker propertiesUseTracker, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem) Line 1529 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.PropertyExpander.ExpandPropertiesLeaveTypedAndEscaped(string expression, Microsoft.Build.Evaluation.IPropertyProvider properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation, Microsoft.Build.Evaluation.PropertiesUseTracker propertiesUseTracker, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem) Line 1373 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.PropertyExpander.ExpandPropertiesLeaveEscaped(string expression, Microsoft.Build.Evaluation.IPropertyProvider properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation, Microsoft.Build.Evaluation.PropertiesUseTracker propertiesUseTracker, Microsoft.Build.Shared.FileSystem.IFileSystem fileSystem) Line 1239 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander.ExpandIntoStringLeaveEscaped(string expression, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation) Line 497 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator.ProcessMetadataElements(Microsoft.Build.Construction.ProjectItemElement itemElement, Microsoft.Build.Evaluation.LazyItemEvaluator.OperationBuilderWithMetadata operationBuilder) Line 642 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator.BuildIncludeOperation(string rootDirectory, Microsoft.Build.Construction.ProjectItemElement itemElement, bool conditionResult) Line 576 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator.ProcessItemElement(string rootDirectory, Microsoft.Build.Construction.ProjectItemElement itemElement, bool conditionResult) Line 512 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator.EvaluateItemElement(bool itemGroupConditionResult, Microsoft.Build.Construction.ProjectItemElement itemElement, Microsoft.Build.Evaluation.LazyItemEvaluator lazyEvaluator) Line 1338 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator.EvaluateItemGroupElement(Microsoft.Build.Construction.ProjectItemGroupElement itemGroupElement, Microsoft.Build.Evaluation.LazyItemEvaluator lazyEvaluator) Line 1032 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator.Evaluate() Line 707 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator.Evaluate(Microsoft.Build.Evaluation.IEvaluatorData data, Microsoft.Build.Evaluation.Project project, Microsoft.Build.Construction.ProjectRootElement root, Microsoft.Build.Evaluation.ProjectLoadSettings loadSettings, int maxNodeCount, Microsoft.Build.Collections.PropertyDictionary environmentProperties, System.Collections.Generic.ICollection propertiesFromCommandLine, Microsoft.Build.BackEnd.Logging.ILoggingService loggingService, Microsoft.Build.Evaluation.IItemFactory itemFactory, Microsoft.Build.Evaluation.IToolsetProvider toolsetProvider, Microsoft.Build.FileSystem.IDirectoryCacheFactory directoryCacheFactory, Microsoft.Build.Evaluation.ProjectRootElementCacheBase projectRootElementCache, Microsoft.Build.Framework.BuildEventContext buildEventContext, Microsoft.Build.BackEnd.SdkResolution.ISdkResolverService sdkResolverService, int submissionId, Microsoft.Build.Evaluation.Context.EvaluationContext evaluationContext, bool interactive) Line 346 C#
Microsoft.Build.dll!Microsoft.Build.Execution.ProjectInstance.Initialize(Microsoft.Build.Construction.ProjectRootElement xml, System.Collections.Generic.IDictionary globalProperties, string explicitToolsVersion, string explicitSubToolsetVersion, int visualStudioVersionFromSolution, Microsoft.Build.Execution.BuildParameters buildParameters, Microsoft.Build.BackEnd.Logging.ILoggingService loggingService, Microsoft.Build.Framework.BuildEventContext buildEventContext, Microsoft.Build.BackEnd.SdkResolution.ISdkResolverService sdkResolverService, int submissionId, Microsoft.Build.Evaluation.ProjectLoadSettings? projectLoadSettings, Microsoft.Build.Evaluation.Context.EvaluationContext evaluationContext, Microsoft.Build.FileSystem.IDirectoryCacheFactory directoryCacheFactory) Line 3264 C#
```

This is during property function expansion. MSBuild is fairly faithfully exposing the behavior of `System.Version.Parse(string)` when passed a not-version string.

The RC2 related change appears to be https://github.com/dotnet/sdk/pull/50264/files#diff-d03cc3306a0d963151f7479546c175970b3a6fdb9771dcbddc2ba849f9359ae4R29.

https://github.com/dotnet/msbuild/blob/76aced20dd20a2831f7f7fa772857ce73f4f5e38/src/Build/Evaluation/Expander/WellKnownFunctions.cs#L856-L866

Today, the exception is caught here:

https://github.com/dotnet/msbuild/blob/76aced20dd20a2831f7f7fa772857ce73f4f5e38/src/Build/Evaluation/Expander.cs#L4070-L4079

And in this specific case it falls into the `LeavePropertiesUnexpandedOnError` case rather than throwing `InvalidProjectException`.

But MSBuild doesn't necessarily need to do that! We could instead call `System.Version.TryParse(string, out Version)` and return either the `Version` or the same behavior we have today, but without the exception.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.