Invalid property name passed at the command line does not emit error

[Forgind]

Issue Description

Specifying /p:Property:value instead of /p:Property=value leads to an aborted build when using dotnet build.

Steps to Reproduce

> dotnet new console
> dotnet build <csproj name> /p:UseRidGraph:false

Expected Behavior

One line for the MSBuild version followed by a line specifying that the property is not valid (MSB1006) then telling me which exactly was wrong like:
MSBuild version 17.9.0-dev-23579-01+5fcddc790 for .NET Framework
MSBUILD : error MSB1006: Property is not valid.
Switch: UseRidGraph:false

For switch syntax, type "MSBuild -help"

Actual Behavior

It just told me the version with no errors:
MSBuild version 17.8.3+195e7f5a3 for .NET

Analysis

I tried this with MSBuild.exe (from main, not 17.8.3), and it worked as expected. I tried dotnet MSBuild.dll with the MSBuild freshly built from main, and it worked as expected. I tried using MSBuild.exe from my VS preview (version 17.9.0-preview-23574-01+7b37a280a), and this bug still didn't reproduce. Then I built MSBuild main and used the deploy script to overwrite my 8.0.100 SDK, and this bug finally reproduced.

That means this may be Core-specific (a CLI bug?) or it may be specific to some component that MSBuild does not overwrite with the deploy script. Of note, it does still print out the version, which (I think) means MSBuild knows it's supposed to be executing and tries to execute but ultimately fails without logging anything further.

Versions & Configurations

8.0.100 SDK with MSBuild version above.

[rainersigwald]

We throw:

System.ArgumentException
  HResult=0x80070057
  Message=The name "UseRidGraph:false" contains an invalid character ":".
  Source=Microsoft.Build
  StackTrace:
   at Microsoft.Build.dll!Microsoft.Build.Shared.ErrorUtilities.ThrowArgument(System.Exception innerException, string resourceName, object[] args) Line 350
	at /_/src/Shared/ErrorUtilities.cs(350)
Microsoft.Build.dll!Microsoft.Build.Shared.XmlUtilities.VerifyThrowArgumentValidElementName(string name) Line 78
	at /_/src/Shared/XmlUtilities.cs(78)
Microsoft.Build.dll!Microsoft.Build.Execution.ProjectPropertyInstance.Create(string name, string escapedValue, bool mayBeReserved, Microsoft.Build.Construction.ElementLocation location, bool isImmutable, bool isEnvironmentProperty, Microsoft.Build.BackEnd.Logging.LoggingContext loggingContext) Line 312
	at /_/src/Build/Instance/ProjectPropertyInstance.cs(312)
Microsoft.Build.dll!Microsoft.Build.Evaluation.ProjectCollection.ProjectCollection(System.Collections.Generic.IDictionary<string, string> globalProperties, System.Collections.Generic.IEnumerable<Microsoft.Build.Framework.ILogger> loggers, System.Collections.Generic.IEnumerable<Microsoft.Build.Logging.ForwardingLoggerRecord> remoteLoggers, Microsoft.Build.Evaluation.ToolsetDefinitionLocations toolsetDefinitionLocations, int maxNodeCount, bool onlyLogCriticalEvents, bool loadProjectsReadOnly, bool useAsynchronousLogging, bool reuseProjectRootElementCache) Line 343
	at /_/src/Build/Definition/ProjectCollection.cs(343)
MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.BuildProject(string projectFile, string[] targets, string toolsVersion, System.Collections.Generic.Dictionary<string, string> globalProperties, System.Collections.Generic.Dictionary<string, string> restoreProperties, Microsoft.Build.Framework.ILogger[] loggers, Microsoft.Build.Framework.LoggerVerbosity verbosity, Microsoft.Build.CommandLine.DistributedLoggerRecord[] distributedLoggerRecords, int cpuCount, bool enableNodeReuse, System.IO.TextWriter preprocessWriter, System.IO.TextWriter targetsWriter, bool detailedSummary, System.Collections.Generic.ISet<string> warningsAsErrors, System.Collections.Generic.ISet<string> warningsNotAsErrors, System.Collections.Generic.ISet<string> warningsAsMessages, bool enableRestore, Microsoft.Build.Logging.ProfilerLogger profilerLogger, bool enableProfiler, bool interactive, Microsoft.Build.Execution.ProjectIsolationMode isolateProjects, Microsoft.Build.Graph.GraphBuildOptions graphBuildOptions, bool lowPriority, bool question, string[] inputResultsCaches, string outputResultsCache, bool saveProjectResult, ref Microsoft.Build.Execution.BuildResult result, string[] commandLine) Line 1334
	at /_/src/MSBuild/XMake.cs(1334)
MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.Execute(string[] commandLine) Line 822
	at /_/src/MSBuild/XMake.cs(822)
MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.Main(string[] args) Line 268
	at /_/src/MSBuild/XMake.cs(268)
Microsoft.DotNet.Cli.Utils.dll!Microsoft.DotNet.Cli.Utils.MSBuildForwardingAppWithoutLogging.ExecuteInProc(string[] arguments) Line 120
	at Microsoft.DotNet.Cli.Utils\MSBuildForwardingAppWithoutLogging.cs(120)
dotnet.dll!Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingApp.Execute() Line 90
	at Microsoft.DotNet.Tools.MSBuild\MSBuildForwardingApp.cs(90)
dotnet.dll!Microsoft.DotNet.Tools.RestoringCommand.Execute() Line 99
	at Microsoft.DotNet.Tools\RestoringCommand.cs(99)

That is caught and a new exception thrown

Microsoft.Build.Exceptions.InvalidProjectFileException
  HResult=0x80131500
  Message=Invalid property. The name "UseRidGraph:false" contains an invalid character ":".
  Source=<Cannot evaluate the exception source>
  StackTrace:
[Managed to Native Transition]
Microsoft.Build.dll!Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject(string errorSubCategoryResourceName, Microsoft.Build.Shared.IElementLocation elementLocation, string resourceName, object[] args) Line 268
	at /_/src/Shared/ProjectErrorUtilities.cs(268)
Microsoft.Build.dll!Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject<System.__Canon>(Microsoft.Build.Shared.IElementLocation elementLocation, string resourceName, System.__Canon arg0) Line 41
	at /_/src/Shared/ProjectErrorUtilities.cs(41)
Microsoft.Build.dll!Microsoft.Build.Evaluation.ProjectCollection.ProjectCollection(System.Collections.Generic.IDictionary<string, string> globalProperties, System.Collections.Generic.IEnumerable<Microsoft.Build.Framework.ILogger> loggers, System.Collections.Generic.IEnumerable<Microsoft.Build.Logging.ForwardingLoggerRecord> remoteLoggers, Microsoft.Build.Evaluation.ToolsetDefinitionLocations toolsetDefinitionLocations, int maxNodeCount, bool onlyLogCriticalEvents, bool loadProjectsReadOnly, bool useAsynchronousLogging, bool reuseProjectRootElementCache) Line 349
	at /_/src/Build/Definition/ProjectCollection.cs(349)
MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.BuildProject(string projectFile, string[] targets, string toolsVersion, System.Collections.Generic.Dictionary<string, string> globalProperties, System.Collections.Generic.Dictionary<string, string> restoreProperties, Microsoft.Build.Framework.ILogger[] loggers, Microsoft.Build.Framework.LoggerVerbosity verbosity, Microsoft.Build.CommandLine.DistributedLoggerRecord[] distributedLoggerRecords, int cpuCount, bool enableNodeReuse, System.IO.TextWriter preprocessWriter, System.IO.TextWriter targetsWriter, bool detailedSummary, System.Collections.Generic.ISet<string> warningsAsErrors, System.Collections.Generic.ISet<string> warningsNotAsErrors, System.Collections.Generic.ISet<string> warningsAsMessages, bool enableRestore, Microsoft.Build.Logging.ProfilerLogger profilerLogger, bool enableProfiler, bool interactive, Microsoft.Build.Execution.ProjectIsolationMode isolateProjects, Microsoft.Build.Graph.GraphBuildOptions graphBuildOptions, bool lowPriority, bool question, string[] inputResultsCaches, string outputResultsCache, bool saveProjectResult, ref Microsoft.Build.Execution.BuildResult result, string[] commandLine) Line 1334
	at /_/src/MSBuild/XMake.cs(1334)
MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.Execute(string[] commandLine) Line 822
	at /_/src/MSBuild/XMake.cs(822)
MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.Main(string[] args) Line 268
	at /_/src/MSBuild/XMake.cs(268)
Microsoft.DotNet.Cli.Utils.dll!Microsoft.DotNet.Cli.Utils.MSBuildForwardingAppWithoutLogging.ExecuteInProc(string[] arguments) Line 120
	at Microsoft.DotNet.Cli.Utils\MSBuildForwardingAppWithoutLogging.cs(120)
dotnet.dll!Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingApp.Execute() Line 90
	at Microsoft.DotNet.Tools.MSBuild\MSBuildForwardingApp.cs(90)
dotnet.dll!Microsoft.DotNet.Tools.RestoringCommand.Execute() Line 99
	at Microsoft.DotNet.Tools\RestoringCommand.cs(99)

That's caught, logged, and rethrown

msbuild/src/Build/Definition/ProjectCollection.cs

Lines 356 to 360 in 5fcddc7

	catch (InvalidProjectFileException ex2)
	{
	BuildEventContext buildEventContext = new BuildEventContext(0 /* node ID */, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTaskId);
	LoggingService.LogInvalidProjectFileError(buildEventContext, ex2);
	throw;

That's caught, the logger shut down, and rethrown.

I would expect the logger shutdown to flush the message.

[rainersigwald]

Ah, but there are no loggers, so nowhere for that message to go.

[rainersigwald]

Ah but in dotnet msbuild /p:UseRidGraph:false the first exception is at

Microsoft.Build.CommandLine.CommandLineSwitchException
  HResult=0x80131500
  Message=MSBUILD : error MSB1006: Property is not valid.
Switch: UseRidGraph:false
  Source=MSBuild
  StackTrace:
   at >	MSBuild.dll!Microsoft.Build.CommandLine.CommandLineSwitchException.Throw(string messageResourceName, string commandLineArg, string[] messageArgs) Line 154	C#
 	MSBuild.dll!Microsoft.Build.CommandLine.CommandLineSwitchException.Throw(string messageResourceName, string commandLineArg) Line 139	C#
 	MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.ProcessPropertySwitch(string[] parameters) Line 3630	C#
 	MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.ProcessCommandLineSwitches(Microsoft.Build.CommandLine.CommandLineSwitches switchesFromAutoResponseFile, Microsoft.Build.CommandLine.CommandLineSwitches switchesNotFromAutoResponseFile, ref string projectFile, ref string[] targets, ref string toolsVersion, ref System.Collections.Generic.Dictionary<string, string> globalProperties, ref Microsoft.Build.Framework.ILogger[] loggers, ref Microsoft.Build.Framework.LoggerVerbosity verbosity, ref Microsoft.Build.Framework.LoggerVerbosity originalVerbosity, ref System.Collections.Generic.List<Microsoft.Build.CommandLine.DistributedLoggerRecord> distributedLoggerRecords, ref int cpuCount, ref bool enableNodeReuse, ref System.IO.TextWriter preprocessWriter, ref System.IO.TextWriter targetsWriter, ref bool detailedSummary, ref System.Collections.Generic.ISet<string> warningsAsErrors, ref System.Collections.Generic.ISet<string> warningsNotAsErrors, ref System.Collections.Generic.ISet<string> warningsAsMessages, ref bool enableRestore, ref bool interactive, ref Microsoft.Build.Logging.ProfilerLogger profilerLogger, ref bool enableProfiler, ref System.Collections.Generic.Dictionary<string, string> restoreProperties, ref Microsoft.Build.Execution.ProjectIsolationMode isolateProjects, ref Microsoft.Build.Graph.GraphBuildOptions graphBuild, ref string[] inputResultsCaches, ref string outputResultsCache, ref bool lowPriority, ref bool question, ref string[] getProperty, ref string[] getItem, ref string[] getTargetResult, bool recursing, string commandLine) Line 2566	C#
 	MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.ProcessCommandLineSwitches(Microsoft.Build.CommandLine.CommandLineSwitches switchesFromAutoResponseFile, Microsoft.Build.CommandLine.CommandLineSwitches switchesNotFromAutoResponseFile, ref string projectFile, ref string[] targets, ref string toolsVersion, ref System.Collections.Generic.Dictionary<string, string> globalProperties, ref Microsoft.Build.Framework.ILogger[] loggers, ref Microsoft.Build.Framework.LoggerVerbosity verbosity, ref Microsoft.Build.Framework.LoggerVerbosity originalVerbosity, ref System.Collections.Generic.List<Microsoft.Build.CommandLine.DistributedLoggerRecord> distributedLoggerRecords, ref int cpuCount, ref bool enableNodeReuse, ref System.IO.TextWriter preprocessWriter, ref System.IO.TextWriter targetsWriter, ref bool detailedSummary, ref System.Collections.Generic.ISet<string> warningsAsErrors, ref System.Collections.Generic.ISet<string> warningsNotAsErrors, ref System.Collections.Generic.ISet<string> warningsAsMessages, ref bool enableRestore, ref bool interactive, ref Microsoft.Build.Logging.ProfilerLogger profilerLogger, ref bool enableProfiler, ref System.Collections.Generic.Dictionary<string, string> restoreProperties, ref Microsoft.Build.Execution.ProjectIsolationMode isolateProjects, ref Microsoft.Build.Graph.GraphBuildOptions graphBuild, ref string[] inputResultsCaches, ref string outputResultsCache, ref bool lowPriority, ref bool question, ref string[] getProperty, ref string[] getItem, ref string[] getTargetResult, bool recursing, string commandLine) Line 2691	C#
 	MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.Execute(string[] commandLine) Line 721	C#
 	MSBuild.dll!Microsoft.Build.CommandLine.MSBuildApp.Main(string[] args) Line 268	C#
 	Microsoft.DotNet.Cli.Utils.dll!Microsoft.DotNet.Cli.Utils.MSBuildForwardingAppWithoutLogging.ExecuteInProc(string[] arguments)	Unknown
 	dotnet.dll!Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingApp.Execute()	Unknown

msbuild/src/MSBuild/XMake.cs

Lines 3633 to 3635 in 5fcddc7

	// check that the property name is not blank, and the property has a value
	CommandLineSwitchException.VerifyThrow((parameterSections[0].Length > 0) && (parameterSections.Length == 2),
	"InvalidPropertyError", parameter);

In the dotnet build /p:UseRidGraph:false case, what MSBuild is actually passed is

--property:UseRidGraph:false=

[rainersigwald]

The proximate cause of this bug is dotnet/sdk#37230, but you can repro the no-logger-so-no-error problem with any MSBuild by passing /p:UseRidGraph:false=asdf.

[ShreyasJejurkar]

Not able to reproduce this on 8.

[Forgind]

Not able to reproduce this on 8.

It looks like you successfully reproduced the problem. /p:UseRidGraph:false is incorrect, yet we fail without giving any indication of that failure. You can see the initial message telling you that MSBuild is trying to execute, but then it fails and exits without logging anything else. This is a bug in error reporting, not a bug in outcome, and you reproduced it.