support test scope by ignoring repeated pipelining flags

Author: bishabosha

Diff for: compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

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

Diff for: compiler/src/dotty/tools/dotc/config/Settings.scala

@@ -79,6 +79,7 @@ object Settings:
     aliases: List[String] = Nil,
     depends: List[(Setting[?], Any)] = Nil,
     ignoreInvalidArgs: Boolean = false,
+    preferPrevious: Boolean = false,
     propertyClass: Option[Class[?]] = None,
     deprecationMsg: Option[String] = None,
     // kept only for -Ykind-projector option compatibility
@@ -125,11 +126,16 @@ object Settings:
             valueList.filter(current.contains).foreach(s => dangers :+= s"Setting $name set to $s redundantly")
             current ++ valueList
           else
-            if sstate.wasChanged(idx) then dangers :+= s"Flag $name set repeatedly"
+            if sstate.wasChanged(idx) then
+              assert(!preferPrevious, "should have shortcutted with ignoreValue, side-effect may be present!")
+              dangers :+= s"Flag $name set repeatedly"
             value
         ArgsSummary(updateIn(sstate, valueNew), 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)
 
@@ -196,7 +202,8 @@ object Settings:
       def doSet(argRest: String) =
         ((summon[ClassTag[T]], args): @unchecked) match {
           case (BooleanTag, _) =>
-            setBoolean(argRest, args)
+            if sstate.wasChanged(idx) && preferPrevious then ignoreValue(args)
+            else setBoolean(argRest, args)
           case (OptionTag, _) =>
             update(Some(propertyClass.get.getConstructor().newInstance()), args)
           case (ct, args) =>
@@ -216,7 +223,10 @@ object Settings:
           case StringTag =>
             setString(arg, argsLeft)
           case OutputTag =>
-            setOutput(arg, argsLeft)
+            if sstate.wasChanged(idx) && preferPrevious then
+              ignoreValue(argsLeft) // do not risk side effects e.g. overwriting a jar
+            else
+              setOutput(arg, argsLeft)
           case IntTag =>
             setInt(arg, argsLeft)
           case VersionTag =>
@@ -333,8 +343,8 @@ object Settings:
       assert(!name.startsWith("-"), s"Setting $name cannot start with -")
       "-" + name
 
-    def BooleanSetting(category: SettingCategory, name: String, descr: String, initialValue: Boolean = false, aliases: List[String] = Nil): Setting[Boolean] =
-      publish(Setting(category, prependName(name), descr, initialValue, aliases = aliases))
+    def BooleanSetting(category: SettingCategory, name: String, descr: String, initialValue: Boolean = false, aliases: List[String] = Nil, preferPrevious: Boolean = false): Setting[Boolean] =
+      publish(Setting(category, prependName(name), descr, initialValue, aliases = aliases, preferPrevious = preferPrevious))
 
     def StringSetting(category: SettingCategory, name: String, helpArg: String, descr: String, default: String, aliases: List[String] = Nil): Setting[String] =
       publish(Setting(category, prependName(name), descr, default, helpArg, aliases = aliases))
@@ -357,8 +367,8 @@ object Settings:
     def MultiStringSetting(category: SettingCategory, name: String, helpArg: String, descr: String, default: List[String] = Nil, aliases: List[String] = Nil): Setting[List[String]] =
       publish(Setting(category, prependName(name), descr, default, helpArg, aliases = aliases))
 
-    def OutputSetting(category: SettingCategory, name: String, helpArg: String, descr: String, default: AbstractFile, aliases: List[String] = Nil): Setting[AbstractFile] =
-      publish(Setting(category, prependName(name), descr, default, helpArg, aliases = aliases))
+    def OutputSetting(category: SettingCategory, name: String, helpArg: String, descr: String, default: AbstractFile, aliases: List[String] = Nil, preferPrevious: Boolean = false): Setting[AbstractFile] =
+      publish(Setting(category, prependName(name), descr, default, helpArg, aliases = aliases, preferPrevious = preferPrevious))
 
     def PathSetting(category: SettingCategory, name: String, descr: String, default: String, aliases: List[String] = Nil): Setting[String] =
       publish(Setting(category, prependName(name), descr, default, aliases = aliases))

Diff for: 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)
+}

Diff for: 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))
+  }
+}

Diff for: 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",
+  )

Diff for: 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"
+  )
+}

Diff for: 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