Two additions to batch files

[michelou]

These additions are follow-ups of the two merged pull requests :

• dist/bin/scalac.bat, see PR#13759.
• dist/bin/scaladoc.bat, see PR#13577.

The changes are Windows-specific and have no impact on the CI build.

[BarkingBad]

LGTM

[BarkingBad]

@philwalk would you also take a look? I know you test a lot on Windows

[philwalk]

@philwalk would you also take a look? I know you test a lot on Windows

FWIW, I don't see any problems, although I rarely use these batch files, so I don't fully understand the context.
I'll see if I can spend some time with it.

Here's my first naive attempt to evaluate it, from a cygwin bash session:

$ git clone [email protected]:michelou/dotty.git win-dotty
$ cd win-dotty
$ git checkout dotty-test-mrJar
$ sbt dist/pack
$ PATH="`pwd -P`/dist/target/pack/bin:$PATH"
$ which scala.bat
/home/philwalk/workspace/win-dotty/dist/target/pack/bin/scala.bat
$ scala.bat `cygpath -w /opt/ue/jsrc/scalacVersion.sc`
Error: Could not find or load main class C:\opt\ue\jsrc\scalacVersion.sc
Caused by: java.lang.ClassNotFoundException: C:\opt\ue\jsrc\scalacVersion.sc

This tells me that scala.bat isn't (yet?) intended to support the same usage model as the scala bash script for launching scala scripts (however, see my successful attempt below)

The same test with the bash script went like this:

$ bin/scala /opt/ue/jsrc/scalacVersion.sc
scalaHome: C:/Users/philwalk/workspace/win-dotty/dist/target/pack

Here's my REPL test from a Windows CMD session:

Microsoft Windows [Version 10.0.19042.1288]
(c) Microsoft Corporation. All rights reserved.

C:\Windows\system32>cd \Users\philwalk\workspace\win-dotty

C:\Users\philwalk\workspace\win-dotty>set PATH=c:\Users\philwalk\workspace\win-dotty\dist\target\pack\bin;%PATH%

C:\Users\philwalk\workspace\win-dotty>
C:\Users\philwalk\workspace\win-dotty>scala.bat
Welcome to Scala 3.1.2-RC1-bin-SNAPSHOT-git-387b0a7 (11.0.12, Java Java HotSpot(TM) 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> sys.props("scala.home")
val res0: String = c:\Users\philwalk\workspace\win-dotty\dist\target\pack

scala>

That looks correct, and I suspect the PR is good, because the changes are quite minimal, but will continue to experiment.

Aha, I discovered the problem with my first experiment, the script name is required to have extension ".scala" in order to be recognized as a script. Here's my successful experiment:

C:\Users\philwalk\workspace\win-dotty>scala.bat scalacVersion.scala
scalaHome: c:\Users\philwalk\workspace\win-dotty\dist\target\pack

Here's my original cygwin experiment, with the renamed script:

$ dist/target/pack/bin/scala.bat scalacVersion.scala
scalaHome: C:\Users\philwalk\workspace\win-dotty\dist\target\pack

So everything looks good.

[michelou]

@smarter Backport candidate to 3.1.1-RC2.

[smarter]

Does this fix a regression from a previous scala 3 release? Otherwise it's unlikely to be backported.

[michelou]

@smarter My reasoning is as follows :

• PR #13577 is part of 3.1.0 final (and 3.1.1-RC1 too).
• PR #13790 and #13806 include small fixes to PR #13577.

In other words the feature "batch files support installation path containing spaces"
(Reminder: Windows specific and no impact on the CI build)

• was introduced in 3.1.0 (see "Other bug fixes" in announcement)
• would work as expected in upcoming 3.1.1.

The final decision is yours.

[smarter]

I'm not familiar with this code so I shouldn't decide really :). You can submit a backport and have @anatoliykmetyuk decide since he reviewed this PR already.