-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.Function<Microsoft.Build.Execution.ProjectPropertyInstance>.Execute(object objectInstance, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> properties, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation) Line 4061 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Expander<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.PropertyExpander<Microsoft.Build.Execution.ProjectPropertyInstance>.ExpandPropertyBody(string propertyBody, object propertyValue, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> 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<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.PropertyExpander<Microsoft.Build.Execution.ProjectPropertyInstance>.ExpandPropertiesLeaveTypedAndEscaped(string expression, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> 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<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.PropertyExpander<Microsoft.Build.Execution.ProjectPropertyInstance>.ExpandPropertiesLeaveEscaped(string expression, Microsoft.Build.Evaluation.IPropertyProvider<Microsoft.Build.Execution.ProjectPropertyInstance> 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<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance>.ExpandIntoStringLeaveEscaped(string expression, Microsoft.Build.Evaluation.ExpanderOptions options, Microsoft.Build.Shared.IElementLocation elementLocation) Line 497 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.ProcessMetadataElements(Microsoft.Build.Construction.ProjectItemElement itemElement, Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.OperationBuilderWithMetadata operationBuilder) Line 642 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.BuildIncludeOperation(string rootDirectory, Microsoft.Build.Construction.ProjectItemElement itemElement, bool conditionResult) Line 576 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.ProcessItemElement(string rootDirectory, Microsoft.Build.Construction.ProjectItemElement itemElement, bool conditionResult) Line 512 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.EvaluateItemElement(bool itemGroupConditionResult, Microsoft.Build.Construction.ProjectItemElement itemElement, Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance> lazyEvaluator) Line 1338 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.EvaluateItemGroupElement(Microsoft.Build.Construction.ProjectItemGroupElement itemGroupElement, Microsoft.Build.Evaluation.LazyItemEvaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance> lazyEvaluator) Line 1032 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.Evaluate() Line 707 C#
Microsoft.Build.dll!Microsoft.Build.Evaluation.Evaluator<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance>.Evaluate(Microsoft.Build.Evaluation.IEvaluatorData<Microsoft.Build.Execution.ProjectPropertyInstance, Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectMetadataInstance, Microsoft.Build.Execution.ProjectItemDefinitionInstance> data, Microsoft.Build.Evaluation.Project project, Microsoft.Build.Construction.ProjectRootElement root, Microsoft.Build.Evaluation.ProjectLoadSettings loadSettings, int maxNodeCount, Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance> environmentProperties, System.Collections.Generic.ICollection<string> propertiesFromCommandLine, Microsoft.Build.BackEnd.Logging.ILoggingService loggingService, Microsoft.Build.Evaluation.IItemFactory<Microsoft.Build.Execution.ProjectItemInstance, Microsoft.Build.Execution.ProjectItemInstance> 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<string, string> 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.
msbuild/src/Build/Evaluation/Expander/WellKnownFunctions.cs
Lines 856 to 866 in 76aced2
else if (receiverType == typeof(Version)) | |
{ | |
if (string.Equals(methodName, nameof(Version.Parse), StringComparison.OrdinalIgnoreCase)) | |
{ | |
if (ParseArgs.TryGetArg(args, out string? arg0) && arg0 != null) | |
{ | |
returnVal = Version.Parse(arg0); | |
return true; | |
} | |
} | |
} |
Today, the exception is caught here:
msbuild/src/Build/Evaluation/Expander.cs
Lines 4070 to 4079 in 76aced2
catch (Exception ex) | |
{ | |
string partiallyEvaluated = GenerateStringOfMethodExecuted(_expression, objectInstance, _methodMethodName, args); | |
if (options.HasFlag(ExpanderOptions.LeavePropertiesUnexpandedOnError)) | |
{ | |
return partiallyEvaluated; | |
} | |
ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "InvalidFunctionPropertyExpression", partiallyEvaluated, ex.Message.Replace("\r\n", " ")); | |
} |
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.