Fix #4513: Set up versionScheme for all the "library" artifacts. #4517

sjrd · 2021-06-22T14:26:31Z

Based on #4516.

gzm0 · 2021-06-25T14:01:38Z

project/Build.scala

@@ -789,7 +818,7 @@ object Build {

  val commonLinkerInterfaceSettings = Def.settings(
      commonSettings,
-      publishSettings,
+      publishSettings(VersionScheme.BreakOnSevere),


I don't think this is correct: The unstable package in this artifact may break on minor versions. One might not consider this part of the "public" API, but it is public in the sense that it needs to match the Linker's API.

I think this one should be fine. We create the dependency on scalajs-linker with a version number that is dynamically looked up from ScalaJSVersions.current. If there is a newer version of linker-interface, then there will be a newer version of scalajs-ir, which will in turn cause the newer version of scalajs-linker to be selected.

We could still end up in a scenario where scalajs-ir is bumped higher than linker-interface. But that is an illegal scenario to begin with, since scalajs-ir breaks on Minor, so having a dependency from linker-interface 1.x to scalajs-ir 1.x evicted in favor of scalajs-ir 1.y is already reported as an issue.

gzm0 · 2021-06-28T16:33:50Z

project/Build.scala

@@ -1037,7 +1066,7 @@ object Build {
      id = "testAdapter", base = file("test-adapter")
  ).settings(
      commonSettings,
-      publishSettings,
+      publishSettings(VersionScheme.BreakOnSevere),


I do not think this is correct either: The testAdapter has a strict version dependency with the testBridge.

Right, so it should be BreakOnMinor.

That means we end up with the same problem as with scalajs-ir, so we have to apply the same workaround as for scalajs-ir.

We could also solve this better by putting a val scalaJSBridgeVersion inside the test-adapter artifact. Then sbt-scalajs would read that version to decide what version of scalajs-test-bridge to depend on. That would solve the issue, I believe.

gzm0 · 2021-06-28T16:36:34Z

project/Build.scala

@@ -741,7 +770,7 @@ object Build {
      id = "compiler", base = file("compiler")
  ).settings(
      commonSettings,
-      publishSettings,
+      publishSettings(VersionScheme.BreakOnPatch),


Why is this BreakOnPatch? Or maybe more specifically, what is the public API of a compiler plugin?

A compiler plugin has no public API. In that sense, we can choose any version scheme. The two possible extremes are:

BreakOnPatch: it signals that, even though some methods are technically public, they can break at any time.

"always" (makes even 1.x and 2.x compatible): since there is officially no public API, there is no public API than can ever break, so all versions are compatible.

Of those two choices, the latter is more theoretically adequate. However, IMO the former has more practical value.

gzm0 · 2021-06-28T16:37:10Z

project/Build.scala

+  object VersionScheme {
+    final val BreakOnPatch = "strict"
+    final val BreakOnMinor = "pvp"
+    final val BreakOnSevere = "semver-spec"


What happens if two artifacts that differ only in version have different values for VersionScheme?

According to sbt/librarymanagement#339, the version scheme of the “winner” artifact (which is always the one with the highest version number?) applies. In short, changing the version scheme of an artifact during its life is not supported.

sbt/sbt#6572 (comment) confirmed the behavior. However, I disagree that the use case is not supported. To me that is exactly the behavior we want: if we change the version scheme later for something stricter, the resolution algorithm will correctly pick the stricter value of versionScheme and report the correct incompatibilities.

What if we have:

1.0.0 semver

1.1.0 pvp

1.2.0 semver

Will this accurately detect a conflict between 1.0.0 / 1.2.0? (Or can we simply never make versioningScheme less strict anymore?)

Or can we simply never make versioningScheme less strict anymore?

I think that's the answer. Also it makes sense to me, independently of the specifics of the resolution algo: you can't become more lenient as time progresses, since you'll still have old releases that were breaking on minor. This is not different than having pvp since v1.0.0, and then changing to semver in v1.2.0. You don't need the double-change to create a problem; just going once in the direction from stricter to more lenient is the problem.

I'm more and more getting the impression, that our definition / plan for severe changes is broken. IMO if that happens, we might be better off decoupling SJS linker and and binary IR version.

if we change the version scheme later for something stricter, the resolution algorithm will correctly pick the stricter value of versionScheme and report the correct incompatibilities.

AFAIU, it works by chance in this situation, but what if you change the scheme later for something less strict?

As I mentioned above: becoming less strict in the future is not valid (for fundamental reasons, not just because of the specific resolution algorithm). But becoming stricter in the future is allowed.

gzm0 · 2021-06-28T16:42:15Z

project/Build.scala

@@ -1627,7 +1656,7 @@ object Build {
      id = "jUnitPlugin", base = file("junit-plugin")
  ).settings(
      commonSettings,
-      publishSettings,
+      publishSettings(VersionScheme.BreakOnPatch),


Same question here RE versioning of compiler plugins.

sjrd · 2021-06-30T09:44:16Z

I posted several clarification questions about the precise behavior of versionScheme at sbt/sbt#6572

sjrd · 2021-07-11T08:40:29Z

project/Build.scala

@@ -705,7 +734,7 @@ object Build {

  val commonIrProjectSettings = Def.settings(
      commonSettings,
-      publishSettings,
+      publishSettings(VersionScheme.BreakOnMinor),


sbt/sbt#6572 revealed that using BreakOnMinor for ir, although technically correct, is going to destroy practical scenarios of dependencies. It won't be possible to resolve a conflict between sbt-scalajs 1.6.x and 1.7.x, although sbt-scalajs has semver-spec, because they depend on scalajs-ir, which is pvp. And that, even though the things that depend on sbt-scalajs do not directly reference scalajs-ir.

There are two workarounds:

Declare semver-spec (BreakOnSevere, like sbt-scalajs): allows correct scenarios, but also silently lets through situations where two dependencies actually directly depend on conflicting versions of scalajs-ir.

Do not set versionScheme at all for scalajs-ir: it allows correct scenarios (but sometimes with a spurious warning), and warns (but does not fail) for actually conflicting scenarios.

I don't know yet which choice is best.

…acts. We do not set the version scheme for "tools" artifacts yet, as the jury is still out on what is the best way to specify them.

sjrd · 2021-07-30T11:36:26Z

I have pushed a restricted version where I only specify versionScheme for the artifacts of the "library ecosystem". This should hopefully be less controversial at this time, and would provide strict progress.

sjrd requested a review from gzm0 June 22, 2021 14:26

sjrd force-pushed the version-scheme branch 2 times, most recently from 8574be9 to 0fada39 Compare June 28, 2021 13:51

gzm0 requested changes Jun 28, 2021

View reviewed changes

sjrd commented Jul 11, 2021

View reviewed changes

sjrd linked an issue Jul 13, 2021 that may be closed by this pull request

Scala.js artifacts don’t declare a versionScheme #4513

Closed

Fix scala-js#4513: Set up versionScheme for all the "library" artif…

8c60492

…acts. We do not set the version scheme for "tools" artifacts yet, as the jury is still out on what is the best way to specify them.

sjrd force-pushed the version-scheme branch from 0fada39 to 8c60492 Compare July 30, 2021 11:35

sjrd requested a review from gzm0 July 30, 2021 11:37

gzm0 changed the title ~~Fix #4513: Set up versionScheme for all the published artifacts.~~ Fix #4513: Set up versionScheme for all the "library" artifacts. Jul 30, 2021

gzm0 approved these changes Jul 30, 2021

View reviewed changes

sjrd merged commit 46be6ba into scala-js:master Jul 30, 2021

sjrd deleted the version-scheme branch July 30, 2021 20:09

This was referenced Nov 9, 2022

Update to Scala Native 0.4 portable-scala/sbt-crossproject#148

Closed

Remove version scheme scala-native/scala-native#2985

Merged

Fix #4513: Set up versionScheme for all the "library" artifacts. #4517

Fix #4513: Set up versionScheme for all the "library" artifacts. #4517

Uh oh!

Conversation

sjrd commented Jun 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjrd Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjrd commented Jun 30, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjrd commented Jul 30, 2021

Uh oh!

Uh oh!

sjrd Jul 11, 2021 •

edited

Loading