Skip to content

Fixes for various problems related to scripting. #11489

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

Merged
merged 4 commits into from
Feb 22, 2021
Merged

Fixes for various problems related to scripting. #11489

merged 4 commits into from
Feb 22, 2021

Conversation

philwalk
Copy link
Contributor

@philwalk philwalk commented Feb 21, 2021

  1. set $scala_exit_status & trap onExit; script exit code correct whether called via scalac or java -jar
  2. added missing 'scala -version' option via call to 'scalac -version'
  3. extended and verified REPL and scripting for Cygwin, MinGW, Msys2, and other Windows shells with a cygpath tool
  4. MinGW based Windows shell environments require TERM=dumb to support REPL
  5. simpify and extend jar manifest classpath to support for changing default drive
  6. . added tests

Manual REPL and script -save tested for the following Windows and Linux environments:
CYGWIN_NT-10.0 host 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin
MINGW64_NT-10.0-19042 host 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
MSYS_NT-10.0-19042 host 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys
Linux host 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
Linux host 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Manual test results for executing a script via compile mode and then java -jar mode:

CYGWIN_NT-10.0 d5 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin
1st run: real 0m6.183s user 0m0.407s sys 0m1.314s
2nd run: real 0m2.431s user 0m0.225s sys 0m0.652s

Linux quadd 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
1st run: real 0m3.612s user 0m10.368s sys 0m0.362s
2nd run: real 0m0.979s user 0m2.106s sys 0m0.082s

Linux d5 5.4.72-microsoft-standard-WSL2 1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
1st run: real 0m4.344s user 0m9.725s sys 0m0.553s
2nd run: real 0m1.100s user 0m0.981s sys 0m0.067s

MINGW64_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
1st run: real 0m6.838s user 0m0.420s sys 0m1.543s
2nd run: real 0m3.129s user 0m0.272s sys 0m0.799s

MSYS_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys
1st run: real 0m6.527s user 0m0.455s sys 0m1.338s
2nd run: real 0m3.012s user 0m0.151s sys 0m0.679s

CYGWIN_NT-10.0 d5 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin
1st run: real 0m6.183s user 0m0.407s sys 0m1.314s
2nd run: real 0m2.431s user 0m0.225s sys 0m0.652s

1. set $scala_exit_status & trap onExit; script exit code correct whether called via scalac or java -jar
2. added missing 'scala -version' option to via call to 'scalac -version'
3. extended and verified REPL and scripting for Cygwin, MinGW, Msys2, and Windows shells with a cygpath tool
4. MinGW based Windows shell environments require TERM=dumb to support REPL
5. simpify and extend jar manifest classpath to support for changing default drive
6. tested for the following Windows and Linux environments:
7. added tests

  CYGWIN_NT-10.0 host 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin
  MINGW64_NT-10.0-19042 host 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
  MSYS_NT-10.0-19042 host 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys
  Linux host 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
  Linux host 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Manual test results for executing a script via compile mode and then java -jar mode:

uname -a :: CYGWIN_NT-10.0 d5 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin

uname -a :: Linux quadd 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

uname -a :: Linux d5 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

uname -a :: MINGW64_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys

uname -a :: MSYS_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys

uname -a :: CYGWIN_NT-10.0 d5 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin
@michelou
Copy link
Contributor

@philwalk Here are a few comments :

  1. File dist/bin/common, lines 105-107 :

    # cygpath is used by various windows shells: cygwin, git-sdk, gitbash, msys, etc.
    has_cygpath=false
    if `which cygpath >/dev/null 2>&1`; then has_cygpath=true; fi

    The above is called unconditionally, even for "non Windows-hosted" Unix environments.
    I suggest to move it to the if-statement on line 57 :

    if [[ ($cygwin||$mingw||$msys) ]]; then
       ...
    fi
  2. File dist/bin/common, lines 105-107 :

    Instead of having both $has_cygpath and then PROG_HOME='cygpath -am "$PROG_HOME"' I would introduce a variable CYGPATHCMD (similar to JAVACMD) and use it instead when defined, e.g. PROG_HOME='$CYGPATHCMD -am "$PROG_HOME"'.

  3. File dist/bin/scalac :

    Why are you so defensive on line 63 ff ?

  4. See also @smarter comment about execution time of shell scripts in PR improved readability of help messages #10304.

@philwalk
Copy link
Contributor Author

philwalk commented Feb 21, 2021

@michelou I will incorporate your suggestions, and hopefully can push the update today sometime.

File dist/bin/scalac :
Why are you so defensive on line 63 ff ?

That was added during development so I could put this at the top of dist/bin/common:

set -o nounset ; set -o errexit

It reduces the noise when debugging, if any of the library jars aren't found, but I had intended to revert it before checkin.
I will do so in the next push.

@philwalk
Copy link
Contributor Author

philwalk commented Feb 21, 2021

