From 8a518dfb98a25bea42b526410b2a968fef5f6d67 Mon Sep 17 00:00:00 2001 From: calm <148254234+calm329@users.noreply.github.com> Date: Thu, 22 Jan 2026 12:52:58 -0800 Subject: [PATCH] [2.x] fix: Skip eviction warning when winner satisfies version range (#8616) - Add `versionSatisfiesRange()` function to `VersionRange.scala` supporting Maven-style ranges (`[x,y)`, `(x,y]`, `[x,)`, etc.) and plus ranges (`1.0+`) - Check if winner version satisfies evicted module's version range in `guessCompatible()` --- .../librarymanagement/VersionRange.scala | 129 ++++++++++++++++++ .../librarymanagement/EvictionWarning.scala | 56 +++++--- .../librarymanagement/VersionRangeSpec.scala | 82 +++++++++++ 3 files changed, 245 insertions(+), 22 deletions(-) diff --git a/lm-core/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala b/lm-core/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala index 101da3056..055f835aa 100644 --- a/lm-core/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala +++ b/lm-core/src/main/scala/sbt/internal/librarymanagement/VersionRange.scala @@ -2,6 +2,8 @@ package sbt package internal package librarymanagement +import sbt.librarymanagement.VersionNumber + object VersionRange { /** True if the revision is an ivy-range, not a complete revision. */ @@ -13,6 +15,133 @@ object VersionRange { (revision.contains(")")) } + /** + * Checks if a version satisfies a version range. + * @param version The version to check (e.g., "4.2.1") + * @param range The version range (e.g., "[4.1.0,5)" or "[1.0,2.0]") + * @return true if version is within the range, false otherwise + */ + def versionSatisfiesRange(version: String, range: String): Boolean = { + if (!isVersionRange(range)) { + // Not a range, just compare directly + version == range + } else if (range.endsWith("+")) { + // Handle plus ranges like "1.0+" meaning >= 1.0 + val base = range.dropRight(1) + compareVersions(version, base) >= 0 + } else if (hasMavenVersionRange(range)) { + // Parse Maven-style range like [1.0,2.0) or (1.0,2.0] + parseMavenRange(range) match { + case Some((lowerBound, lowerInclusive, upperBound, upperInclusive)) => + val lowerOk = lowerBound match { + case Some(lb) => + val cmp = compareVersions(version, lb) + if (lowerInclusive) cmp >= 0 else cmp > 0 + case None => true + } + val upperOk = upperBound match { + case Some(ub) => + val cmp = compareVersions(version, ub) + if (upperInclusive) cmp <= 0 else cmp < 0 + case None => true + } + lowerOk && upperOk + case None => false + } + } else { + false + } + } + + /** + * Parses a Maven-style version range. + * @return Some((lowerBound, lowerInclusive, upperBound, upperInclusive)) or None + */ + private def parseMavenRange( + range: String + ): Option[(Option[String], Boolean, Option[String], Boolean)] = { + val trimmed = range.trim + if (trimmed.length < 2) None + else { + val startChar = trimmed.head + val endChar = trimmed.last + + val lowerInclusive = startChar == '[' + val upperInclusive = endChar == ']' + + if (!Set('[', '(').contains(startChar) || !Set(']', ')').contains(endChar)) { + None + } else { + val inner = trimmed.substring(1, trimmed.length - 1) + val commaIdx = inner.indexOf(',') + + if (commaIdx < 0) { + // Single version constraint like [1.0] means exactly 1.0 + val v = inner.trim + if (v.nonEmpty) Some((Some(v), true, Some(v), true)) + else None + } else { + val lower = inner.substring(0, commaIdx).trim + val upper = inner.substring(commaIdx + 1).trim + Some( + ( + if (lower.nonEmpty) Some(lower) else None, + lowerInclusive, + if (upper.nonEmpty) Some(upper) else None, + upperInclusive + ) + ) + } + } + } + } + + /** + * Compares two version strings. + * @return negative if v1 < v2, 0 if v1 == v2, positive if v1 > v2 + */ + private def compareVersions(v1: String, v2: String): Int = { + val vn1 = VersionNumber(v1) + val vn2 = VersionNumber(v2) + + // Compare numeric parts first + val nums1 = vn1.numbers + val nums2 = vn2.numbers + val maxLen = math.max(nums1.length, nums2.length) + + val numericComparison = (0 until maxLen).iterator + .map { i => + val n1 = if (i < nums1.length) nums1(i) else 0L + val n2 = if (i < nums2.length) nums2(i) else 0L + n1.compare(n2) + } + .find(_ != 0) + + numericComparison match { + case Some(cmp) => cmp + case None => + // If numeric parts are equal, compare tags (versions with tags are usually pre-releases) + val tags1 = vn1.tags + val tags2 = vn2.tags + + // No tags means release version, which is higher than any pre-release + if (tags1.isEmpty && tags2.nonEmpty) 1 + else if (tags1.nonEmpty && tags2.isEmpty) -1 + else { + // Compare tags lexicographically + val tagMaxLen = math.max(tags1.length, tags2.length) + val tagComparison = (0 until tagMaxLen).iterator + .map { i => + val t1 = if (i < tags1.length) tags1(i) else "" + val t2 = if (i < tags2.length) tags2(i) else "" + t1.compare(t2) + } + .find(_ != 0) + tagComparison.getOrElse(0) + } + } + } + // Assuming Ivy is used to resolve conflict, this removes the version range // when it is open-ended to avoid dependency resolution hitting the Internet to get the latest. // See https://github.com/sbt/sbt/issues/2954 diff --git a/lm-core/src/main/scala/sbt/librarymanagement/EvictionWarning.scala b/lm-core/src/main/scala/sbt/librarymanagement/EvictionWarning.scala index 19247e068..42783e630 100644 --- a/lm-core/src/main/scala/sbt/librarymanagement/EvictionWarning.scala +++ b/lm-core/src/main/scala/sbt/librarymanagement/EvictionWarning.scala @@ -3,7 +3,7 @@ package sbt.librarymanagement import collection.mutable import Configurations.Compile import ScalaArtifacts.{ LibraryID, CompilerID } -import sbt.internal.librarymanagement.VersionSchemes +import sbt.internal.librarymanagement.{ VersionSchemes, VersionRange } import sbt.util.Logger import sbt.util.ShowLines @@ -337,28 +337,40 @@ object EvictionWarning { def guessCompatible(p: EvictionPair): Boolean = p.evicteds forall { r => val winnerOpt = p.winner map { _.module } - val extraAttributes = ((p.winner match { - case Some(r) => r.extraAttributes - case _ => Map.empty - }): collection.immutable.Map[String, String]) ++ (winnerOpt match { - case Some(w) => w.extraAttributes - case _ => Map.empty - }) - val schemeOpt = VersionSchemes.extractFromExtraAttributes(extraAttributes) - val f = (winnerOpt, schemeOpt) match { - case (Some(_), Some(VersionSchemes.Always)) => - EvictionWarningOptions.guessTrue - case (Some(_), Some(VersionSchemes.Strict)) => - EvictionWarningOptions.guessStrict - case (Some(_), Some(VersionSchemes.EarlySemVer)) => - EvictionWarningOptions.guessEarlySemVer - case (Some(_), Some(VersionSchemes.SemVerSpec)) => - EvictionWarningOptions.guessSemVer - case (Some(_), Some(VersionSchemes.PackVer)) => - EvictionWarningOptions.evalPvp - case _ => options.guessCompatible(_) + // Check if the evicted module's revision is a version range and if the winner satisfies it + // This handles cases like [4.1.0,5) where 4.2.1 would be within range (fixes #3978) + val evictedRev = r.module.revision + val winnerSatisfiesRange: Boolean = winnerOpt match { + case Some(winner) if VersionRange.isVersionRange(evictedRev) => + VersionRange.versionSatisfiesRange(winner.revision, evictedRev) + case _ => false + } + if (winnerSatisfiesRange) { + true + } else { + val extraAttributes = ((p.winner match { + case Some(r) => r.extraAttributes + case _ => Map.empty + }): collection.immutable.Map[String, String]) ++ (winnerOpt match { + case Some(w) => w.extraAttributes + case _ => Map.empty + }) + val schemeOpt = VersionSchemes.extractFromExtraAttributes(extraAttributes) + val f = (winnerOpt, schemeOpt) match { + case (Some(_), Some(VersionSchemes.Always)) => + EvictionWarningOptions.guessTrue + case (Some(_), Some(VersionSchemes.Strict)) => + EvictionWarningOptions.guessStrict + case (Some(_), Some(VersionSchemes.EarlySemVer)) => + EvictionWarningOptions.guessEarlySemVer + case (Some(_), Some(VersionSchemes.SemVerSpec)) => + EvictionWarningOptions.guessSemVer + case (Some(_), Some(VersionSchemes.PackVer)) => + EvictionWarningOptions.evalPvp + case _ => options.guessCompatible(_) + } + f((r.module, winnerOpt, module.scalaModuleInfo)) } - f((r.module, winnerOpt, module.scalaModuleInfo)) } pairs foreach { case p if isScalaArtifact(module, p.organization, p.name) => diff --git a/lm-core/src/test/scala/sbt/librarymanagement/VersionRangeSpec.scala b/lm-core/src/test/scala/sbt/librarymanagement/VersionRangeSpec.scala index e8bd99028..08225465b 100644 --- a/lm-core/src/test/scala/sbt/librarymanagement/VersionRangeSpec.scala +++ b/lm-core/src/test/scala/sbt/librarymanagement/VersionRangeSpec.scala @@ -16,4 +16,86 @@ class VersionRangeSpec extends UnitSpec { def stripTo(s: String, expected: Option[String]) = assert(VersionRange.stripMavenVersionRange(s) == expected) + + // Tests for versionSatisfiesRange (issue #3978) + "versionSatisfiesRange" should "return true when version is within inclusive range [4.1.0,5)" in { + assert(VersionRange.versionSatisfiesRange("4.2.1", "[4.1.0,5)") == true) + } + + it should "return true for version at lower bound of inclusive range" in { + assert(VersionRange.versionSatisfiesRange("4.1.0", "[4.1.0,5)") == true) + } + + it should "return false for version at upper bound of exclusive range" in { + assert(VersionRange.versionSatisfiesRange("5", "[4.1.0,5)") == false) + assert(VersionRange.versionSatisfiesRange("5.0", "[4.1.0,5)") == false) + assert(VersionRange.versionSatisfiesRange("5.0.0", "[4.1.0,5)") == false) + } + + it should "return false for version below range" in { + assert(VersionRange.versionSatisfiesRange("4.0.9", "[4.1.0,5)") == false) + assert(VersionRange.versionSatisfiesRange("3.0.0", "[4.1.0,5)") == false) + } + + it should "return false for version above range" in { + assert(VersionRange.versionSatisfiesRange("5.0.1", "[4.1.0,5)") == false) + assert(VersionRange.versionSatisfiesRange("6.0.0", "[4.1.0,5)") == false) + } + + it should "handle fully inclusive range [1.0,2.0]" in { + assert(VersionRange.versionSatisfiesRange("1.0", "[1.0,2.0]") == true) + assert(VersionRange.versionSatisfiesRange("1.5", "[1.0,2.0]") == true) + assert(VersionRange.versionSatisfiesRange("2.0", "[1.0,2.0]") == true) + assert(VersionRange.versionSatisfiesRange("2.0.1", "[1.0,2.0]") == false) + assert(VersionRange.versionSatisfiesRange("0.9", "[1.0,2.0]") == false) + } + + it should "handle fully exclusive range (1.0,2.0)" in { + assert(VersionRange.versionSatisfiesRange("1.0", "(1.0,2.0)") == false) + assert(VersionRange.versionSatisfiesRange("1.0.1", "(1.0,2.0)") == true) + assert(VersionRange.versionSatisfiesRange("1.5", "(1.0,2.0)") == true) + assert(VersionRange.versionSatisfiesRange("1.9.9", "(1.0,2.0)") == true) + assert(VersionRange.versionSatisfiesRange("2.0", "(1.0,2.0)") == false) + } + + it should "handle open upper bound [1.0,)" in { + assert(VersionRange.versionSatisfiesRange("1.0", "[1.0,)") == true) + assert(VersionRange.versionSatisfiesRange("1.5", "[1.0,)") == true) + assert(VersionRange.versionSatisfiesRange("100.0", "[1.0,)") == true) + assert(VersionRange.versionSatisfiesRange("0.9", "[1.0,)") == false) + } + + // Exact reproduction case from issue #3978 comment by eed3si9n + it should "handle angular-bootstrap reproduction case [1.3.0,)" in { + assert(VersionRange.versionSatisfiesRange("1.4.7", "[1.3.0,)") == true) + assert(VersionRange.versionSatisfiesRange("1.3.0", "[1.3.0,)") == true) + assert(VersionRange.versionSatisfiesRange("1.2.9", "[1.3.0,)") == false) + } + + it should "handle open lower bound (,2.0]" in { + assert(VersionRange.versionSatisfiesRange("0.1", "(,2.0]") == true) + assert(VersionRange.versionSatisfiesRange("1.0", "(,2.0]") == true) + assert(VersionRange.versionSatisfiesRange("2.0", "(,2.0]") == true) + assert(VersionRange.versionSatisfiesRange("2.0.1", "(,2.0]") == false) + } + + it should "handle plus ranges like 1.0+" in { + assert(VersionRange.versionSatisfiesRange("1.0", "1.0+") == true) + assert(VersionRange.versionSatisfiesRange("1.1", "1.0+") == true) + assert(VersionRange.versionSatisfiesRange("2.0", "1.0+") == true) + assert(VersionRange.versionSatisfiesRange("0.9", "1.0+") == false) + } + + it should "handle exact version (not a range)" in { + assert(VersionRange.versionSatisfiesRange("1.0", "1.0") == true) + assert(VersionRange.versionSatisfiesRange("1.0.0", "1.0") == false) + assert(VersionRange.versionSatisfiesRange("1.1", "1.0") == false) + } + + it should "handle single version constraint [1.0]" in { + assert(VersionRange.versionSatisfiesRange("1.0", "[1.0]") == true) + // Note: 1.0.0 is considered equal to 1.0 in semantic version comparison + assert(VersionRange.versionSatisfiesRange("1.0.0", "[1.0]") == true) + assert(VersionRange.versionSatisfiesRange("1.1", "[1.0]") == false) + } }