Fix redirection of stdout stream to stderr when using show
#2689
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2680
When you use
show
, we want to ensure that nothing goes tostdout
except the JSON result of the task shown, so it can easily be piped and parsed and handled by external programs. This was likely broken in #2428.The basic problem is that
PrintLogger
andPrefixLogger
are different types, with onlyPrintLogger
supportingwithOutputStream
but it quickly gets wrapped inPrefixLogger
when passed around inside Mill, meaningMainModule.show0
sees that it's not an instance ofPrintLogger
and skips withwithOutputStream
.This fix in this PR is to move
withOutputStream
toColorLogger
, and implement it inPrefixLogger
as well.Tested manually by via the patch and command below. On
main
,log.txt
contains theCOMPILING
line in addition to the JSON blob (the bug above), and most of the Scala compiler output just disappears (which is also a bug!), and most of stdout goes to stderr (another bug!). On this PR, the Scala compiler output andCOMPILING
are both properly shown in the console, while thelog.txt
file contains only the JSON dictionary{"analysisFile": "...", "classes": "..."}
and nothing elseAlso updated the unit tests in
mill.main.MainModuleTests
to assert separately on the contents ofstdout
andstderr
after runningshow
, to ensure that exactly the right things end up in each place.mill.integration.WatchSourceInputTests
also needed to be updated to properly assert on the expected stdout/stderr whenshow
is givenThere's probably more cleanup we can do w.r.t.
ColorLogger
/PrintLogger
/PrefixLogger
, but this PR just fixes the immediate problems for now and leaves better structuring of theLogger
class hierarchy for later