From 0cbbee44181094818dd91103455bdf5afdb2c173 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 3 Dec 2019 09:53:45 -0800 Subject: [PATCH 1/2] Don't import fields from local variables I found it hard to reason about where certain local variables, like currentRef, were coming from. I also changed 'x' to 'extracted' in a few places for clarity as well. --- main/src/main/scala/sbt/Cross.scala | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/main/src/main/scala/sbt/Cross.scala b/main/src/main/scala/sbt/Cross.scala index 778002089..7b8332053 100644 --- a/main/src/main/scala/sbt/Cross.scala +++ b/main/src/main/scala/sbt/Cross.scala @@ -82,16 +82,17 @@ object Cross { s => if (s get sessionSettings isEmpty) failure("No project loaded") else p(s) private def resolveAggregates(extracted: Extracted): Seq[ProjectRef] = { - import extracted._ - def findAggregates(project: ProjectRef): List[ProjectRef] = { - project :: (structure.allProjects(project.build).find(_.id == project.project) match { + def findAggregates(project: ProjectRef): Seq[ProjectRef] = { + project :: (extracted.structure + .allProjects(project.build) + .find(_.id == project.project) match { case Some(resolved) => resolved.aggregate.toList.flatMap(findAggregates) case None => Nil }) } - (currentRef :: currentProject.aggregate.toList.flatMap(findAggregates)).distinct + (extracted.currentRef +: extracted.currentProject.aggregate.flatMap(findAggregates)).distinct } private def crossVersions(extracted: Extracted, proj: ResolvedReference): Seq[String] = { @@ -127,12 +128,11 @@ object Cross { Command.arb(requireSession(crossParser), crossHelp)(crossBuildCommandImpl) private def crossBuildCommandImpl(state: State, args: CrossArgs): State = { - val x = Project.extract(state) - import x._ - val (aggs, aggCommand) = parseSlashCommand(x)(args.command) + val extracted = Project.extract(state) + val (aggs, aggCommand) = parseSlashCommand(extracted)(args.command) val projCrossVersions = aggs map { proj => - proj -> crossVersions(x, proj) + proj -> crossVersions(extracted, proj) } // if we support scalaVersion, projVersions should be cached somewhere since // running ++2.11.1 is at the root level is going to mess with the scalaVersion for the aggregated subproj @@ -146,7 +146,7 @@ object Cross { state } else { // Detect whether a task or command has been issued - val allCommands = Parser.parse(aggCommand, Act.aggregatedKeyParser(x)) match { + val allCommands = Parser.parse(aggCommand, Act.aggregatedKeyParser(extracted)) match { case Left(_) => // It's definitely not a task, check if it's a valid command, because we don't want to emit the warning // message below for typos. @@ -168,7 +168,7 @@ object Cross { } // Execute using a blanket switch - projCrossVersions.toMap.apply(currentRef).flatMap { version => + projCrossVersions.toMap.apply(extracted.currentRef).flatMap { version => // Force scala version Seq(s"$SwitchCommand $verbose $version!", aggCommand) } @@ -195,7 +195,7 @@ object Cross { } } - allCommands.toList ::: CrossRestoreSessionCommand :: captureCurrentSession(state, x) + allCommands.toList ::: CrossRestoreSessionCommand :: captureCurrentSession(state, extracted) } } @@ -229,7 +229,6 @@ object Cross { Command.arb(requireSession(switchParser), switchHelp)(switchCommandImpl) private def switchCommandImpl(state: State, args: Switch): State = { - val x = Project.extract(state) val (switchedState, affectedRefs) = switchScalaVersion(args, state) val strictCmd = @@ -238,7 +237,7 @@ object Cross { args.command } else { args.command.map { rawCmd => - val (aggs, aggCommand) = parseSlashCommand(x)(rawCmd) + val (aggs, aggCommand) = parseSlashCommand(Project.extract(state))(rawCmd) aggs .intersect(affectedRefs) .map({ case ProjectRef(_, proj) => s"$proj/$aggCommand" }) From 53788ba3561a4e79bc9c36c329f43540ad48814e Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 3 Dec 2019 09:49:02 -0800 Subject: [PATCH 2/2] Support input tasks in cross (+) command --- main/src/main/scala/sbt/Cross.scala | 123 +++++++++--------- .../actions/cross-test-only/build.sbt | 2 + .../actions/cross-test-only/foo/build.sbt | 3 + .../foo/src/test/scala/foo/FooSpec.scala | 9 ++ sbt/src/sbt-test/actions/cross-test-only/test | 3 + .../sbt/scriptedtest/ScriptedTests.scala | 7 +- 6 files changed, 83 insertions(+), 64 deletions(-) create mode 100644 sbt/src/sbt-test/actions/cross-test-only/build.sbt create mode 100644 sbt/src/sbt-test/actions/cross-test-only/foo/build.sbt create mode 100644 sbt/src/sbt-test/actions/cross-test-only/foo/src/test/scala/foo/FooSpec.scala create mode 100644 sbt/src/sbt-test/actions/cross-test-only/test diff --git a/main/src/main/scala/sbt/Cross.scala b/main/src/main/scala/sbt/Cross.scala index 7b8332053..a37e8bbc7 100644 --- a/main/src/main/scala/sbt/Cross.scala +++ b/main/src/main/scala/sbt/Cross.scala @@ -129,74 +129,75 @@ object Cross { private def crossBuildCommandImpl(state: State, args: CrossArgs): State = { val extracted = Project.extract(state) - val (aggs, aggCommand) = parseSlashCommand(extracted)(args.command) - - val projCrossVersions = aggs map { proj => - proj -> crossVersions(extracted, proj) - } - // if we support scalaVersion, projVersions should be cached somewhere since - // running ++2.11.1 is at the root level is going to mess with the scalaVersion for the aggregated subproj - val projVersions = (projCrossVersions flatMap { - case (proj, versions) => versions map { proj.project -> _ } - }).toList - + val parser = Act.aggregatedKeyParser(extracted) ~ matched(any.*) val verbose = if (args.verbose) "-v" else "" + val allCommands = Parser.parse(args.command, parser) match { + case Left(_) => + val (aggs, aggCommand) = parseSlashCommand(extracted)(args.command) + val projCrossVersions = aggs map { proj => + proj -> crossVersions(extracted, proj) + } + // It's definitely not a task, check if it's a valid command, because we don't want to emit the warning + // message below for typos. + val validCommand = Parser.parse(aggCommand, state.combinedParser).isRight - if (projVersions.isEmpty) { - state - } else { - // Detect whether a task or command has been issued - val allCommands = Parser.parse(aggCommand, Act.aggregatedKeyParser(extracted)) match { - case Left(_) => - // It's definitely not a task, check if it's a valid command, because we don't want to emit the warning - // message below for typos. - val validCommand = Parser.parse(aggCommand, state.combinedParser).isRight - - val distinctCrossConfigs = projCrossVersions.map(_._2.toSet).distinct - if (validCommand && distinctCrossConfigs.size > 1) { - state.log.warn( - "Issuing a cross building command, but not all sub projects have the same cross build " + - "configuration. This could result in subprojects cross building against Scala versions that they are " + - "not compatible with. Try issuing cross building command with tasks instead, since sbt will be able " + - "to ensure that cross building is only done using configured project and Scala version combinations " + - "that are configured." - ) - state.log.debug("Scala versions configuration is:") - projCrossVersions.foreach { - case (project, versions) => state.log.debug(s"$project: $versions") - } + val distinctCrossConfigs = projCrossVersions.map(_._2.toSet).distinct + if (validCommand && distinctCrossConfigs.size > 1) { + state.log.warn( + "Issuing a cross building command, but not all sub projects have the same cross build " + + "configuration. This could result in subprojects cross building against Scala versions that they are " + + "not compatible with. Try issuing cross building command with tasks instead, since sbt will be able " + + "to ensure that cross building is only done using configured project and Scala version combinations " + + "that are configured." + ) + state.log.debug("Scala versions configuration is:") + projCrossVersions.foreach { + case (project, versions) => state.log.debug(s"$project: $versions") } + } - // Execute using a blanket switch - projCrossVersions.toMap.apply(extracted.currentRef).flatMap { version => - // Force scala version - Seq(s"$SwitchCommand $verbose $version!", aggCommand) + // Execute using a blanket switch + projCrossVersions.toMap.apply(extracted.currentRef).flatMap { version => + // Force scala version + Seq(s"$SwitchCommand $verbose $version!", aggCommand) + } + case Right((keys, taskArgs)) => + def project(key: ScopedKey[_]): Option[ProjectRef] = key.scope.project.toOption match { + case Some(p: ProjectRef) => Some(p) + case _ => None + } + val fullArgs = if (taskArgs.trim.isEmpty) "" else s" ${taskArgs.trim}" + val keysByVersion = keys + .flatMap { k => + project(k).toSeq.flatMap(crossVersions(extracted, _).map(v => v -> k)) } - - case Right(_) => - // We have a key, we're likely to be able to cross build this using the per project behaviour. - - // Group all the projects by scala version - projVersions.groupBy(_._2).mapValues(_.map(_._1)).toSeq.flatMap { - case (version, Seq(project)) => - // If only one project for a version, issue it directly - Seq(s"$SwitchCommand $verbose $version $project/$aggCommand") - case (version, projects) if aggCommand.contains(" ") => - // If the command contains a space, then the `all` command won't work because it doesn't support issuing - // commands with spaces, so revert to running the command on each project one at a time - s"$SwitchCommand $verbose $version" :: projects - .map(project => s"$project/$aggCommand") - case (version, projects) => - // First switch scala version, then use the all command to run the command on each project concurrently - Seq( - s"$SwitchCommand $verbose $version", - projects.map(_ + "/" + aggCommand).mkString("all ", " ", "") - ) + .groupBy(_._1) + .mapValues(_.map(_._2).toSet) + val commandsByVersion = keysByVersion.toSeq + .flatMap { + case (v, keys) => + val projects = keys.flatMap(project) + keys.toSeq.flatMap { k => + project(k).filter(projects.contains).flatMap { p => + if (p == extracted.currentRef || !projects.contains(extracted.currentRef)) { + val parts = project(k).map(_.project) ++ k.scope.config.toOption.map { + case ConfigKey(n) => n.head.toUpper + n.tail + } ++ k.scope.task.toOption.map(_.label) ++ Some(k.key.label) + Some(v -> parts.mkString("", " / ", fullArgs)) + } else None + } + } } - } - - allCommands.toList ::: CrossRestoreSessionCommand :: captureCurrentSession(state, extracted) + .groupBy(_._1) + .mapValues(_.map(_._2)) + .toSeq + .sortBy(_._1) + commandsByVersion.flatMap { + case (v, commands) => + Seq(s"$SwitchCommand $verbose $v!") ++ commands + } } + allCommands.toList ::: CrossRestoreSessionCommand :: captureCurrentSession(state, extracted) } def crossRestoreSession: Command = diff --git a/sbt/src/sbt-test/actions/cross-test-only/build.sbt b/sbt/src/sbt-test/actions/cross-test-only/build.sbt new file mode 100644 index 000000000..687731363 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-test-only/build.sbt @@ -0,0 +1,2 @@ +val foo = project +val root = (project in file(".")).aggregate(foo) \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/cross-test-only/foo/build.sbt b/sbt/src/sbt-test/actions/cross-test-only/foo/build.sbt new file mode 100644 index 000000000..c7126a4fc --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-test-only/foo/build.sbt @@ -0,0 +1,3 @@ +crossScalaVersions := Seq("2.12.10", "2.13.1") + +libraryDependencies += "org.scalatest" %% "scalatest" % "3.1.0" \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/cross-test-only/foo/src/test/scala/foo/FooSpec.scala b/sbt/src/sbt-test/actions/cross-test-only/foo/src/test/scala/foo/FooSpec.scala new file mode 100644 index 000000000..87c315f92 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-test-only/foo/src/test/scala/foo/FooSpec.scala @@ -0,0 +1,9 @@ +package foo + +import org.scalatest.FlatSpec + +class FooSpec extends FlatSpec { + "foo" should "bar" in { + assert(true) + } +} \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/cross-test-only/test b/sbt/src/sbt-test/actions/cross-test-only/test new file mode 100644 index 000000000..3183628c0 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-test-only/test @@ -0,0 +1,3 @@ +> + foo / testOnly foo.FooSpec + +> + testOnly foo.FooSpec diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala index c07d5263a..db1041c44 100644 --- a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala +++ b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala @@ -192,9 +192,10 @@ final class ScriptedTests( LauncherBased // java.lang.ClassNotFoundException: javax.tools.DiagnosticListener when run with java 11 and an old sbt launcher case "actions/multi-command" => LauncherBased // java.lang.ClassNotFoundException: javax.tools.DiagnosticListener when run with java 11 and an old sbt launcher - case "actions/external-doc" => LauncherBased // sbt/Package$ - case "actions/input-task" => LauncherBased // sbt/Package$ - case "actions/input-task-dyn" => LauncherBased // sbt/Package$ + case "actions/cross-test-only" => LauncherBased // tbd + case "actions/external-doc" => LauncherBased // sbt/Package$ + case "actions/input-task" => LauncherBased // sbt/Package$ + case "actions/input-task-dyn" => LauncherBased // sbt/Package$ case gn if gn.startsWith("classloader-cache/") => LauncherBased // This should be tested using launcher case "compiler-project/dotty-compiler-plugin" => LauncherBased // sbt/Package$