fix for #11424 #11425

philwalk · 2021-02-15T20:29:02Z

this fixes #11424
manually verified for currently defined dotc script-only options:

-save
-compile-only

liufengyun · 2021-02-16T07:58:14Z

I think we need to have this in RC1, given that it is user-facing for anyone trying the repl.

michelou · 2021-02-16T08:10:13Z

I think we need to have this in RC1, given that it is user-facing for anyone trying the repl.

Note also :

line 126 : java
line 151 : $JAVACMD

Question : Did anybody test script file scala
on Mingw64 (Windows), Cygwin (Windows) or any other flavor of Linux ?!

liufengyun · 2021-02-16T08:16:16Z

Good catch @michelou . We have a test for distribution https://github.com/lampepfl/packtest. I think it's time to have that test as part of CI.

philwalk · 2021-02-16T16:17:12Z

@michelou - I tested in these environments:

Linux x 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Linux x 5.4.0-62-generic #70~18.04.1-Ubuntu SMP Tue Jan 12 17:18:00 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
CYGWIN_NT-10.0 x 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin

Although my msys environment is possibly misconfigured, it's broken independent of this PR.
The REPL doesn't appear to be working in general on msys/mingw.

$ uname -a
MINGW64_NT-10.0-19042 x 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
$ scala
Starting scala3 REPL...
Exception in thread "main" java.lang.IllegalStateException: Unable to create a system terminal
        at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:317)
        at org.jline.terminal.TerminalBuilder.build(TerminalBuilder.java:263)
        at dotty.tools.repl.JLineTerminal.<init>(JLineTerminal.scala:23)
        at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:108)
        at dotty.tools.repl.Main$.main(Main.scala:6)
        at dotty.tools.repl.Main.main(Main.scala)
        Suppressed: com.sun.jna.LastErrorException: [6] The handle is invalid.
                at com.sun.jna.Native.invokeVoid(Native Method)
                at com.sun.jna.Function.invoke(Function.java:415)
                at com.sun.jna.Function.invoke(Function.java:361)
                at com.sun.jna.Library$Handler.invoke(Library.java:265)
                at org.jline.terminal.impl.jna.win.$Proxy0.GetConsoleMode(Unknown Source)
                at org.jline.terminal.impl.jna.win.JnaWinSysTerminal.createTerminal(JnaWinSysTerminal.java:40)
                at org.jline.terminal.impl.jna.JnaSupportImpl.winSysTerminal(JnaSupportImpl.java:39)
                at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:326)
                ... 5 more
        Suppressed: java.util.NoSuchElementException
                at java.base/java.util.ServiceLoader$2.next(ServiceLoader.java:1308)
                at java.base/java.util.ServiceLoader$2.next(ServiceLoader.java:1296)
                at java.base/java.util.ServiceLoader$3.next(ServiceLoader.java:1394)
                at org.jline.terminal.TerminalBuilder.load(TerminalBuilder.java:487)
                at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:334)
                ... 5 more

michelou · 2021-02-16T16:45:13Z

@michelou - I tested in these environments:

Linux x 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Linux x 5.4.0-62-generic #70~18.04.1-Ubuntu SMP Tue Jan 12 17:18:00 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
CYGWIN_NT-10.0 x 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin

Although my msys environment is possibly misconfigured, it's broken independent of this PR.
The REPL doesn't appear to be working in general on msys/mingw.

$ uname -a
MINGW64_NT-10.0-19042 x 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
$ scala
Starting scala3 REPL...
Exception in thread "main" java.lang.IllegalStateException: Unable to create a system terminal
        at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:317)
        at (TerminalBuilder.java:263)
        [...]
                ... 5 more

@philwalk I know the issue (I was waiting for someone else to also report it). In my case I made the following local change on line 119 of file dist/bin/common :

[[ $mingw ]] || JNA=$(find_lib "*jna-5*")

instead of

JNA=$(find_lib "*jna-5*")

philwalk · 2021-02-17T15:41:11Z

@michelou - I can do a PR for the $mingw fix if it's not already in the works

michelou · 2021-02-17T16:01:57Z

@michelou - I can do a PR for the $mingw fix if it's not already in the works

