Don't expand list comparisons against empty strings in logging

[dsplaisted]

Context: dotnet/roslyn#46445

AFAIK, we special case evaluation of conditions such as '@(ReferencePathWithRefAssemblies)' == '' to avoid expanding the item list. However, if we are logging with a verbose enough level (or binlogging, probably), then we will end up expanding the item list anyway. To avoid this, projects can use the following instead: '@(ReferencePathWithRefAssemblies->Count())' == '0'

If possible, we should consider special-casing the log message in cases like this to also avoid expanding the item list.

[marcpopMSFT]

@ladipro thoughts on whether this is a perf fix worth looking into? Can we improve perf outside of the logging scenario?

[JaynieBai]

The related code should be

msbuild/src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs

Lines 369 to 391 in 58c6723

	// Expand the expression for the Log. Since we know the condition evaluated to false, leave unexpandable properties in the condition so as not to cause an error
	string expanded = _expander.ExpandIntoStringAndUnescape(_target.Condition, ExpanderOptions.ExpandPropertiesAndItems | ExpanderOptions.LeavePropertiesUnexpandedOnError | ExpanderOptions.Truncate, _target.ConditionLocation);
	// By design: Not building dependencies. This is what NAnt does too.
	// NOTE: In the original code, this was logged from the target logging context. However, the target
	// hadn't been "started" by then, so you'd get a target message outside the context of a started
	// target. In the Task builder (and original Task Engine), a Task Skipped message would be logged in
	// the context of the target, not the task. This should be the same, especially given that we
	// wish to allow batching on the condition of a target.
	var skippedTargetEventArgs = new TargetSkippedEventArgs(message: null)
	{
	BuildEventContext = projectLoggingContext.BuildEventContext,
	TargetName = _target.Name,
	TargetFile = _target.Location.File,
	ParentTarget = ParentEntry?.Target?.Name,
	BuildReason = BuildReason,
	SkipReason = TargetSkipReason.ConditionWasFalse,
	Condition = _target.Condition,
	EvaluatedCondition = expanded
	};
	projectLoggingContext.LogBuildEvent(skippedTargetEventArgs);
	}

[JaynieBai]

@AR-May Since the fix #5553. The expanding the item list string has been truncated. I think this issue has been fixed.
Project sample:

<Project>
  <ItemGroup>
    <theItem Include="ThisIsAFairlyLongFileName_0.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_1.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_2.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_3.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_4.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_5.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_6.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_7.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_8.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_9.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_10.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_11.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_12.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_13.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_14.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_15.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_16.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_17.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_18.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_19.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_20.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_21.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_22.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_23.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_24.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_25.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_26.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_27.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_28.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_29.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_30.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_31.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_32.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_33.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_34.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_35.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_36.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_37.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_38.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_39.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_40.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_41.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_42.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_43.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_44.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_45.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_46.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_47.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_48.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_49.bmp"/>
    <theItem Include="ThisIsAFairlyLongFileName_50.bmp"/>
</ItemGroup>
<Target Name = "Message"  Condition ="'@(theItem)' == ''">
    <Message Text="@(theItem)" />
</Target>
</Project>

MSBuild the project with -v:diag

The output log will be Target "Message" skipped, due to false condition; ('@(theItem)' == '') was evaluated as ('ThisIsAFairlyLongFileName_0.bmp;ThisIsAFairlyLongFileName_1.bmp;ThisIsAFairlyLongFileName_2.bmp;...' == '').

[AR-May]

I think it is fixed as well.