- 
                Notifications
    
You must be signed in to change notification settings  - Fork 149
 
fix for 3307 (windows launcher rejects filenames with utf8 chars) #3923
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
base: main
Are you sure you want to change the base?
Conversation
| 
           The error messages for the failing tests are the result of having Windows code page 437 active. The failure is that a script named  object TestÅÄÖåäö {
  def main(args: Array[String]): Unit = {
    println("Hello from TestÅÄÖåäö")
  }
}failed with this error message: [950] Error: Could not find or load main class TestÅÄÖåäö
[950] Caused by: java.lang.ClassNotFoundException: TestÅÄÖåäö# ./mill integration.test.native 'scala.cli.integration.RunTestsDefault.UTF-8'[949] Produced artifacts:
[949]  C:\Users\philwalk\workspace\scala-cli-3307\out\cli\3.3.7\base-image\nativeImage.dest\scala-cli.build_artifacts.txt (txt)
[949]  C:\Users\philwalk\workspace\scala-cli-3307\out\cli\3.3.7\base-image\nativeImage.dest\scala-cli.exe (executable)
[949] ========================================================================================================================
[949] Finished generating 'scala-cli' in 3m 49s.
[950/950] integration.test.native
[950] >==== Running 'UTF-8' from RunTestDefinitions
[950] Running warmup testà
[950] Compiling project (Scala 3.7.3, JVM (17))
[950] Compiled project (Scala 3.7.3, JVM (17))
[950] Done running warmup test.
[950] Compiling project (Scala 3.7.3, JVM (17))
[950] Compiled project (Scala 3.7.3, JVM (17))
[950] Error: Could not find or load main class TestÅÄÖåäö
[950] Caused by: java.lang.ClassNotFoundException: TestÅÄÖåäö
[950] X==== Finishing 'UTF-8' from RunTestDefinitions
[950] scala.cli.integration.RunTestsDefault:
[950] ==> X scala.cli.integration.RunTestsDefault.UTF-8  2.017s os.SubprocessException: Result of C:\Users\philwalk\workspace\scala-cli-3307\out\cli\3.3.7\base-image\nativeImage.dest\scala-cli.exeà: 1
[950]
[950]     at os.proc.call(ProcessOps.scala:232)
[950]     at scala.cli.integration.RunTestDefinitions.$anonfun$new$117(RunTestDefinitions.scala:1080)
[950]     at scala.cli.integration.RunTestDefinitions.$anonfun$new$117$adapted(RunTestDefinitions.scala:1072)
[950]     at scala.cli.integration.TestInputs.$anonfun$fromRoot$1(TestInputs.scala:35)
[950]     at scala.cli.integration.TestInputs$.scala$cli$integration$TestInputs$$withTmpDir(TestInputs.scala:95)
[950]     at scala.cli.integration.TestInputs.fromRoot(TestInputs.scala:33)
[950]     at scala.cli.integration.RunTestDefinitions.$anonfun$new$116(RunTestDefinitions.scala:1072)
[950]     at scala.cli.integration.WithWarmUpScalaCliSuite.$anonfun$test$1(WithWarmUpScalaCliSuite.scala:34)
[950]     at scala.cli.integration.WithWarmUpScalaCliSuite.$anonfun$test$2(WithWarmUpScalaCliSuite.scala:37)
[950/950, 1 failed] ============================== integration.test.native scala.cli.integration.RunTestsDefault.UTF-8 ============================== 265s
1 tasks failed
integration.test.native 1 tests failed:
  scala.cli.integration.RunTestsDefault.UTF-8 scala.cli.integration.RunTestsDefault.UTF-8
philwalk@d5 MSYS ~/workspace/scala-cli-3307 | 
    
          
 Do you maybe know how to do it? Can't say I'm familiar with code pages in Windows, myself.  | 
    
      
        
              This comment was marked as outdated.
        
        
      
    
  This comment was marked as outdated.
          
 Depends what "running with JDK18" means here. scala-cli/modules/integration/src/test/scala/scala/cli/integration/RunJdkTestDefinitions.scala Lines 37 to 59 in 863bec3 
 
 scala-cli -Dfile.encoding=UTF-8 compile (...)If, however, this would mean the   | 
    
      
        
              This comment was marked as outdated.
        
        
      
    
  This comment was marked as outdated.
| 
           It turns out that when an  
 If I duplicate the failing test in a  
 The same is true of the launcher  # time ./mill integration.test.jvm 'scala.cli.integration.RunTestsDefault.UTF-8' 
# time ./mill integration.test.native 'scala.cli.integration.RunTestsDefault.UTF-8' Here's the demo shell script. #!/bin/bash
cat > "/tmp/testÅÄÖåäö.sc" <<'EOF'
#!/usr/bin/env -S scala-cli shebang
//> using dep com.lihaoyi::os-lib:0.8.1
object TestÅÄÖåäö {
  import java.nio.charset.Charset
  var launcherName = "scala-cli.exe"
  val filename = "testÅÄÖåäö.sc"
  def showEncoding(): Unit = {
    printf("======= jvm encoding configuration:\n")
    import scala.jdk.CollectionConverters.*
    import scala.sys.process.*
    System.err.printf("code-page: [%s]\n", ("chcp.com".!!).trim)
    System.err.printf("JAVA_TOOL_OPTIONS[%s]\n", System.getenv("JAVA_TOOL_OPTIONS"))
    System.err.printf("native.encoding = %s\n",  System.getProperty("native.encoding"))
    System.err.printf("sun.jnu.encoding = %s\n", System.getProperty("sun.jnu.encoding"))
    System.err.printf("file.encoding = %s\n",    System.getProperty("file.encoding"))
    System.err.printf("Charset.defaultCharset = %s\n", java.nio.charset.Charset.defaultCharset())
    System.err.printf("Class name = %s\n", this.getClass.getName)
    printf("fileName[%s]\n", sys.props("scala.source.names"))
    import java.lang.ProcessHandle
    def displayAncestors(ph: ProcessHandle): Unit = {
      import java.util.Optional
      val parentHandle: Optional[ProcessHandle] = ph.parent()
      if (parentHandle.isPresent) {
        val parent = parentHandle.get()
        val command = parent.info().command().orElse("N/A")
        println(s"PID: ${parent.pid()}, Command: $command")
        displayAncestors(parent)
      } else {
        println("--- Reached top of process tree ---")
      }
    }
    println("--- Process Ancestry ---")
    // Display the current process first
    val currentHandle = ProcessHandle.current()
    val currentCommand = currentHandle.info().command().orElse("N/A")
    println(s"PID: ${currentHandle.pid()} (Current Process), Command: $currentCommand")
    displayAncestors(ProcessHandle.current())
  }
  def main(args: Array[String]): Unit = {
    val asciiOnly: Boolean = args.contains("-ascii")
    //val p = if scriptPath.fwd.startsWith("/") then os.Path(scriptPath) else os.Path(scriptPath, pwd)
    showEncoding()
    println(s"Hello from $filename")
    println(s"Hello from $scriptPath")
    printf("launcher: %s\n", launcher)
    val subProcessScriptpath = if asciiOnly then "/tmp/"+filename.replaceAll("[^\\x00-\\x7F]", "") else filename
    printf("subProcessScriptpath [%s]\n", subProcessScriptpath)
  
    val endOfCopy = "=== script cloning ends here ==="
    val p = java.nio.file.Paths.get(scriptPath)
    val osp = osPath(scriptPath)
    val content = os.read(osp, scala.io.Codec.UTF8)
    val topPart = content.split("[\r\n]+").takeWhile(!_.contains(endOfCopy)).filter(!_.contains("launcher")).mkString("\n")+"\n  }\n}"
    os.write.over(osPath(subProcessScriptpath), topPart)
    val extraArgs = Seq("--bloop-startup-timeout", "2min", "--bloop-bsp-timeout", "1min")
    printf("\nos.proc.call cloned copy: subProcessScriptpath [%s]\n", subProcessScriptpath)
    val res = os.proc(
      launcher,
      subProcessScriptpath
    )
      .call()
    val outstr = res.out.text(scala.io.Codec.UTF8).trim
    printf("outstr[%s]\n", outstr)
  }
  import scala.sys.process.*
  lazy val launcher = Seq("where.exe", launcherName).!!.trim.replace('\\', '/').split("[\r\n]+").toSeq.head
  def osPath(path: String): os.Path = {
    import java.nio.file.{Paths, Files}
    val p = Paths.get(path).toAbsolutePath
    os.Path(p.toString)
  }
}
EOF
FNAME=/tmp/testÅÄÖåäö.sc
scala-cli.exe run $FNAME # okay
echo "######################################" 1>&2
java -Xmx512m -Xms128m -jar 'out/cli/3.3.7/standaloneLauncher.dest/launcher.bat' $FNAMEThe script in the HERE-doc is a self-contained   | 
    
| 
           This most recent commit adds extensive logging to the failing integration test for diagnostic purposes. @Gedochao - are there any restrictions to using code created in collaboration with Copilot?  | 
    
          
 There are none, go ahead. We use a standard Apache 2.0 License and there's no specific policy against code created with LLM-reliant tooling.  | 
    
| 
           @philwalk I converted the PR to draft format. I will keep track of what's happening here, feel free to mark it back as ready to review whenever. thanks for working on this.  | 
    
          
 Thanks. I'm trying to determine whether the two failing Windows tests for the initial PR commit are actually valid in Windows: I'm trying to evolve the code into a valid UTF-8 test in the  ✅ Encodings with Valid Byte Representations for 
 | 
    
| Encoding | Byte Representation (Hex) for ÅÄÖåäö | Notes | 
|---|---|---|
| UTF-8 | C3 85 C3 84 C3 96 C3 A5 C3 A4 C3 B6 | Fully supported; portable across platforms | 
| Windows-1252 | C5 C4 D6 E5 E4 F6 | Common on Windows; compatible with Western European locales | 
| ISO-8859-1 | C5 C4 D6 E5 E4 F6 | Similar to Windows-1252 but lacks some extra characters | 
| ISO-8859-15 | C5 C4 D6 E5 E4 F6 | Adds euro symbol and other tweaks to ISO-8859-1 | 
| MacRoman | 8A 8E 8F 86 84 94 | Legacy Mac encoding; not recommended for new systems | 
| Latin-9 | C5 C4 D6 E5 E4 F6 | Alias for ISO-8859-15 | 
| CP437 | 8F 8E 99 86 84 94 | Original IBM PC encoding; limited compatibility | 
This PR modifies
generate-native-image.shto generatescala-cli.exewith code page set to 65001 if running in Windows:Using a test script named
/opt/ue/jsrc/løp.sc:and with the PATH set to use the generated
scala-cli.exe:PATH="out/cli/3.3.7/base-image/nativeImage.dest:$PATH"Manual verification that the bug and the fix depend on the code page set during GraalVM compile.
verify the bug
create a buggy
scala-cli.exeby runninggenerated-native-image.shtemporarily modified to set code page 437Verify that the bug manifests itself when attempting to execute the script:
Verify the fix
create fixed
scala-cli.exeby runninggenerated-native-image.shas proposed in this PR (set code page 65001)Verify that the bug is fixed:
# /opt/ue/jsrc/løp.sc Hello /opt/ue/jsrc/løp.sc Hello C:/opt/ue/jsrc/løp.sc Hello løp.sc