Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow checking in an on-the-fly mode while still benefiting from other sbt settings #680

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

nafg
Copy link
Contributor

@nafg nafg commented Jan 7, 2022

See slick/slick#2356, scalacenter/sbt-version-policy#119, scalacenter/sbt-version-policy#121

Basically, while I want to use MiMa the way most people do, to make sure version bumps are the correct bump type, I also want to generate a report of changes (and not by the widespread "solution" of encoding the changelog as MiMa filters). This means that while the "real" SBT setting may be a check direction of "none", or with sbt-version-policy the versionPollcyIntention may be Compatibility.None, I still want to have the ability to run MiMa and get back information that I can present in an arbitrary format to the user. It just shouldn't break the build.

I first implemented a PR generating a markdown report for Slick by creating a separate SBT configuration so that there can be a separate "namespace" of settings, but that's inelegant. This PR allows to hook in to the way the plugin configure MiMa from SBT while still specifying certain parameters on the fly.

With this change plus scalacenter/sbt-version-policy#121, my Slick markdown report task is a lot more straightforward (expand to see diff). (Well it's longer, which would be nice to improve somehow, but it's less hacky...)
diff --git a/project/Versioning.scala b/project/Versioning.scala
index 457183cc..377b27e8 100644
--- a/project/Versioning.scala
+++ b/project/Versioning.scala
@@ -1,16 +1,13 @@
-import com.typesafe.tools.mima.plugin.MimaKeys.mimaFindBinaryIssues
 import com.typesafe.tools.mima.plugin.MimaPlugin
 import coursier.version.Version
-import sbt.librarymanagement.CrossVersion
-import sbt.{AutoPlugin, Compile, Def, Defaults, Keys, config, inConfig, settingKey, taskKey}
+import sbt.{AutoPlugin, Keys, taskKey}
 import sbtdynver.DynVerPlugin.autoImport.{dynver, dynverCurrentDate, dynverGitDescribeOutput}
 import sbtdynver.{DynVer, GitDescribeOutput}
-import sbtversionpolicy.SbtVersionPolicyMima.autoImport.versionPolicyPreviousVersions
-import sbtversionpolicy.SbtVersionPolicyPlugin.autoImport._
-import sbtversionpolicy.{DependencyCheckReport, Direction, SbtVersionPolicyMima, SbtVersionPolicyPlugin}
+import sbtversionpolicy.SbtVersionPolicyMima.previousVersionsFromRepo
+import sbtversionpolicy.SbtVersionPolicyPlugin.autoImport.{versionPolicyDependencyIssuesReporter, versionPolicyIntention}
+import sbtversionpolicy._
 
 import java.util.Date
