Merge pull request #1036 from dansanduleac/derivedSettings

Allow derived settings to replace previously-defined but non-default settings

util/collection/src/main/scala/sbt/Settings.scala

@@ -3,7 +3,7 @@
  */
 package sbt
 
-	import Types._
+import Types._
 
 sealed trait Settings[Scope]
 {
@@ -88,19 +88,17 @@ trait Init[Scope]
 	* is explicitly defined and the where the scope matches `filter`.
 	* A setting initialized with dynamic dependencies is only allowed if `allowDynamic` is true.
 	* Only the static dependencies are tracked, however.  Dependencies on previous values do not introduce a derived setting either. */
-	final def derive[T](s: Setting[T], allowDynamic: Boolean = false, filter: Scope => Boolean = const(true), trigger: AttributeKey[_] => Boolean = const(true)): Setting[T] = {
+	final def derive[T](s: Setting[T], allowDynamic: Boolean = false, filter: Scope => Boolean = const(true), trigger: AttributeKey[_] => Boolean = const(true), default: Boolean = false): Setting[T] = {
 		deriveAllowed(s, allowDynamic) foreach error
-		new DerivedSetting[T](s.key, s.init, s.pos, filter, trigger, nextDefaultID())
+		val d = new DerivedSetting[T](s.key, s.init, s.pos, filter, trigger)
+		if (default) d.default() else d
 	}
 	def deriveAllowed[T](s: Setting[T], allowDynamic: Boolean): Option[String] = s.init match {
 		case _: Bind[_,_] if !allowDynamic => Some("Cannot derive from dynamic dependencies.")
 		case _ => None
 	}
 	// id is used for equality
-	private[sbt] final def defaultSetting[T](s: Setting[T]): Setting[T] = s match {
+	private[sbt] final def defaultSetting[T](s: Setting[T]): Setting[T] = s.default()
-		case _: DefaultSetting[_] | _: DerivedSetting[_] => s
-		case _ => new DefaultSetting[T](s.key, s.init, s.pos, nextDefaultID())
-	}
 	private[sbt] def defaultSettings(ss: Seq[Setting[_]]): Seq[Setting[_]] = ss.map(s => defaultSetting(s))
 	private[this] final val nextID = new java.util.concurrent.atomic.AtomicLong
 	private[this] final def nextDefaultID(): Long = nextID.incrementAndGet()
@@ -293,6 +291,14 @@ trait Init[Scope]
 		} else ""
 	}
 
