Skip to content
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

fix arg processing in argument files in Scala 2.13.10 #10316

Closed
wants to merge 1 commit into from

Conversation

michelou
Copy link

@michelou michelou commented Feb 18, 2023

Scala 2.13.10 does not handle argument file contents correctly.
NB. Scala 2.13.8, Scala 2.13.9 and Scala 3.x.x work fine.

This PR solves two issues with argument files (tested on MS Windows Pro) :

  1. Several arguments specified on the same line are (wrongly) recognized as one single argument.
  2. Arguments with quotes (single/double) are kept unchanged.

Change locality of this PR is very high.

Workaround (temporary, 2.13.10 only)

We specify command line options (eg. "-deprecation -d <target_dir> ...") directly on the command prompt (and not in an argument file) when working with 2.13.10.

Session examples

Here are some results for the tiny mixed Java/Scala project hello-scala :
NB. Project hello-scala is available from Github repository michelou/dotty-examples.

  1. Scala 2.13.8, Scala 2.13.9 and Scala 3.x.x work fine.
    NB. They behave the same as the Java SDK (i.e. javac, java, kotlinc, etc.).

    $ echo %JAVA_HOME%
    C:\opt\jdk-temurin-11.0.18_10
    
    $ set SCALA_HOME=c:\opt\scala-2.13.8
    $ build -verbose -scala2 clean run
    Delete directory "target"
    Compile 1 Java source file to directory "target\classes"
    Compile 1 Scala source file to directory "target\classes"
    Execute Scala main class "hello"
    Hello!
    Java Hello!
    
    $ set SCALA_HOME=c:\opt\scala-2.13.9
    $ build -verbose -scala2 clean run
    Delete directory "target"
    Compile 1 Java source file to directory "target\classes"
    Compile 1 Scala source file to directory "target\classes"
    Execute Scala main class "hello"
    Hello!
    Java Hello!
    
    $ set SCALA3_HOME=C:\opt\scala3-3.3.0-RC3
    $ build -verbose -scala3 clean run
    Delete directory "target"
    Compile 1 Java source file to directory "target\classes"
    Compile 1 Scala source file to directory "target\classes"
    Execute Scala main class "hello"
    Hello!
    Java Hello!
    
  2. Scala 2.13.10 fails with the following error message :

    $ set SCALA_HOME=c:\opt\scala-2.13.10
    $ build -verbose -scala2 clean run
    Delete directory "target"
    Compile 1 Java source file to directory "target\classes"
    Compile 1 Scala source file to directory "target\classes"
    scalac error: bad option: '-deprecation -feature -classpath 'Y:\\examples\\hello-scala\\target\\classes' -d "Y:\\examples\\hello-scala\\target\\classes"'
      scalac -help  gives more information
    Error: Compilation of 1 Scala source file failed
    

    NB. When working with a local build based on this PR we get the expected output :

    $ set SCALA_HOME=%USERPROFILE%\workspace-perso\scala\build\pack
    $ build -verbose -scala2 clean run
    Delete directory "target"
    Compile 1 Java source file to directory "target\classes"
    Compile 1 Scala source file to directory "target\classes"
    Execute Scala main class "hello"
    Hello!
    Java Hello!
    
    $ %SCALA_HOME%\bin\scalac -version
    Scala compiler version 2.13.11-20230217-152005-3d41637 -- Copyright 2002-2023, LAMP/EPFL and Lightbend, Inc.
    
  3. In the above use cases we actually specify 2 argument files, *_opts.txt files contain command line options (on the same line) and *_sources.txt contain Scala source files (one file path on each line) :

    [...]
    [build] "C:\opt\jdk-temurin-11.0.18_10\bin\javac.exe" "@Y:\examples\hello-scala\target\javac_opts.txt" "@Y:\examples\hello-scala\target\javac_sources.txt"
    [...]
    [build] "%USERPROFILE%\workspace-perso\scala\build\pack\bin\scalac.bat" "@Y:\examples\hello-scala\target\scalac_opts.txt" "@Y:\examples\hello-scala\target\scalac_sources.txt"
    [...]
    

    For completeness we show here the contents of the 4 argument files :
    NB. The careful reader will notice that we deliberately use both single quotes (-classpath) and double quotes ( -d).

    $ type target\javac_*.txt
    
    target\javac_opts.txt
    
    -deprecation -classpath '%USERPROFILE%\\.m2\\repository\\org\\portable-scala\\portable-scala-reflect_2.13\\1.1.2\\portable-scala-reflect_2.13-1.1.2.jar;%USERPROFILE%\\.m2\\repository\\org\\scala-lang\\modules\\scala-xml_3\\2.1.0\\scala-xml_3-2.1.0.jar;%USERPROFILE%\\.m2\\repository\\org\\scala-lang\\modules\\scala-parser-combinators_3\\2.1.1\\scala-parser-combinators_3-2.1.1.jar;[...];Y:\\examples\\hello-scala\\target\\classes' -d "Y:\\examples\\hello-scala\\target\\classes"
    
    target\javac_sources.txt
    
    Y:\examples\hello-scala\src\main\java\Main.java
    
    $ type target\scalac_*.txt
    
    target\scalac_opts.txt
    
    -deprecation -feature -classpath 'Y:\\examples\\hello-scala\\target\\classes' -d "Y:\\examples\\hello-scala\\target\\classes"
    
    target\scalac_sources.txt
    
    Y:\examples\hello-scala\src\main\scala\hello.scala
    

@scala-jenkins scala-jenkins added this to the 2.13.11 milestone Feb 18, 2023
@som-snytt
Copy link
Contributor

som-snytt commented Feb 18, 2023

I'll try to find the commit, but IIRC this was intentional. It used to concatenate the lines and reparse, which is wrong. (I'm just asserting and not legislating.) You want to be able to specify file path with spaces on a line, for example.

The CommandLineParser has the code for doing bash-like quoting.

Edit: forgot to say my point of reference was the args file for Eclipse IDE.

This is consonant with -Vprint-args.

The original PR was #10123 (with a tango to wait until after 2.13.9 was released). The explanation there is that if you try to tokenize the line of an argfile, you must cope with quoting and escaping. But the argfile should just present an arg per line, which is the moral equivalent of handing a Seq of args to sys.process instead of a string to parse.

The previous code used settings.splitParams to use CommandLineParser.tokenize; the original tokenize was more primitive.

@som-snytt
Copy link
Contributor

som-snytt commented Feb 18, 2023

(Re-reading the comment) I didn't compare javac behavior, so maybe that should set the expectation. Indeed,

An argument file can include command-line options and source file names in any combination. The arguments within a file can be separated by spaces or new line characters. If a file name contains embedded spaces, then put the whole file name in double quotation marks.

@som-snytt
Copy link
Contributor

I created a ticket to track the issue, and I'll try to follow up with a correction, if it improves upon this solution.

@michelou
Copy link
Author

@som-snytt That's fine. Thanks.

@michelou
Copy link
Author

@som-snytt Superseded by PR#10319.

@michelou michelou closed this Feb 19, 2023
@michelou
Copy link
Author

@som-snytt Superseded by #10319.

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