fix 13618 - undo windows wildcard classpath globbing #13619
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philwalk
changed the title
|
I have an improved version of MainGenericRunner, with improved platform portability, better comments and simplified logic. I'll push a commit after running tests. |
|
Thank you, I will review the changes tomorrow. Will you be pushing some more improvements or it is the final state? |
That's all I have, it's now ready to merge. |
BarkingBad
reviewed
Sep 29, 2021
|
working on it ... |
|
Tests were successful, although this is what I get when I run coursier tests on my Windows workstation, FWIW: [info] Wrote C:\Users\philwalk\workspace\newPR-dotty\library\..\out\bootstrap\scala3-library-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\scala3-library_3-3.1.1-RC1-bin-SNAPSHOT.pom [info] Wrote C:\Users\philwalk\workspace\newPR-dotty\compiler\..\out\bootstrap\scala3-compiler-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\scala3-compiler_3-3.1.1-RC1-bin-SNAPSHOT.pom [info] Wrote C:\Users\philwalk\workspace\newPR-dotty\interfaces\target\scala3-interfaces-3.1.1-RC1-bin-SNAPSHOT.pom [info] Wrote C:\Users\philwalk\workspace\newPR-dotty\tasty\..\out\bootstrap\tasty-core-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\tasty-core_3-3.1.1-RC1-bin-SNAPSHOT.pom [info] :: delivering :: org.scala-lang#scala3-interfaces;3.1.1-RC1-bin-SNAPSHOT :: 3.1.1-RC1-bin-SNAPSHOT :: integration :: Wed Sep 29 16:15:13 MDT 2021 [info] delivering ivy file to C:\Users\philwalk\workspace\newPR-dotty\interfaces\target\ivy-3.1.1-RC1-bin-SNAPSHOT.xml [info] published scala3-interfaces to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-interfaces\3.1.1-RC1-bin-SNAPSHOT\poms\scala3-interfaces.pom [info] published scala3-interfaces to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-interfaces\3.1.1-RC1-bin-SNAPSHOT\jars\scala3-interfaces.jar [info] published scala3-interfaces to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-interfaces\3.1.1-RC1-bin-SNAPSHOT\srcs\scala3-interfaces-sources.jar [info] published scala3-interfaces to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-interfaces\3.1.1-RC1-bin-SNAPSHOT\docs\scala3-interfaces-javadoc.jar [info] published ivy to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-interfaces\3.1.1-RC1-bin-SNAPSHOT\ivys\ivy.xml [info] :: delivering :: org.scala-lang#tasty-core_3;3.1.1-RC1-bin-SNAPSHOT :: 3.1.1-RC1-bin-SNAPSHOT :: integration :: Wed Sep 29 16:15:15 MDT 2021 [info] delivering ivy file to C:\Users\philwalk\workspace\newPR-dotty\out\bootstrap\tasty-core-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\ivy-3.1.1-RC1-bin-SNAPSHOT.xml [info] Main Scala API documentation to C:\Users\philwalk\workspace\newPR-dotty\library\..\out\bootstrap\scala3-library-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\api... [info] published tasty-core_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\tasty-core_3\3.1.1-RC1-bin-SNAPSHOT\poms\tasty-core_3.pom [info] published tasty-core_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\tasty-core_3\3.1.1-RC1-bin-SNAPSHOT\jars\tasty-core_3.jar [info] published tasty-core_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\tasty-core_3\3.1.1-RC1-bin-SNAPSHOT\srcs\tasty-core_3-sources.jar [info] published tasty-core_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\tasty-core_3\3.1.1-RC1-bin-SNAPSHOT\docs\tasty-core_3-javadoc.jar [info] published ivy to C:\Users\philwalk\.ivy2\local\org.scala-lang\tasty-core_3\3.1.1-RC1-bin-SNAPSHOT\ivys\ivy.xml [info] Skipping unused scalacOptions: -Werror, -Yno-experimental, -deprecation, -feature, -language [warn] Setting -encoding is currently not supported. [warn] Setting -sourcepath is currently not supported. [warn] -- Warning: C:\Users\philwalk\workspace\newPR-dotty\library\src\scala\compiletime\package.scala:58:11 [warn] 58 |inline def error(inline msg: String): Nothing = ??? [warn] |^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [warn] |java.util.NoSuchElementException: None.get [warn] | at scala.None$.get(Option.scala:627) [warn] | at scala.None$.get(Option.scala:626) [warn] | at dotty.tools.dotc.typer.PrepareInlineable$.makeInlineable(PrepareInlineable.scala:35) [warn] | at dotty.tools.dotc.typer.PrepareInlineable$.registerInlineInfo$$anonfun$2$$anonfun$1(PrepareInlineable.scala:267) [error] -- [E069] Syntax Error: C:\Users\philwalk\workspace\newPR-dotty\library\src\scala\IArray.scala:13:7 [error] 13 |object IArray: [error] | ^ [error] |object IArray cannot have the same name as object IArray in package scala -- cannot define object member with the same name as a object member in self reference _. [error] |(Note: this can be resolved by using another name) [warn] three warnings found [error] one error found [info] :: delivering :: org.scala-lang#scala3-library_3;3.1.1-RC1-bin-SNAPSHOT :: 3.1.1-RC1-bin-SNAPSHOT :: integration :: Wed Sep 29 16:15:24 MDT 2021 [info] delivering ivy file to C:\Users\philwalk\workspace\newPR-dotty\out\bootstrap\scala3-library-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\ivy-3.1.1-RC1-bin-SNAPSHOT.xml [info] published scala3-library_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-library_3\3.1.1-RC1-bin-SNAPSHOT\poms\scala3-library_3.pom [info] published scala3-library_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-library_3\3.1.1-RC1-bin-SNAPSHOT\jars\scala3-library_3.jar [info] published scala3-library_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-library_3\3.1.1-RC1-bin-SNAPSHOT\srcs\scala3-library_3-sources.jar [info] published scala3-library_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-library_3\3.1.1-RC1-bin-SNAPSHOT\docs\scala3-library_3-javadoc.jar [info] published ivy to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-library_3\3.1.1-RC1-bin-SNAPSHOT\ivys\ivy.xml [info] :: delivering :: org.scala-lang#scala3-compiler_3;3.1.1-RC1-bin-SNAPSHOT :: 3.1.1-RC1-bin-SNAPSHOT :: integration :: Wed Sep 29 16:15:30 MDT 2021 [info] delivering ivy file to C:\Users\philwalk\workspace\newPR-dotty\out\bootstrap\scala3-compiler-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\ivy-3.1.1-RC1-bin-SNAPSHOT.xml [info] published scala3-compiler_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-compiler_3\3.1.1-RC1-bin-SNAPSHOT\poms\scala3-compiler_3.pom [info] published scala3-compiler_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-compiler_3\3.1.1-RC1-bin-SNAPSHOT\jars\scala3-compiler_3.jar [info] published scala3-compiler_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-compiler_3\3.1.1-RC1-bin-SNAPSHOT\srcs\scala3-compiler_3-sources.jar [info] published scala3-compiler_3 to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-compiler_3\3.1.1-RC1-bin-SNAPSHOT\docs\scala3-compiler_3-javadoc.jar [info] published ivy to C:\Users\philwalk\.ivy2\local\org.scala-lang\scala3-compiler_3\3.1.1-RC1-bin-SNAPSHOT\ivys\ivy.xml [info] compiling 2 Scala sources to C:\Users\philwalk\workspace\newPR-dotty\out\bootstrap\scala3-compiler-bootstrapped\scala-3.1.1-RC1-bin-SNAPSHOT-nonbootstrapped\test-classes ... [info] Test run started [info] Test dotty.tools.coursier.CoursierScalaTests.allTests started expected: arg 0:[a] actual : arg 0:[a]er-bootstrapped / Scala3CompilerCoursierTest / executeTests 3s expected: arg 1:[b] actual : arg 1:[b] expected: arg 2:[c] actual : arg 2:[c] expected: arg 3:[-repl] actual : arg 3:[-repl] expected: arg 4:[-run] actual : arg 4:[-run] expected: arg 5:[-script] actual : arg 5:[-script] expected: arg 6:[-debug] actual : arg 6:[-debug] actual : arg 6:[-debug][info] Test run finished: 0 failed, 0 ignored, 1 total, 22.628s [info] Passed: Total 1, Failed 0, Errors 0, Passed 1 [success] Total time: 43 s, completed Sep 29, 2021, 4:15:56 PM |
|
Do you mean that there are some errors etc? They all come from publishing artifacts - coursier tests depend on Couriser, which is built on to of some scala jars. I don't see anything particularly wrong with the MainGenericRunner itself, the warnings are not related. |
BarkingBad
reviewed
Sep 30, 2021
| val (tailargs, cpstr) = if globdir.nonEmpty && classpathSeparator != ";" || cp.contains(classpathSeparator) then | ||
| (tail, cp) | ||
| val cpEntries = cp.split(classpathSeparator).toList | ||
| val singleEntryClasspath: Boolean = cpEntries.nonEmpty && cpEntries.drop(1).isEmpty |
There was a problem hiding this comment.
Isn't just cpEntries.size == 1 enough?
There was a problem hiding this comment.
That would be okay, although a wildcard classpath directory could have thousands of jar files, so size could be expensive.
Should I change it?
There was a problem hiding this comment.
We can leave it done your way if you feel that it is better.
| @@ -241,4 +243,22 @@ object MainGenericRunner { | |||
| e.foreach(_.printStackTrace()) | |||
| !isFailure | |||
| } | |||
|
|
|||
| def display(settings: Settings)= Seq( | |||
There was a problem hiding this comment.
Isn't it dead code? I guess it's for debugging purposes, maybe we could add some flag for displaying this debug, or let's remove it
There was a problem hiding this comment.
How about an environment variable, maybe something like this:
lazy val debug = Option(System.getenv("RUNNER_DEBUG")) != None
[...]
if debug then printf("settings: %s\n", display(settings))I also don't mind removing it, I can always add it back temporarily, if needed.
There was a problem hiding this comment.
I think we can remove it.
BarkingBad
approved these changes
Sep 30, 2021
| @@ -106,7 +106,7 @@ object MainGenericRunner { | |||
| process(tail, settings.withExecuteMode(ExecuteMode.Run).withTargetToRun(fqName)) | |||
| case ("-cp" | "-classpath" | "--class-path") :: cp :: tail => | |||
| val cpEntries = cp.split(classpathSeparator).toList | |||
| val singleEntryClasspath: Boolean = cpEntries.nonEmpty && cpEntries.drop(1).isEmpty | |||
| val singleEntryClasspath: Boolean = cpEntries.take(2).size == 1 | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@BarkingBad
This is the fix referred to in #13618, and discussed at the tail end of #13562.
The error output generated by the compiler by this problem can be simulated before this fix by the following command line (notice that there are no quotes around the wildcard classpath entry):
To verify the fix, a new test is added to ClasspathTests.