@philwalk I guess my fix works for you as well.
Then you can do a PR (I'm myself focusing on scala.bat and scaladoc.bat for Windows users).

PS. To remind us : PR #5444 (closed, november 2018).

philwalk · 2021-02-18T23:49:01Z

@michelou the fix worked for MINGW but not for MSYS2, so I haven't done the PR yet.
The MSYS2 stack is similar but not identical (see below).

A possible fix that works for both MINGW and MSYS2:

diff --git a/compiler/src/dotty/tools/repl/JLineTerminal.scala b/compiler/src/dotty/tools/repl/JLineTerminal.scala
index 916df851cb..7110af5605 100644
--- a/compiler/src/dotty/tools/repl/JLineTerminal.scala
+++ b/compiler/src/dotty/tools/repl/JLineTerminal.scala
@@ -20,10 +20,12 @@ final class JLineTerminal extends java.io.Closeable {

   private val terminal =
     TerminalBuilder.builder()
-    .dumb(false) // fail early if not able to create a terminal
+    .dumb(dumbRequested) // fail early if not able to create a terminal
     .build()
   private val history = new DefaultHistory

+  def dumbRequested = Option(System.getenv("TERM")) == Some("dumb")
+
   private def blue(str: String)(using Context) =
     if (ctx.settings.color.value != "never") Console.BLUE + str + Console.RESET
     else str

$ uname -a
MSYS_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys
philwalk@d5:~/workspace/dotty
$ scala
Starting scala3 REPL...
Exception in thread "main" java.lang.IllegalStateException: Unable to create a system terminal
        at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:317)
        at org.jline.terminal.TerminalBuilder.build(TerminalBuilder.java:263)
        at dotty.tools.repl.JLineTerminal.<init>(JLineTerminal.scala:23)
        at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:108)
        at dotty.tools.repl.Main$.main(Main.scala:6)
        at dotty.tools.repl.Main.main(Main.scala)
        Suppressed: java.lang.NoClassDefFoundError: com/sun/jna/LastErrorException
                at org.jline.terminal.impl.jna.JnaSupportImpl.isWindowsConsole(JnaSupportImpl.java:44)
                at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:325)
                ... 5 more
        Caused by: java.lang.ClassNotFoundException: com.sun.jna.LastErrorException
                at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
                at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
                at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
                ... 7 more
        Suppressed: java.util.NoSuchElementException
                at java.base/java.util.ServiceLoader$2.next(ServiceLoader.java:1308)
                at java.base/java.util.ServiceLoader$2.next(ServiceLoader.java:1296)
                at java.base/java.util.ServiceLoader$3.next(ServiceLoader.java:1394)
                at org.jline.terminal.TerminalBuilder.load(TerminalBuilder.java:487)
                at org.jline.terminal.TerminalBuilder.doBuild(TerminalBuilder.java:334)
                ... 5 more

michelou · 2021-02-19T09:16:32Z

@philwalk Here is a first observation : the shell script dist/bin/common currently handles the 3 cases cygwin, mingw and darwin (lines 7-9). Did you consider adding a case msys ?

In my case I get MINGW... when running uname with my MSYS2 (64-bit) installation. I need to search how one can get MSYS... when running uname. Can you provide me the download link of the software installer you used to set up your MSYS environment ?

PS. Maybe I should also update my GitHub project dotty-examples to describe how my Scala code examples can be build/run with Cygwin/MSYS2.

philwalk · 2021-02-19T12:50:20Z

@michelou
I get mingw here: https://gitforwindows.org/, also called git-bash by conemu.
I get msys2 here: https://www.msys2.org/
Here's a comparison of msys2 with git-for-windows:
https://github.com/git-for-windows/build-extra#the-difference-between-msys2-and-mingw

Both msys2 and mingw report the same thing (MSys) in response to uname -o
On my system, all three cygwin, mingw and msys2 provide cygpath, as shown here:

$ uname
MINGW64_NT-10.0-19042
$ cygpath -m /
C:/opt/gitbash/

$ uname
MSYS_NT-10.0-19042
$ cygpath -m /
C:/msys64/

$ uname
CYGWIN_NT-10.0
$ cygpath -m /
C:/cygwin64

So a simplification to dist/bin/common would be:

# winshell=true if any of $cygwin, $msys, or $mingw are true

# For Cygwin, Mingw and MSys, switch paths to Windows-mixed format before running java
if $winshell; then
  [ -n "$PROG_HOME" ] &&
    PROG_HOME=`cygpath -am "$PROG_HOME"`
  [ -n "$JAVA_HOME" ] &&
    JAVA_HOME=`cygpath -am "$JAVA_HOME"`
  CLASSPATH_SUFFIX=";"
  PSEP=";"
fi

The advantage of using cygpath is that, in addition to changing \ to /, it incorporates mounted paths in /etc/fstab.

The same change would happen here as well:

