From 7beb23e818a181e2980da7261e6300d1d50e91da Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Thu, 12 Oct 2023 13:59:36 -0400 Subject: [PATCH] Make loading order alphabetical for plugins **Problem** Plugins are topologically sorted, but plugins with equal weigh could modify the same key via `~=` etc, resulting in different builds depending on the artifact. **Solution** This attempts to fix that by first sorting the selected plugins by the class name before sorting it topologically. --- main/src/main/scala/sbt/Plugins.scala | 4 +- main/src/test/scala/PluginsTest.scala | 6 +- .../sbt-test/project/auto-plugins/build.sbt | 65 ++++++++++--------- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/main/src/main/scala/sbt/Plugins.scala b/main/src/main/scala/sbt/Plugins.scala index 92a9947cdc..42a19b4bc3 100644 --- a/main/src/main/scala/sbt/Plugins.scala +++ b/main/src/main/scala/sbt/Plugins.scala @@ -216,12 +216,12 @@ object Plugins extends PluginsFunctions { case Right(results) => log.debug(s" :: deduced result: ${results}") val selectedAtoms: List[Atom] = results.ordered - val selectedPlugins = selectedAtoms map { a => + val selectedPlugins = (selectedAtoms map { a => byAtomMap.getOrElse( a, throw AutoPluginException(s"${a} was not found in atom map.") ) - } + }).sortBy(_.getClass.getName) val forbidden: Set[AutoPlugin] = (selectedPlugins flatMap { Plugins.asExclusions }).toSet val c = selectedPlugins.toSet & forbidden diff --git a/main/src/test/scala/PluginsTest.scala b/main/src/test/scala/PluginsTest.scala index e7bdcf8262..f0fc850e1e 100644 --- a/main/src/test/scala/PluginsTest.scala +++ b/main/src/test/scala/PluginsTest.scala @@ -48,7 +48,7 @@ object PluginsTest extends verify.BasicTestSuite { assertEquals( s"""Contradiction in enabled plugins: - requested: sbt.AI$$S - - enabled: sbt.AI$$S, sbt.AI$$Q, sbt.AI$$R, sbt.AI$$B, sbt.AI$$A + - enabled: sbt.AI$$A, sbt.AI$$B, sbt.AI$$Q, sbt.AI$$R, sbt.AI$$S - conflict: sbt.AI$$R is enabled by sbt.AI$$Q; excluded by sbt.AI$$S""", e.message ) @@ -63,8 +63,8 @@ object PluginsTest extends verify.BasicTestSuite { assertEquals( s"""Contradiction in enabled plugins: - requested: sbt.AI$$T && sbt.AI$$U - - enabled: sbt.AI$$U, sbt.AI$$T, sbt.AI$$A, sbt.AI$$Q, sbt.AI$$R, sbt.AI$$B - - conflict: sbt.AI$$Q is enabled by sbt.AI$$A && sbt.AI$$B; required by sbt.AI$$T, sbt.AI$$R; excluded by sbt.AI$$U + - enabled: sbt.AI$$A, sbt.AI$$B, sbt.AI$$Q, sbt.AI$$R, sbt.AI$$T, sbt.AI$$U + - conflict: sbt.AI$$Q is enabled by sbt.AI$$A && sbt.AI$$B; required by sbt.AI$$R, sbt.AI$$T; excluded by sbt.AI$$U - conflict: sbt.AI$$R is enabled by sbt.AI$$Q; excluded by sbt.AI$$T""", e.message ) diff --git a/sbt-app/src/sbt-test/project/auto-plugins/build.sbt b/sbt-app/src/sbt-test/project/auto-plugins/build.sbt index f564d48797..99b366f08b 100644 --- a/sbt-app/src/sbt-test/project/auto-plugins/build.sbt +++ b/sbt-app/src/sbt-test/project/auto-plugins/build.sbt @@ -29,37 +29,38 @@ lazy val projI = project.enablePlugins(TopC) lazy val disableAutoNoRequirePlugin = project.disablePlugins(OrgPlugin) check := { - // Ensure organization on root is overridable. - val rorg = (organization).value // Should be None - same(rorg, "override", "organization") - // this will pass when the raw disablePlugin works. - val dversion = (projectID in projD).?.value // Should be None - same(dversion, None, "projectID in projD") - -// Ensure with multiple .sbt files that disabling/enabling works across them - val fDel = (del in Quux in projF).?.value - same(fDel, Some(" Q"), "del in Quux in projF") -// - val adel = (del in projA).?.value // should be None - same(adel, None, "del in projA") - val bdel = (del in projB).?.value // should be None - same(bdel, None, "del in projB") - val ddel = (del in projD).?.value // should be None - same(ddel, None, "del in projD") -// - val buildValue = (demo in ThisBuild).value - same(buildValue, "build 0", "demo in ThisBuild") - val globalValue = (demo in Global).value - same(globalValue, "global 0", "demo in Global") - val projValue = (demo in projC).?.value - same(projValue, Some("project projC Q R"), "demo in projC") - val qValue = (del in projC in Quux).?.value - same(qValue, Some(" Q R"), "del in projC in Quux") - val optInValue = (del in projE in Quux).value - same(optInValue, " Q S R", "del in projE in Quux") - val overrideOrgValue = (organization in projE).value - same(overrideOrgValue, "S", "organization in projE") -// tests for top level plugins + // Ensure organization on root is overridable. + val rorg = (organization).value // Should be None + same(rorg, "override", "organization") + // this will pass when the raw disablePlugin works. + val dversion = (projectID in projD).?.value // Should be None + same(dversion, None, "projectID in projD") + + // Ensure with multiple .sbt files that disabling/enabling works across them + val fDel = (del in Quux in projF).?.value + same(fDel, Some(" Q"), "del in Quux in projF") + + val adel = (del in projA).?.value // should be None + same(adel, None, "del in projA") + val bdel = (del in projB).?.value // should be None + same(bdel, None, "del in projB") + val ddel = (del in projD).?.value // should be None + same(ddel, None, "del in projD") + + val buildValue = (demo in ThisBuild).value + same(buildValue, "build 0", "demo in ThisBuild") + val globalValue = (demo in Global).value + same(globalValue, "global 0", "demo in Global") + val projValue = (demo in projC).?.value + same(projValue, Some("project projC Q R"), "demo in projC") + val qValue = (del in projC in Quux).?.value + same(qValue, Some(" Q R"), "del in projC in Quux") + val optInValue = (del in projE in Quux).value + same(optInValue, " Q R S", "del in projE in Quux") + val overrideOrgValue = (organization in projE).value + same(overrideOrgValue, "S", "organization in projE") + + // tests for top level plugins val topLevelAValueG = (topLevelDemo in projG).value same(topLevelAValueG, "TopA: topLevelDemo project projG", "topLevelDemo in projG") val demoValueG = (demo in projG).value @@ -75,5 +76,5 @@ keyTest := "foo" topLevelKeyTest := "bar" def same[T](actual: T, expected: T, label: String): Unit = { - assert(actual == expected, s"Expected '$expected' for `$label`, got '$actual'") + assert(actual == expected, s"Expected '$expected' for `$label`, got '$actual'") }