+	/**
+	 * Intersects two scopes, returning the more specific one if they intersect, or None otherwise.
+	 */
+	private[sbt] def intersect(s1: Scope, s2: Scope)(implicit delegates: Scope => Seq[Scope]): Option[Scope] =
+		if      (delegates(s1).contains(s2))  Some(s1) // s1 is more specific
+		else if (delegates(s2).contains(s1))  Some(s2) // s2 is more specific
+		else None
+
 	private[this] def deriveAndLocal(init: Seq[Setting[_]])(implicit delegates: Scope => Seq[Scope], scopeLocal: ScopeLocal): Seq[Setting[_]] =
 	{
 			import collection.mutable
@@ -301,6 +307,7 @@ trait Init[Scope]
 			val dependencies = setting.dependencies.map(_.key)
 			def triggeredBy = dependencies.filter(setting.trigger)
 			val inScopes = new mutable.HashSet[Scope]
+			val outputs = new mutable.ListBuffer[Setting[_]]
 		}
 		final class Deriveds(val key: AttributeKey[_], val settings: mutable.ListBuffer[Derived]) {
 			def dependencies = settings.flatMap(_.dependencies)
@@ -312,6 +319,7 @@ trait Init[Scope]
 		val (derived, rawDefs) = Util.separate[Setting[_],Derived,Setting[_]](init) { case d: DerivedSetting[_] => Left(new Derived(d)); case s => Right(s) }
 		val defs = addLocal(rawDefs)(scopeLocal)
 
+
 		// group derived settings by the key they define
 		val derivsByDef = new mutable.HashMap[AttributeKey[_], Deriveds]
 		for(s <- derived) {
@@ -329,6 +337,10 @@ trait Init[Scope]
 		for(s <- derived; d <- s.triggeredBy)
 			derivedBy.getOrElseUpdate(d, new mutable.ListBuffer) += s
 
+		// Map a DerivedSetting[_] to the `Derived` struct wrapping it. Used to ultimately replace a DerivedSetting with
+		// the `Setting`s that were actually derived from it: `Derived.outputs`
+		val derivedToStruct: Map[DerivedSetting[_], Derived] = (derived map { s => s.setting -> s }).toMap
+
 		// set of defined scoped keys, used to ensure a derived setting is only added if all dependencies are present
 		val defined = new mutable.HashSet[ScopedKey[_]]
 		def addDefs(ss: Seq[Setting[_]]) { for(s <- ss) defined += s.key }
@@ -342,45 +354,53 @@ trait Init[Scope]
 		def allDepsDefined(d: Derived, scope: Scope, local: Set[AttributeKey[_]]): Boolean =
 			d.dependencies.forall(dep => local(dep) || isDefined(dep, scope))
 
-		// List of injectable derived settings and their local settings for `sk`.
+		// Returns the list of injectable derived settings and their local settings for `sk`.
+		// The settings are to be injected under `outputScope` = whichever scope is more specific of:
+		//   * the dependency's (`sk`) scope
+		//   * the DerivedSetting's scope in which it has been declared, `definingScope`
+		// provided that these two scopes intersect.
 		//  A derived setting is injectable if:
-		//   1. it has not been previously injected into this scope
+		//   1. it has not been previously injected into outputScope
-		//   2. it applies to this scope (as determined by its `filter`)
+		//   2. it applies to outputScope (as determined by its `filter`)
-		//   3. all of its dependencies that match `trigger` are defined for that scope (allowing for delegation)
+		//   3. all of its dependencies are defined for outputScope (allowing for delegation)
 		// This needs to handle local settings because a derived setting wouldn't be injected if it's local setting didn't exist yet.
 		val deriveFor = (sk: ScopedKey[_]) => {
 			val derivedForKey: List[Derived] = derivedBy.get(sk.key).toList.flatten
 			val scope = sk.scope
-			def localAndDerived(d: Derived): Seq[Setting[_]] =
+			def localAndDerived(d: Derived): Seq[Setting[_]] = {
-				if(!d.inScopes.contains(scope) && d.setting.filter(scope))
+				def definingScope = d.setting.key.scope
-				{
+				val outputScope = intersect(scope, definingScope)
-					val local = d.dependencies.flatMap(dep => scopeLocal(ScopedKey(scope, dep)))
+				outputScope collect { case s if !d.inScopes.contains(s) && d.setting.filter(s) =>
-					if(allDepsDefined(d, scope, local.map(_.key.key).toSet)) {
+					val local = d.dependencies.flatMap(dep => scopeLocal(ScopedKey(s, dep)))
-						d.inScopes.add(scope)
+					if(allDepsDefined(d, s, local.map(_.key.key).toSet)) {
-						local :+ d.setting.setScope(scope)
+						d.inScopes.add(s)
+						val out = local :+ d.setting.setScope(s)
+						d.outputs ++= out
+						out
 					} else
 						Nil
-				}
+				} getOrElse Nil
-				else Nil
+			}
 			derivedForKey.flatMap(localAndDerived)
 		}
 
 		val processed = new mutable.HashSet[ScopedKey[_]]
-		// valid derived settings to be added before normal settings
-		val out = new mutable.ListBuffer[Setting[_]]
 
 		// derives settings, transitively so that a derived setting can trigger another
 		def process(rem: List[Setting[_]]): Unit = rem match {
 			case s :: ss =>
 				val sk = s.key
 				val ds = if(processed.add(sk)) deriveFor(sk) else Nil
-				out ++= ds
 				addDefs(ds)
 				process(ds ::: ss)
 			case Nil =>
 		}
 		process(defs.toList)
-		out.toList ++ defs
+
+		// Take all the original defs and DerivedSettings along with locals, replace each DerivedSetting with the actual
+		// settings that were derived.
+		val allDefs = addLocal(init)(scopeLocal)
+		allDefs flatMap { case d: DerivedSetting[_] => (derivedToStruct get d map (_.outputs)).toStream.flatten; case s => Stream(s) }
 	}
 
 	sealed trait Initialize[T]
@@ -455,17 +475,28 @@ trait Init[Scope]
 		protected[this] def make[T](key: ScopedKey[T], init: Initialize[T], pos: SourcePosition): Setting[T] = new Setting[T](key, init, pos)
 		protected[sbt] def isDerived: Boolean = false
 		private[sbt] def setScope(s: Scope): Setting[T] = make(key.copy(scope = s), init.mapReferenced(mapScope(const(s))), pos)
+		/** Turn this setting into a `DefaultSetting` if it's not already, otherwise returns `this` */
+		private[sbt] def default(id: => Long = nextDefaultID()): DefaultSetting[T] = DefaultSetting(key, init, pos, id)
 	}
-	private[Init] final class DerivedSetting[T](sk: ScopedKey[T], i: Initialize[T], p: SourcePosition, val filter: Scope => Boolean, val trigger: AttributeKey[_] => Boolean, id: Long) extends DefaultSetting[T](sk, i, p, id) {
+	private[Init] sealed class DerivedSetting[T](sk: ScopedKey[T], i: Initialize[T], p: SourcePosition, val filter: Scope => Boolean, val trigger: AttributeKey[_] => Boolean) extends Setting[T](sk, i, p) {
-		override def make[T](key: ScopedKey[T], init: Initialize[T], pos: SourcePosition): Setting[T] = new DerivedSetting[T](key, init, pos, filter, trigger, id)
+		override def make[T](key: ScopedKey[T], init: Initialize[T], pos: SourcePosition): Setting[T] = new DerivedSetting[T](key, init, pos, filter, trigger)
 		protected[sbt] override def isDerived: Boolean = true
+		override def default(_id: => Long): DefaultSetting[T] = new DerivedSetting[T](sk, i, p, filter, trigger) with DefaultSetting[T] { val id = _id }
+		override def toString = "derived " + super.toString
 	}
 	// Only keep the first occurence of this setting and move it to the front so that it has lower precedence than non-defaults.
 	//  This is intended for internal sbt use only, where alternatives like Plugin.globalSettings are not available.
-	private[Init] sealed class DefaultSetting[T](sk: ScopedKey[T], i: Initialize[T], p: SourcePosition, val id: Long) extends Setting[T](sk, i, p) {
+	private[Init] sealed trait DefaultSetting[T] extends Setting[T] {
-		override def make[T](key: ScopedKey[T], init: Initialize[T], pos: SourcePosition): Setting[T] = new DefaultSetting[T](key, init, pos, id)
+		val id: Long
+		override def make[T](key: ScopedKey[T], init: Initialize[T], pos: SourcePosition): Setting[T] = super.make(key, init, pos) default id
 		override final def hashCode = id.hashCode
 		override final def equals(o: Any): Boolean = o match { case d: DefaultSetting[_] => d.id == id; case _ => false }
+		override def toString = s"default($id) " + super.toString
+		override def default(id: => Long) = this
+	}
+
+	object DefaultSetting {
+		def apply[T](sk: ScopedKey[T], i: Initialize[T], p: SourcePosition, _id: Long) = new Setting[T](sk, i, p) with DefaultSetting[T] { val id = _id }
 	}
 
 

util/collection/src/test/scala/SettingsExample.scala

@@ -3,7 +3,7 @@ package sbt
 /** Define our settings system */
 
 // A basic scope indexed by an integer.
-final case class Scope(index: Int)
+final case class Scope(nestIndex: Int, idAtIndex: Int = 0)
 
 // Extend the Init trait.
 //  (It is done this way because the Scope type parameter is used everywhere in Init.
@@ -14,12 +14,12 @@ object SettingsExample extends Init[Scope]
 {
 	// Provides a way of showing a Scope+AttributeKey[_]
 	val showFullKey: Show[ScopedKey[_]] = new Show[ScopedKey[_]] {
-		def apply(key: ScopedKey[_]) = key.scope.index + "/" + key.key.label
+		def apply(key: ScopedKey[_]) = s"${key.scope.nestIndex}(${key.scope.idAtIndex})/${key.key.label}"
 	}
 
 	// A sample delegation function that delegates to a Scope with a lower index.
-	val delegates: Scope => Seq[Scope] = { case s @ Scope(index) =>
+	val delegates: Scope => Seq[Scope] = { case s @ Scope(index, proj) =>
-		s +: (if(index <= 0) Nil else delegates(Scope(index-1)) )
+		s +: (if(index <= 0) Nil else { (if (proj > 0) List(Scope(index)) else Nil) ++: delegates(Scope(index-1)) })
 	}
 
 	// Not using this feature in this example.

util/collection/src/test/scala/SettingsTest.scala

@@ -7,6 +7,9 @@ import SettingsExample._
 
 object SettingsTest extends Properties("settings")
 {
+
+	import scala.reflect.Manifest
+
 	final val ChainMax = 5000
 	lazy val chainLengthGen = Gen.choose(1, ChainMax)
 
@@ -35,31 +38,84 @@ object SettingsTest extends Properties("settings")
 		evaluate( setting(chk, iterate(top)) :: Nil); true
 	}
 
-  property("Derived setting chain depending on (prev derived, normal setting)") = forAllNoShrink(Gen.choose(1, 100)) { derivedSettings }
+	property("Derived setting chain depending on (prev derived, normal setting)") = forAllNoShrink(Gen.choose(1, 100)) { derivedSettings }
-  final def derivedSettings(nr: Int): Prop =
+	final def derivedSettings(nr: Int): Prop =
-  {
+	{
-    val alphaStr = Gen.alphaStr
+		val genScopedKeys = {
-    val genScopedKeys = {
+			val attrKeys = mkAttrKeys[Int](nr)
-      val attrKeys = for {
+			attrKeys map (_ map (ak => ScopedKey(Scope(0), ak)))
-        list <- Gen.listOfN(nr, alphaStr) suchThat (l => l.size == l.distinct.size)
+		}
-        item <- list
+		forAll(genScopedKeys) { scopedKeys =>
-      } yield AttributeKey[Int](item)
+			val last = scopedKeys.last
-      attrKeys map (_ map (ak => ScopedKey(Scope(0), ak)))
+			val derivedSettings: Seq[Setting[Int]] = (
-    }
+				for {
-    forAll(genScopedKeys) { scopedKeys =>
+					List(scoped0, scoped1) <- chk :: scopedKeys sliding 2
-      val last = scopedKeys.last
+					nextInit = if (scoped0 == chk) chk
-      val derivedSettings: Seq[Setting[Int]] = (
+										 else (scoped0 zipWith chk) { (p, _) => p + 1 }
-        for {
+				} yield derive(setting(scoped1, nextInit))
-          List(scoped0, scoped1) <- chk :: scopedKeys sliding 2
+			).toSeq
-          nextInit = if (scoped0 == chk) chk
-                     else (scoped0 zipWith chk) { (p, _) => p + 1 }
-        } yield derive(setting(scoped1, nextInit))
-      ).toSeq
 
-      { checkKey(last, Some(nr-1), evaluate(setting(chk, value(0)) +: derivedSettings)) :| "Not derived?" } &&
+			{ checkKey(last, Some(nr-1), evaluate(setting(chk, value(0)) +: derivedSettings)) :| "Not derived?" } &&
-      { checkKey( last, None, evaluate(derivedSettings)) :| "Should not be derived" }
+			{ checkKey( last, None, evaluate(derivedSettings)) :| "Should not be derived" }
-    }
+		}
-  }
+	}
+
+	private def mkAttrKeys[T](nr: Int)(implicit mf: Manifest[T]): Gen[List[AttributeKey[T]]] =
+	{
+		val alphaStr = Gen.alphaStr
+		for {
+			list <- Gen.listOfN(nr, alphaStr) suchThat (l => l.size == l.distinct.size)
+			item <- list
+		} yield AttributeKey[T](item)
+	}
+
+	property("Derived setting(s) replace DerivedSetting in the Seq[Setting[_]]") = derivedKeepsPosition
+	final def derivedKeepsPosition: Prop =
+	{
+		val a: ScopedKey[Int] = ScopedKey(Scope(0), AttributeKey[Int]("a"))
+		val b: ScopedKey[Int] = ScopedKey(Scope(0), AttributeKey[Int]("b"))
+		val prop1 = {
+			val settings: Seq[Setting[_]] = Seq(
+				setting(a, value(3)),
+				setting(b, value(6)),
+				derive(setting(b, a)),
+				setting(a, value(5)),
+				setting(b, value(8))
+			)
+			val ev = evaluate(settings)
+			checkKey(a, Some(5), ev) && checkKey(b, Some(8), ev)
+		}
+		val prop2 = {
+			val settings: Seq[Setting[Int]] = Seq(
+				setting(a, value(3)),
+				setting(b, value(6)),
+				derive(setting(b, a)),
+				setting(a, value(5))
+			)
+			val ev = evaluate(settings)
+			checkKey(a, Some(5), ev) && checkKey(b, Some(5), ev)
+		}
+		prop1 && prop2
+	}
+
+	property("DerivedSetting in ThisBuild scopes derived settings under projects thus allowing safe +=") = forAllNoShrink(Gen.choose(1, 100)) { derivedSettingsScope }
+	final def derivedSettingsScope(nrProjects: Int): Prop =
+	{
+		forAll(mkAttrKeys[Int](2)) { case List(key, derivedKey) =>
+			val projectKeys = for { proj <- 1 to nrProjects } yield ScopedKey(Scope(1, proj), key)
+			val projectDerivedKeys = for { proj <- 1 to nrProjects } yield ScopedKey(Scope(1, proj), derivedKey)
+			val globalKey   = ScopedKey(Scope(0), key)
+			val globalDerivedKey = ScopedKey(Scope(0), derivedKey)
+			// Each project defines an initial value, but the update is defined in globalKey.
+			// However, the derived Settings that come from this should be scoped in each project.
+			val settings: Seq[Setting[_]] =
+				derive(setting(globalDerivedKey, SettingsExample.map(globalKey)(_ + 1))) +: projectKeys.map(pk => setting(pk, value(0)))
+			val ev = evaluate(settings)
+			// Also check that the key has no value at the "global" scope
+			val props = for { pk <- projectDerivedKeys } yield checkKey(pk, Some(1), ev)
+			checkKey(globalDerivedKey, None, ev) && Prop.all(props: _*)
+		}
+	}
 
 // Circular (dynamic) references currently loop infinitely.
 //  This is the expected behavior (detecting dynamic cycles is expensive),