Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
define & apply binary compatibility strategy
- Loading branch information
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,10 @@ import scalafix.Versions | |
| import scalafix.cli.ExitStatus | ||
| import scalafix.interfaces._ | ||
| import scalafix.internal.config.ScalaVersion | ||
| import scalafix.internal.util.Compatibility | ||
| import scalafix.internal.util.Compatibility.Compatible | ||
| import scalafix.internal.util.Compatibility.Incompatible | ||
| import scalafix.internal.util.Compatibility.Unknown | ||
| import scalafix.internal.v1.Args | ||
| import scalafix.internal.v1.MainOps | ||
| import scalafix.internal.v1.Rules | ||
|
|
@@ -76,7 +80,51 @@ final case class ScalafixArgumentsImpl(args: Args = Args.default) | |
| customDependenciesCoordinates, | ||
| Versions.scalaVersion | ||
| ) | ||
|
|
||
| // External rules are built against `scalafix-core` to expose `scalafix.v1.Rule` implementations. The | ||
| // classloader loading `scalafix-cli` already contains `scalafix-core` to be able to discover them (which | ||
| // is why it must be the parent of the one loading the tool classpath), so effectively, the version/instance | ||
| // in the tool classpath will not be used. This adds a sanity check to warn or prevent the user in case of | ||
| // mismatch. | ||
| val scalafixCore = coursierapi.Module.parse( | ||
| "ch.epfl.scala::scalafix-core", | ||
| coursierapi.ScalaVersion.of(Versions.scalaVersion) | ||
| ) | ||
| customDependenciesJARs.getDependencies.asScala | ||
| .find(_.getModule == scalafixCore) | ||
| .foreach { dependency => | ||
| // We only check compatibility against THE scalafix-core returned by the coursier resolution, but | ||
| // this is not exhaustive as some old rules might coexist with recent ones, so the older versions | ||
| // get evicted. coursier-interface does not provide more granularity while the native coursier | ||
| // is stuck on a 2-years old version because it no longer cross-publishes for 2.11, so for now, | ||
| // the easiest option would be to run several resolutions in isolation, which seems like a massive | ||
| // cost for just issuing a warning. | ||
| Compatibility.earlySemver( | ||
| dependency.getVersion, | ||
| Versions.nightly | ||
| ) match { | ||
| case Incompatible => | ||
| throw new ScalafixException( | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong. 
mlachkar
Collaborator
|
||
| s"Scalafix version ${Versions.nightly} cannot load the registered external rules, " + | ||
| s"please upgrade to ${dependency.getVersion} or later" | ||
| ) | ||
| case Unknown => | ||
| args.out.println( | ||
| s"""INFO: loading external rule(s) built against an old version of scalafix (${dependency.getVersion}). | ||
| |This might not be a problem, but if you run into unexpected behavior, you should either: | ||
| |1. downgrade scalafix to ${dependency.getVersion} | ||
| |2. try a more recent version of the rules(s) if available; request the rule maintainer | ||
| | to build against scalafix ${Versions.nightly} or later if that does not help | ||
| """.stripMargin | ||
| ) | ||
| case Compatible => | ||
| } | ||
| } | ||
|
|
||
| val extraURLs = customURLs.asScala ++ customDependenciesJARs | ||
| .getFiles() | ||
| .asScala | ||
| .map(_.toURI().toURL()) | ||
| val classLoader = new URLClassLoader( | ||
| extraURLs.toArray, | ||
| getClass.getClassLoader | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package scalafix.internal.util | ||
|
|
||
| sealed trait Compatibility | ||
|
|
||
| object Compatibility { | ||
|
|
||
| case object Compatible extends Compatibility | ||
| case object Unknown extends Compatibility | ||
| case object Incompatible extends Compatibility | ||
|
|
||
| private val Snapshot = """.*-SNAPSHOT""".r | ||
| private val XYZ = """^([0-9]+)\.([0-9]+)\.([0-9]+)""".r | ||
|
|
||
| def earlySemver(builtAgainst: String, runWith: String): Compatibility = { | ||
| (builtAgainst, runWith) match { | ||
| case (Snapshot(), _) => Unknown | ||
| case (_, Snapshot()) => Unknown | ||
| case (XYZ(bX, _, _), XYZ(rX, _, _)) if bX.toInt > rX.toInt => | ||
| Incompatible | ||
| case (XYZ(bX, _, _), XYZ(rX, _, _)) if bX.toInt < rX.toInt => | ||
| Unknown | ||
| // --- X match given the cases above | ||
| case (XYZ(_, bY, _), XYZ(_, rY, _)) if bY.toInt > rY.toInt => | ||
| Incompatible | ||
| case (XYZ("0", bY, _), XYZ("0", rY, _)) if bY.toInt < rY.toInt => | ||
| Unknown | ||
| case (XYZ(_, bY, _), XYZ(_, rY, _)) if bY.toInt < rY.toInt => | ||
| Compatible | ||
| // --- X.Y match given the cases above | ||
| case (XYZ("0", _, bZ), XYZ("0", _, rZ)) if bZ.toInt > rZ.toInt => | ||
| Incompatible | ||
| case _ => Compatible | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package scalafix.tests.cli | ||
|
|
||
| import org.scalatest.funsuite.AnyFunSuite | ||
| import scalafix.internal.util.Compatibility | ||
| import scalafix.internal.util.Compatibility._ | ||
|
|
||
| class CompatibilitySuite extends AnyFunSuite { | ||
|
|
||
| // to avoid struggles when testing nightlies | ||
| test("EarlySemver unknown if run or build is a snapshot") { | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.9.34+52-a83785c4-SNAPSHOT", | ||
| runWith = "1.2.3" | ||
| ) == Unknown | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.9.34", | ||
| runWith = "1.2.3+1-bfe5ccd4-SNAPSHOT" | ||
| ) == Unknown | ||
| ) | ||
| } | ||
|
|
||
| // backward compatibility within X.*.*, 0.Y.*, ... | ||
| test( | ||
| "EarlySemver compatible if run is equal or greater by minor (or patch in 0.)" | ||
| ) { | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "1.3.27", | ||
| runWith = "1.3.28" | ||
| ) == Compatible | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "1.10.20", | ||
| runWith = "1.12.0" | ||
| ) == Compatible | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.6.12", | ||
| runWith = "0.6.12" | ||
| ) == Compatible | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.9.0", | ||
| runWith = "0.9.20" | ||
| ) == Compatible | ||
| ) | ||
| } | ||
|
|
||
| // no forward compatibility: build might reference classes unknown to run | ||
| test("EarlySemver incompatible if run is lower by minor (or patch in 0.)") { | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.10.8", | ||
| runWith = "0.9.16" | ||
| ) == Incompatible | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.10.17", | ||
| runWith = "0.10.4" | ||
| ) == Incompatible | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "2.0.0", | ||
| runWith = "1.1.1" | ||
| ) == Incompatible | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "1.4.7", | ||
| runWith = "1.2.8" | ||
| ) == Incompatible | ||
| ) | ||
| } | ||
|
|
||
| // might be false positive/negative tree matches or link failures | ||
| test("EarlySemver unknown if run is greater by major (or minor in 0.)") { | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "1.0.41", | ||
| runWith = "2.0.0" | ||
| ) == Unknown | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.9.38", | ||
| runWith = "0.10.2" | ||
| ) == Unknown | ||
| ) | ||
| assert( | ||
| Compatibility.earlySemver( | ||
| builtAgainst = "0.9.38", | ||
| runWith = "1.0.0" | ||
| ) == Unknown | ||
| ) | ||
| } | ||
| } |
Even though this is the correct thing to do since we can't guarantee forward binary-compatibility in scalafix-core, ahead of releasing 0.10.0-RC1 I am still concerned that this will unnecessarily punish clients that are not frequently updated or easily upgradable. I thought about other CLI clients (mill, maven, etc), but what about Metals for example? Any thought @tgodzik @mlachkar?