From 3d981177df699f30b9e363504d88d290185e4720 Mon Sep 17 00:00:00 2001 From: Antonio Cunei Date: Wed, 10 Sep 2014 02:36:37 +0200 Subject: [PATCH 1/3] Adding test for broken "set every", see #1430 To test, use: > scripted tests/set-every --- sbt/src/sbt-test/tests/set-every/build.sbt | 16 ++++++++++++++++ sbt/src/sbt-test/tests/set-every/test | 3 +++ 2 files changed, 19 insertions(+) create mode 100644 sbt/src/sbt-test/tests/set-every/build.sbt create mode 100644 sbt/src/sbt-test/tests/set-every/test diff --git a/sbt/src/sbt-test/tests/set-every/build.sbt b/sbt/src/sbt-test/tests/set-every/build.sbt new file mode 100644 index 000000000..bd8e9b438 --- /dev/null +++ b/sbt/src/sbt-test/tests/set-every/build.sbt @@ -0,0 +1,16 @@ +val a = project.settings(version := "2.8.1") + +val trySetEvery = taskKey[Unit]("Tests \"set every\"") + +trySetEvery := { + val s = state.value + val extracted = Project.extract(s) + import extracted._ + val allProjs = structure.allProjectRefs + val Some(aProj) = allProjs.find(_.project == "a") + val aVer = (version in aProj get structure.data).get + if (aVer != "1.0") { + println("Version of project a: " + aVer + ", expected: 1.0") + error("\"set every\" did not change the version of all projects.") + } +} diff --git a/sbt/src/sbt-test/tests/set-every/test b/sbt/src/sbt-test/tests/set-every/test new file mode 100644 index 000000000..8fa56c46e --- /dev/null +++ b/sbt/src/sbt-test/tests/set-every/test @@ -0,0 +1,3 @@ +> set every version := '"1.0"' +> trySetEvery + From 9aa0985f4be0d654bf07a84e80ff8faec02fa653 Mon Sep 17 00:00:00 2001 From: Antonio Cunei Date: Thu, 11 Sep 2014 02:04:17 +0200 Subject: [PATCH 2/3] This commit reverts part of 322f6de6551665cade7d56b532348ea5dc3d54db The implementation of Relation should in theory make no difference whether an element is unmapped, or whether it is mapped to an empty set. One of the changes in 322f6de6551665cade7d56b532348ea5dc3d54db introduced an optimization to the '+' operation on Relations that, in theory, should have made no difference to the semantic. The result of that optimization is that some mappings of the form "elem -> Set()" are no longer inserted in the forwardMap of the Relation. Unfortunately, the change resulted in the breakage of #1430, causing "set every" to behave incorrectly. There must be, somewhere in the code, a test on the presence of a key rather than an access via .get(), or some other access that bypasses the supposed semantic equivalence described above. I spent several hours trying to track down exactly the offending test, without success. By undoing the relevant change in 322f6de6551665cade, "set every" works again. That however offers no guarantee that everything else will keep working correctly; the underlying quirk in the code that depends on this supposedly inessential detail is also still lurking in the code, which is less than ideal. --- util/relation/src/main/scala/sbt/Relation.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/relation/src/main/scala/sbt/Relation.scala b/util/relation/src/main/scala/sbt/Relation.scala index 987aafb14..dcd38fa90 100644 --- a/util/relation/src/main/scala/sbt/Relation.scala +++ b/util/relation/src/main/scala/sbt/Relation.scala @@ -129,7 +129,7 @@ private final class MRelation[A, B](fwd: Map[A, Set[B]], rev: Map[B, Set[A]]) ex def +(pair: (A, B)) = this + (pair._1, Set(pair._2)) def +(from: A, to: B) = this + (from, to :: Nil) - def +(from: A, to: Traversable[B]) = if (to.isEmpty) this else + def +(from: A, to: Traversable[B]) = new MRelation(add(fwd, from, to), (rev /: to) { (map, t) => add(map, t, from :: Nil) }) def ++(rs: Traversable[(A, B)]) = ((this: Relation[A, B]) /: rs) { _ + _ } From 9b94a16b73fd708a9fe33cf8c1548d62634a4c07 Mon Sep 17 00:00:00 2001 From: Antonio Cunei Date: Fri, 12 Sep 2014 20:51:04 +0200 Subject: [PATCH 3/3] Undone the revert on the optimization, and fixed setAll() The optimization, and therefore the change in the behavior of Relation, is now needed by the class Logic, and cannot be reverted. This patch (written by Josh) therefore changes the implementation of setAll() so that _1s is no longer used. --- main/src/main/scala/sbt/SettingCompletions.scala | 4 ++-- util/relation/src/main/scala/sbt/Relation.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/main/src/main/scala/sbt/SettingCompletions.scala b/main/src/main/scala/sbt/SettingCompletions.scala index f3668de79..6b94c4c6b 100644 --- a/main/src/main/scala/sbt/SettingCompletions.scala +++ b/main/src/main/scala/sbt/SettingCompletions.scala @@ -27,7 +27,7 @@ private[sbt] object SettingCompletions { { import extracted._ val r = relation(extracted.structure, true) - val allDefs = r._1s.toSeq + val allDefs = Def.flattenLocals(Def.compiled(extracted.structure.settings, true)(structure.delegates, structure.scopeLocal, implicitly[Show[ScopedKey[_]]])).map(_._1) val projectScope = Load.projectScope(currentRef) def resolve(s: Setting[_]): Seq[Setting[_]] = Load.transformSettings(projectScope, currentRef.build, rootProject, s :: Nil) def rescope[T](setting: Setting[T]): Seq[Setting[_]] = @@ -353,4 +353,4 @@ private[sbt] object SettingCompletions { classOf[Long], classOf[String] ) -} \ No newline at end of file +} diff --git a/util/relation/src/main/scala/sbt/Relation.scala b/util/relation/src/main/scala/sbt/Relation.scala index dcd38fa90..987aafb14 100644 --- a/util/relation/src/main/scala/sbt/Relation.scala +++ b/util/relation/src/main/scala/sbt/Relation.scala @@ -129,7 +129,7 @@ private final class MRelation[A, B](fwd: Map[A, Set[B]], rev: Map[B, Set[A]]) ex def +(pair: (A, B)) = this + (pair._1, Set(pair._2)) def +(from: A, to: B) = this + (from, to :: Nil) - def +(from: A, to: Traversable[B]) = + def +(from: A, to: Traversable[B]) = if (to.isEmpty) this else new MRelation(add(fwd, from, to), (rev /: to) { (map, t) => add(map, t, from :: Nil) }) def ++(rs: Traversable[(A, B)]) = ((this: Relation[A, B]) /: rs) { _ + _ }