For Msys and Mingw environments, this PR provides:

  1. full scripting capability
  2. a limited REPL by setting TERM=dumb
  3. REPL arrow keys don't work

If I'm not mistaken, the REPL is similarly limited in scala2 as well, in Msys/Mingw. I'll continue to investigate.

For xterm environments, I'm not aware of any limitations. I hope to be able to verify a OSX/Darwin in a few weeks.

@michelou
Copy link
Contributor

@philwalk
File dist/bin/common : comment on line 108 is outdated.

@philwalk
Copy link
Contributor Author

I just pushed a correction for the outdated comment.

File dist/bin/common : comment on line 108 is outdated.
In addition, I added back the change suggested by @michelou to dist/bin/common:

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

It improves the REPL experience for some versions of MinGW and Msys. It turns out there are lots of different versions out there, both 32-bit and 64-bit.

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @philwalk !

For later: I'm wondering can we test the script on Windows as part of CI? We can install the dependencies on the Windows CI machine if needed.

@liufengyun liufengyun merged commit 59eaeaf into scala:master Feb 22, 2021
@philwalk
Copy link
Contributor Author

Here are the startup timing numbers for 6 different tested environments. The test was:

touch testscript.sc
time ./testscript.sc
time ./testscript.sc

The 1st run is executed by dotty.tools.scripting.Main
The 2nd run is a call to java -jar ./testscript.sc

Example script:

#!/opt/scala3/bin/scala

import better.files._
def main(args: Array[String]): Unit =
  val version = File("/opt/scala3/VERSION").contentAsString.trim
  printf("%s\n",version)
  import scala.sys.process._
  printf("%s\n",Seq("uname","-a").lazyLines_!.toList.mkString(""))

example SCALA_OPTS for running example script:

echo '-classpath lib/better-files_2.13-3.9.1.jar' > bfclasspath.txt
SCALA_OPTS="-save @bfclasspath.txt"
./testscript.sc

Relative performance numbers for each environment:

==> test-linux-wsl.out <==
# Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-e391d23 -- Copyright 2002-2021, LAMP/EPFL
# Linux d5 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
1st run:  3.72 seconds
2nd run:  0.71 seconds
speedup:  3.01 seconds (427 % of min. runtime)
==> test-linux.out <==
# Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-e391d23 -- Copyright 2002-2021, LAMP/EPFL
# Linux quadd 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
1st run:  2.71 seconds
2nd run:  0.47 seconds
speedup:  2.24 seconds (481 % of min. runtime)
==> test-cygwin.out <==
# Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-e391d23 -- Copyright 2002-2021, LAMP/EPFL
# CYGWIN_NT-10.0 d5 3.1.7(0.340/5/3) 2020-08-22 17:48 x86_64 Cygwin
1st run:  4.94 seconds
2nd run:  1.46 seconds
speedup:  3.48 seconds (239 % of min. runtime)
==> test-mingw32.out <==
# Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-e391d23 -- Copyright 2002-2021, LAMP/EPFL
# MINGW32_NT-6.2 D5 1.0.19(0.48/3/2) 2016-07-13 17:45 i686 Msys
1st run:  5.51 seconds
2nd run:  1.76 seconds
speedup:  3.75 seconds (212 % of min. runtime)
==> test-mingw64.out <==
# Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-e391d23 -- Copyright 2002-2021, LAMP/EPFL
# MINGW64_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
1st run:  5.00 seconds
2nd run:  1.45 seconds
speedup:  3.55 seconds (245 % of min. runtime)
==> test-msys.out <==
# Scala compiler version 3.0.0-RC2-bin-SNAPSHOT-git-e391d23 -- Copyright 2002-2021, LAMP/EPFL
# MSYS_NT-10.0-19042 d5 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys
1st run:  4.75 seconds
2nd run:  1.34 seconds
speedup:  3.42 seconds (256 % of min. runtime)

@philwalk
Copy link
Contributor Author

@liufengyun

For later: I'm wondering can we test the script on Windows as part of CI? We can install the dependencies on the Windows CI machine if needed.

Running scripts in each environment would be relatively easy, although the REPL test would require some automation (e.g., LDTP or similar).

@liufengyun
Copy link
Contributor

Thank you @philwalk . Having only command-line tests already helps a lot to avoid regressions that happened in the past. We have some basic tests in project/scripts/bootstrapCmdTests, but only for Unix-like systems.

@philwalk philwalk deleted the mingw-msys-repl-scripting branch February 23, 2021 17:47
@philwalk
Copy link
Contributor Author

I might be able to work on adding tests in a few weeks ...

@philwalk
Copy link
Contributor Author

philwalk commented Feb 23, 2021

FYI, All msys and mingwXX testing occurred in ConEmu sessions.

To use the scala REPL in some mis-configurations of Windows Terminal or possibly in mintty, you may need to do this:

$ TERM=dumb scala

However, the cursor keys may not work in these setups.

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.

4 participants