Fix issue 7828: enabling the binary logger adds console output#7989
Fix issue 7828: enabling the binary logger adds console output#7989Forgind merged 10 commits intodotnet:mainfrom
Conversation
rainersigwald
left a comment
There was a problem hiding this comment.
Can you please add a regression test, using RunnerUtilities.ExecMSBuild like
that validates the stdout output?
src/MSBuild/XMake.cs
Outdated
| } | ||
|
|
||
| if (verbosity == LoggerVerbosity.Diagnostic) | ||
| if (verbosity == LoggerVerbosity.Diagnostic && originalVerbosity != LoggerVerbosity.Quiet) |
There was a problem hiding this comment.
Maybe just:
| if (verbosity == LoggerVerbosity.Diagnostic && originalVerbosity != LoggerVerbosity.Quiet) | |
| if (originalVerbosity == LoggerVerbosity.Diagnostic) |
?
There was a problem hiding this comment.
when bl is set and other verbosity than quiet/diagnostic is set, the output is still wanted.
There was a problem hiding this comment.
This is interesting. My instinct was to suppress the message for "quiet", "minimal" and "normal" but leave it for "detailed" and "diagnostic". Why "quiet" only?
There was a problem hiding this comment.
Hmm... we need more requirement clarifications. My understanding of the requirement is solely from the #7828 statement. It seems we three have different understandings, let me know how we can make an agreement. @rainersigwald / @Forgind
There was a problem hiding this comment.
@Forgind and I actually talked about this this morning, and I think it's ok for it to be there for "diagnostic" only.
|
|
|
|
|
Hi @Forgind / @rainersigwald, the failed test case is from MultiplePlugins, should not be related to my change, what next step do you recommend? Thanks Microsoft.Build.Engine.UnitTests.ProjectCache.ProjectCacheTests.MultiplePlugins |
|
@fanhaipeng, I reran it, and it passed. /cc: @dfederm for awareness on the flaky test. |
rainersigwald
left a comment
There was a problem hiding this comment.
I'd like to hold this for 17.5/7.0.200 since it's a fairly visible behavior change, but this looks good, thanks!
|
Thanks @fanhaipeng! |
Fixes #7828
Context
Binary logger option overrides verbosity by diagnostic and prints a line even verbosity is set to quiet.
Changes Made
The fix introduces a variable to store the original verbosity, if the "original verbosity" is quiet, it suppresses the console output when binary logger is requested.
Testing
Notes