-import scala.jdk.CollectionConverters._
 
 
 object Versioning extends AutoPlugin {
@@ -49,38 +46,9 @@ object Versioning extends AutoPlugin {
   def versionFor(compat: Compatibility, date: Date = new Date): Option[String] =
     DynVer.getGitDescribeOutput(date).map(versionFor(compat, _))
 
-  // Code copied from sbt-version-policy
-  private lazy val previousVersionsFromRepo = Def.setting {
-    val projId = Keys.projectID.value
-    val sv = Keys.scalaVersion.value
-    val sbv = Keys.scalaBinaryVersion.value
-    val name = CrossVersion(projId.crossVersion, sv, sbv).fold(projId.name)(_ (projId.name))
-
-    val ivyProps = sbtversionpolicy.internal.Resolvers.defaultIvyProperties(Keys.ivyPaths.value.ivyHome)
-    val repos = Keys.resolvers.value.flatMap { res =>
-      val repoOpt = sbtversionpolicy.internal.Resolvers.repository(res, ivyProps, s => System.err.println(s))
-      if (repoOpt.isEmpty)
-        System.err.println(s"Warning: ignoring repository ${res.name} to get previous version")
-      repoOpt.toSeq
-    }
-    // Can't reference Keys.fullResolvers, which is a task.
-    // So we're using the usual default repositories from coursier here…
-    val fullRepos = coursierapi.Repository.defaults().asScala ++ repos
-    val res = coursierapi.Versions.create()
-      .withRepositories(fullRepos: _*)
-      .withModule(coursierapi.Module.of(projId.organization, name))
-      .versions()
-    res.getMergedListings.getAvailable.asScala
-  }
-
-
   override def trigger = allRequirements
 
-  val previousRelease = settingKey[Option[String]]("Determine the artifact of the previous release")
-
-  val CompatReportConfig = config("compatReport").extend(Compile)
-
-  val reportCompat = taskKey[Unit]("Print a compatibility report, e.g., for release notes")
+  val printCompatReport = taskKey[Unit]("Print a compatibility report, e.g., for release notes")
 
   private def markdownTable(headers: String*)(rows: Seq[Seq[String]]) = {
     val escaped = rows.map(_.map(_.replaceAllLiterally("|", "\\|")))
@@ -114,7 +82,10 @@ object Versioning extends AutoPlugin {
         DynVer.getGitDescribeOutput(d)
           .mkVersion(versionFor(versionPolicyIntention.value, _), fallbackVersion(d))
       },
-      previousRelease := {
+    ) ++
+      List(
+        printCompatReport := {
+          val previousRelease = {
             val versions = previousVersionsFromRepo.value
             val cur = Version(Keys.version.value)
             val sorted =
@@ -125,23 +96,36 @@ object Versioning extends AutoPlugin {
                 }
             if (sorted.isEmpty) None else Some(sorted.max.repr)
           }
-    ) ++
-      inConfig(CompatReportConfig)(
-        Defaults.configSettings ++
-          MimaPlugin.projectSettings ++
-          SbtVersionPolicyPlugin.projectSettings ++
-          SbtVersionPolicyMima.projectSettings ++
-          List(
-            versionPolicyPreviousVersions := previousRelease.value.toList,
-            versionPolicyCheckDirection := Direction.both,
-            versionPolicyIntention := BumpMinor,
-            reportCompat := {
-              val dependencyIssues = versionPolicyFindDependencyIssues.value.toMap
-              val mimaIssues = mimaFindBinaryIssues.value.map { case (module, problems) =>
-                module.withName(module.name.stripSuffix("_" + Keys.scalaBinaryVersion.value)) -> problems
+
+          val artifacts =
+            SbtVersionPolicyMima.computePreviousArtifacts(
+              Keys.projectID.value,
+              previousRelease.toList
+            )
+
+          val dependencyIssues =
+            versionPolicyDependencyIssuesReporter.value
+              .apply(
+                BumpMinor,
+                SbtVersionPolicySettings.fromMimaArtifacts(
+                  artifacts
+                )
+              )
+              .toMap
+              .map { case (m, report) => m.toString -> report }
+
+          val artifactsToClassfiles = MimaPlugin.artifactsToClassfiles.value
+          val binaryIssuesFinder = MimaPlugin.binaryIssuesFinder.value
+
+          val mimaIssues =
+            binaryIssuesFinder
+              .iterator(artifactsToClassfiles.apply(artifacts), "both")
+              .toMap
+              .map { case (module, problems) =>
+                module.withName(module.name.stripSuffix("_" + Keys.scalaBinaryVersion.value)).toString -> problems
               }
 
-              for (module <- (dependencyIssues.keySet ++ mimaIssues.keySet).toSeq.sortBy(_.name)) {
+          for (module <- (dependencyIssues.keySet ++ mimaIssues.keySet).toSeq.sorted) {
             val depIssues =
               dependencyIssues.get(module)
                 .map { report =>
@@ -216,6 +200,5 @@ object Versioning extends AutoPlugin {
           }
         }
       )
-      )
   }
 }

while still benefiting from other sbt settings
@dwijnand dwijnand enabled auto-merge April 7, 2022 11:11
@dwijnand dwijnand merged commit 1c60145 into lightbend-labs:main Apr 7, 2022
@dwijnand dwijnand added this to the 1.1.0 milestone Apr 7, 2022
@nafg nafg deleted the choose-mode-on-the-fly branch April 7, 2022 18:14
@nafg
Copy link
Contributor Author

nafg commented Apr 7, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants