support test scope by ignoring repeated pipelining flags

Author: bishabosha

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -441,7 +441,7 @@ private sealed trait YSettings:
   val YdebugMacros: Setting[Boolean] = BooleanSetting("-Ydebug-macros", "Show debug info when quote pattern match fails")
 
   // Pipeline compilation options
-  val YjavaTasty: Setting[Boolean] = BooleanSetting("-Yjava-tasty", "Pickler phase should compute pickles for .java defined symbols for use by build tools", aliases = List("-Ypickle-java"))
-  val YearlyTastyOutput: Setting[AbstractFile] = OutputSetting("-Yearly-tasty-output", "directory|jar", "Destination for generated .tasty files containing possibly outline type signatures.", NoAbstractFile, aliases = List("-Ypickle-write"))
+  val YjavaTasty: Setting[Boolean] = BooleanSetting("-Yjava-tasty", "Pickler phase should compute pickles for .java defined symbols for use by build tools", aliases = List("-Ypickle-java"), preferPrevious = true)
+  val YearlyTastyOutput: Setting[AbstractFile] = OutputSetting("-Yearly-tasty-output", "directory|jar", "Destination for generated .tasty files containing possibly outline type signatures.", NoAbstractFile, aliases = List("-Ypickle-write"), preferPrevious = true)
   val YallowOutlineFromTasty: Setting[Boolean] = BooleanSetting("-Yallow-outline-from-tasty", "Allow outline TASTy to be loaded with the -from-tasty option.")
 end YSettings

compiler/src/dotty/tools/dotc/config/Settings.scala

@@ -63,6 +63,7 @@ object Settings:
     aliases: List[String] = Nil,
     depends: List[(Setting[?], Any)] = Nil,
     ignoreInvalidArgs: Boolean = false,
+    preferPrevious: Boolean = false,
     propertyClass: Option[Class[?]] = None)(private[Settings] val idx: Int) {
 
     private var changed: Boolean = false
@@ -96,12 +97,17 @@ object Settings:
             value0.filter(current.contains).foreach(s => dangers :+= s"Setting $name set to $s redundantly")
             current ++ value0
           else
-            if changed then dangers :+= s"Flag $name set repeatedly"
+            if changed then
+              assert(!preferPrevious, "should have shortcutted with ignoreValue, side-effect may be present!")
+              dangers :+= s"Flag $name set repeatedly"
             value
         changed = true
         ArgsSummary(updateIn(sstate, value1), args, errors, dangers)
       end update
 
+      def ignoreValue(args: List[String]): ArgsSummary =
+        ArgsSummary(sstate, args, errors, warnings)
+
       def fail(msg: String, args: List[String]) =
         ArgsSummary(sstate, args, errors :+ msg, warnings)
 
@@ -143,7 +149,8 @@ object Settings:
 
       def doSet(argRest: String) = ((summon[ClassTag[T]], args): @unchecked) match {
         case (BooleanTag, _) =>
-          setBoolean(argRest, args)
+          if changed && preferPrevious then ignoreValue(args)
+          else setBoolean(argRest, args)
         case (OptionTag, _) =>
           update(Some(propertyClass.get.getConstructor().newInstance()), args)
         case (ListTag, _) =>
@@ -161,14 +168,16 @@ object Settings:
           if (arg2 startsWith "-") missingArg
           else setString(arg2, args2)
         case (OutputTag, arg :: args) =>
-          val path = Directory(arg)
-          val isJar = path.ext.isJar
-          if (!isJar && !path.isDirectory)
-            fail(s"'$arg' does not exist or is not a directory or .jar file", args)
-          else {
-            val output = if (isJar) JarArchive.create(path) else new PlainDirectory(path)
-            update(output, args)
-          }
+          if changed && preferPrevious then ignoreValue(args) // do not risk side effects such as overwriting a jar
+          else
+            val path = Directory(arg)
+            val isJar = path.ext.isJar
+            if (!isJar && !path.isDirectory)
+              fail(s"'$arg' does not exist or is not a directory or .jar file", args)
+            else {
+              val output = if (isJar) JarArchive.create(path) else new PlainDirectory(path)
+              update(output, args)
+            }
         case (IntTag, args) if argRest.nonEmpty =>
           setInt(argRest, args)
         case (IntTag, arg2 :: args2) =>
@@ -281,8 +290,8 @@ object Settings:
       setting
     }
 
-    def BooleanSetting(name: String, descr: String, initialValue: Boolean = false, aliases: List[String] = Nil): Setting[Boolean] =
-      publish(Setting(name, descr, initialValue, aliases = aliases))
+    def BooleanSetting(name: String, descr: String, initialValue: Boolean = false, aliases: List[String] = Nil, preferPrevious: Boolean = false): Setting[Boolean] =
+      publish(Setting(name, descr, initialValue, aliases = aliases, preferPrevious = preferPrevious))
 
     def StringSetting(name: String, helpArg: String, descr: String, default: String, aliases: List[String] = Nil): Setting[String] =
       publish(Setting(name, descr, default, helpArg, aliases = aliases))
@@ -308,8 +317,8 @@ object Settings:
     def MultiStringSetting(name: String, helpArg: String, descr: String, default: List[String] = Nil, aliases: List[String] = Nil): Setting[List[String]] =
       publish(Setting(name, descr, default, helpArg, aliases = aliases))
 
-    def OutputSetting(name: String, helpArg: String, descr: String, default: AbstractFile, aliases: List[String] = Nil): Setting[AbstractFile] =
-      publish(Setting(name, descr, default, helpArg, aliases = aliases))
+    def OutputSetting(name: String, helpArg: String, descr: String, default: AbstractFile, aliases: List[String] = Nil, preferPrevious: Boolean = false): Setting[AbstractFile] =
+      publish(Setting(name, descr, default, helpArg, aliases = aliases, preferPrevious = preferPrevious))
 
     def PathSetting(name: String, descr: String, default: String, aliases: List[String] = Nil): Setting[String] =
       publish(Setting(name, descr, default, aliases = aliases))

sbt-test/pipelining/pipelining-test/a/src/main/scala/a/A.scala

@@ -0,0 +1,5 @@
+package a
+
+object A {
+  val foo: (1,2,3) = (1,2,3)
+}

sbt-test/pipelining/pipelining-test/a/src/test/scala/a/Hello.scala

@@ -0,0 +1,12 @@
+package a
+
+import a.A
+
+import org.junit.Test
+
+class Hello {
+
+  @Test def test(): Unit = {
+    assert(A.foo == (1,2,3))
+  }
+}

sbt-test/pipelining/pipelining-test/build.sbt

@@ -0,0 +1,7 @@
+ThisBuild / usePipelining := true
+
+lazy val a = project.in(file("a"))
+  .settings(
+    scalacOptions += "-Ycheck:all",
+    libraryDependencies += "com.novocode" % "junit-interface" % "0.11" % "test",
+  )

sbt-test/pipelining/pipelining-test/project/DottyInjectedPlugin.scala

@@ -0,0 +1,12 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion"),
+    scalacOptions += "-source:3.0-migration"
+  )
+}

sbt-test/pipelining/pipelining-test/test

@@ -0,0 +1,12 @@
+# run the tests on a project with pipelining
+# exercises the fact that -Ypickle-java and -Ypickle-write
+# flags are set twice.
+# steps:
+# - Compile scope is compiled with flags `-Ypickle-java -Ypickle-write early/a-early-7423784.jar`
+# - sbt copies `early/a-early-7423784.jar` to `early/a-early.jar`
+# - Test scope is compiled with flags `-Ypickle-java -Ypickle-write early-test/a-early-963232.jar -Ypickle-java -Ypickle-write early/a-early.jar -classpath early/a-early.jar`
+# e.g. for some reason the classpath has the same `a-early.jar` that
+# is passed with `Ypickle-write`.
+# Therefore we MUST avoid even reading the second `-Ypickle-write` setting,
+# otherwise we will zero-out `a-early.jar`, causing type errors because its contents are blank.
+> a/test