find_lib () {
  local lib=$(find $PROG_HOME/lib/ -name "$1")
if $winshell; then
    cygpath -am $lib
  else
    echo $lib
  fi
}```

michelou · 2021-02-19T21:41:33Z

@philwalk It's also worth looking at the Unix template (tool-unix.tmpl) used for generating the Scala 2 bash scripts scala and scalac :

it defines/uses the three variables cygwin, mingw and msys (see line 64 ff)
it performs some action in relation with the usage of Jline terminal (see line 109)

IMO introducing variable msys is a good idea.
But two questions now have to be examined :

why is currently the mingw case (line 73) handled differently from cygwin (line 83) in file dist/bin/common ?
would the option WINDOWS_OPT="-Djline.terminal=unix (line 113) solve our concern ?

In conclusion we need to investigate further.

philwalk · 2021-02-20T18:16:15Z

@michelou
I incorporated the scala2 approach and it tests well.

why is currently the mingw case (line 73) handled differently from cygwin (line 83) in file dist/bin/common ?

The differences I have discovered between cygwin and either mingw or msys are these:

cygwin always provides the cygpath executable for converting between posix-like and windows paths
early versions (and possibly some current versions?) of mingw and msys did not provide cygpath, although many now do

The problem that dist/bin/common (line 73) addresses is better handled by cygpath if it's available. The difference is that (line 73) only converts between \ and / but the /etc/fstab file can also cause require directory mounts to be translated. The cygpath tool handles both the / path separator and /etc/fstab mounts.

Here's an approach that seems likely to provide a better result.

$ has_cygpath=false
$ if `which cygpath.exe > /dev/null 2>&1`; then has_cygpath=true ; fi

Then, instead of dist/bin/common (lines 73-105):

# For Cygwin, switch paths to Windows-mixed format before running java
if $has_cygpath; then
  [ -n "$PROG_HOME" ] &&
    PROG_HOME=`cygpath -am "$PROG_HOME"`
  [ -n "$JAVA_HOME" ] &&
    JAVA_HOME=`cygpath -am "$JAVA_HOME"`
  CLASSPATH_SUFFIX=";"
  PSEP=";"
elif ($mingw || $msys); then
  # For Mingw / Msys, ensure paths are in UNIX format before anything is touched
  [ -n "$PROG_HOME" ] &&
    PROG_HOME="`(cd "$PROG_HOME"; pwd -W | sed 's|/|\\\\|g')`"
  [ -n "$JAVA_HOME" ] &&
    JAVA_HOME="`(cd "$JAVA_HOME"; pwd -W | sed 's|/|\\\\|g')`"
  CLASSPATH_SUFFIX=";"
  PSEP=";"
fi

#/*--------------------------------------------------
# * The code below is for Dotty
# *-------------------------------------------------*/

find_lib () {
  local lib=$(find $PROG_HOME/lib/ -name "$1")
  if $has_cygpath; then
    cygpath -am $lib
  elif $mingw; then
    echo $lib | sed 's|/|\\\\|g'
  else
    echo $lib
  fi
}

fix for #11424

d166b56

bishabosha requested a review from anatoliykmetyuk February 15, 2021 22:19

bishabosha assigned anatoliykmetyuk Feb 15, 2021

liufengyun added the RC1:high-prio label Feb 16, 2021

anatoliykmetyuk approved these changes Feb 16, 2021

View reviewed changes

anatoliykmetyuk merged commit 1492745 into scala:master Feb 16, 2021

philwalk deleted the fix-11424 branch February 16, 2021 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix for #11424 #11425

fix for #11424 #11425

philwalk commented Feb 15, 2021

liufengyun commented Feb 16, 2021

michelou commented Feb 16, 2021

liufengyun commented Feb 16, 2021

philwalk commented Feb 16, 2021

michelou commented Feb 16, 2021

philwalk commented Feb 17, 2021

michelou commented Feb 17, 2021 •

edited

Loading

philwalk commented Feb 18, 2021 •

edited

Loading

michelou commented Feb 19, 2021 •

edited

Loading

philwalk commented Feb 19, 2021 •

edited

Loading

michelou commented Feb 19, 2021 •

edited

Loading

philwalk commented Feb 20, 2021

fix for #11424 #11425

fix for #11424 #11425

Conversation

philwalk commented Feb 15, 2021

liufengyun commented Feb 16, 2021

michelou commented Feb 16, 2021

liufengyun commented Feb 16, 2021

philwalk commented Feb 16, 2021

michelou commented Feb 16, 2021

philwalk commented Feb 17, 2021

michelou commented Feb 17, 2021 • edited Loading

philwalk commented Feb 18, 2021 • edited Loading

michelou commented Feb 19, 2021 • edited Loading

philwalk commented Feb 19, 2021 • edited Loading

michelou commented Feb 19, 2021 • edited Loading

philwalk commented Feb 20, 2021

michelou commented Feb 17, 2021 •

edited

Loading

philwalk commented Feb 18, 2021 •

edited

Loading

michelou commented Feb 19, 2021 •

edited

Loading

philwalk commented Feb 19, 2021 •

edited

Loading

michelou commented Feb 19, 2021 •

edited

Loading