Skip to content

fix for #3481 #3677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

philwalk
Copy link
Contributor

@philwalk philwalk commented May 13, 2025

This addresses issue #3481.

In Windows, when STDOUT is redirected to /dev/null an exception is thrown:

scala.bat -e "printf("""hello""")" > NUL
java.io.IOException: Error enabling ANSI output: GetConsoleMode error 6
  coursier.jniutils.WindowsAnsiTerminal.enableAnsiOutput(WindowsAnsiTerminal.java:25)
  scala.cli.ScalaCli$.main0(ScalaCli.scala:317)
  scala.cli.ScalaCli$.main(ScalaCli.scala:124)
  scala.cli.ScalaCli.main(ScalaCli.scala)

The fix, only in Windows, is to selectively ignore java.io.IOException if getMessage contains 'GetConsoleMode error 6.

Error 6 means "invalid handle", and is appropriate since stdout redirected to NUL is not a console handle.
Since everything sent to stdout is discarded, it shouldn't matter that Ansi mode is not enabled.

The documentation for GetConsoleScreenBufferInfo describes limitations.

Just noticed that the 2nd isWin in the exception handling is redundant.

@philwalk philwalk force-pushed the fix-3481-stdout-to-devnull-exception branch from 09c0b3a to d7f4a2a Compare May 13, 2025 20:07
@philwalk philwalk force-pushed the fix-3481-stdout-to-devnull-exception branch from d7f4a2a to fdee8a2 Compare May 13, 2025 21:19
@tgodzik tgodzik requested a review from Gedochao May 14, 2025 07:26
@philwalk
Copy link
Contributor Author

philwalk commented May 14, 2025

Here are manual test sessions demonstrating the fix:

philwalk@d5 MINGW64 ~
# scala-cli -e 'System.err.println("hello")'
Compiling project (Scala 3.7.0, JVM (17))
Compiled project (Scala 3.7.0, JVM (17))
hello

philwalk@d5 MINGW64 ~
# scala-cli -e 'System.err.println("hello")' > /dev/null
Compiling project (Scala 3.7.0, JVM (17))
Compiled project (Scala 3.7.0, JVM (17))
hello

in a CMD.EXE session:

C:\opt\ue>scala-cli.exe -e "System.err.println("""hello""")"
Compiling project (Scala 3.7.0, JVM (11))
Compiled project (Scala 3.7.0, JVM (11))
hello

C:\opt\ue>scala-cli.exe -e "System.err.println("""hello""")" > NUL
Compiling project (Scala 3.7.0, JVM (11))
Compiled project (Scala 3.7.0, JVM (11))
hello

@Gedochao
Copy link
Contributor

@philwalk do you think we could have an integration test for this (behind if Properties.isWin)? we do run tests on Windows runners.

@philwalk
Copy link
Contributor Author

philwalk commented May 16, 2025

do you think we could have an integration test for this (behind if Properties.isWin)?

How about if I copy a similar existing test and redirect stdout to NUL?

This one seems like a good candidate: "correctly run a scala snippet". Here's the modified copy:

  test("confirm that redirecting stdout to NUL doesn't throw an exception") {
    if (scala.util.Properties.isWin) {
      val nulPath = os.Path("NUL", os.pwd)
      emptyInputs.fromRoot { root =>
        val msg       = "Hello world"
        val quotation = TestUtil.argQuotationMark
        val res =
          os.proc(
            TestUtil.cli,
            "run",
            "--scala-snippet",
            s"object Hello extends App { System.err.println($quotation$msg$quotation) }",
            extraOptions
          )
            .call(cwd = root, stdout = nulPath)
        expect(res.out.trim() == msg)
      }
    }
  }

I haven't yet verified that the nullPath object does what we want, but you get the idea.

@philwalk
Copy link
Contributor Author

I can reproduce the problem from a Windows batch file, but it only fails when I run it manually.
Here's the one-line batch file:

::# contents of crash.bat:
scala-cli.exe  -e System.err.println("""Hello World""") --server=false > NUL

C:\opt\ue>crash.bat
Error: java.io.IOException: Error enabling ANSI output: GetConsoleMode error 6
For more details, please see 'C:\opt\ue\.scala-build\stacktraces\1747424901-506732676395670742.log'

If I spawn the same batch file from within a scala program, the failure is somehow avoided.
There seems to be some magic that occurs within sys.process and within os.proc.

scala script crashsysprocess.sc:

val msg = "Hello World"
import scala.sys.process.*
val stdout = new StringBuilder
val stderr = new StringBuilder
val status = Seq("cmd.exe", "/c.\\\\crash.bat") ! ProcessLogger(stdout append _, stderr append _)
val out = stdout.toString.trim
val err = stderr.toString.trim
printf("status[%s], stdout[%s], stderr[%s]\n", status, out, err)
assert(msg == err)

# ./crashsysprocess.sc
status[0], stdout[], stderr[Hello World]

scala script crashosproc.sc:

#!/usr/bin/env -S scala-cli shebang
//> using dep com.lihaoyi::os-lib:0.11.4
// crash.bat redirects stdout to NUL
// call `crash.bat` to demonstrate #3481
val res = os.proc(".\\crash.bat").call()
printf("res.out[%s]\n", res.out.trim())
printf("res.err[%s]\n", res.err.trim())

# ./crashosproc.sc
Compiling project (Scala 3.7.0, JVM (17))
Compiled project (Scala 3.7.0, JVM (17))
Hello World
res.out[]
res.err[]

All the various other things I have tried fail to reproduce this bug.
I have concluded that it's not possible with os.proc or scala.sys.process to redirect to Windows NUL

remove redundant `isWin` check

Co-authored-by: Piotr Chabelski <[email protected]>
@philwalk
Copy link
Contributor Author

I'm tempted to click the "Update branch" button, what's the recommended practice?
The message currently says:
image

@Gedochao
Copy link
Contributor

I'm tempted to click the "Update branch" button, what's the recommended practice?

Updating the branch will merge main to your branch / rebase on top of it. While it is recommended, do keep in mind it will also restart the CI.

@Gedochao
Copy link
Contributor

All the various other things I have tried fail to reproduce this bug.
I have concluded that it's not possible with os.proc or scala.sys.process to redirect to Windows NUL

Yeah, I thought that might be the case. I think we can live without a test then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants