printlns interfere with show #2680
Milestone
Comments
|
This is a regression, possibly related to https://github.com/com-lihaoyi/mill/pull/2428/files#diff-10fa43b77073c51ec13ec41ca13cf84707d831a39d344acad2d08a6f9bb1ee14. Needs to be fixed so that |
lihaoyi
added a commit
that referenced
this issue
Aug 8, 2023
Fixes #2680 When you use `show`, we want to ensure that nothing goes to `stdout` 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` and `PrefixLogger` are different types, with only `PrintLogger` supporting `withOutputStream` but it quickly gets wrapped in `PrefixLogger` when passed around inside Mill, meaning `MainModule.show0` sees that it's not an instance of `PrintLogger` and skips with `withOutputStream`. This fix in this PR is to move `withOutputStream` to `ColorLogger`, and implement it in `PrefixLogger` as well. Tested manually by via the patch and command below. On `main`, `log.txt` contains the `COMPILING` 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 and `COMPILING` are both properly shown in the console, while the `log.txt` file contains only the JSON dictionary `{"analysisFile": "...", "classes": "..."}` and nothing else ```diff --- a/example/basic/1-simple-scala/build.sc +++ b/example/basic/1-simple-scala/build.sc @@ -8,6 +8,11 @@ object foo extends RootModule with ScalaModule { ivy"com.lihaoyi::mainargs:0.4.0" ) + def compile = T{ + println("COMPILING") + super.compile() + } + object test extends ScalaTests { def ivyDeps = Agg(ivy"com.lihaoyi::utest:0.7.11") def testFramework = "utest.runner.Framework" ``` ``` echo // >> example/basic/1-simple-scala/src/Foo.scala && ./mill -i dev.run example/basic/1-simple-scala -i show compile > log.txt ``` Also updated the unit tests in `mill.main.MainModuleTests` to assert separately on the contents of `stdout` and `stderr` after running `show`, 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 when `show` is given There'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 the `Logger` class hierarchy for later
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Briefly discussed this in discord
reproduced at miguel-vila/mill-issue-repr
expected behaviour:
mill show foo.testTaskto be valid json without the result of any printlnworks in 0.10.1 , breaks in 0.11.0 and 0.11.1
The text was updated successfully, but these errors were encountered: