define & apply binary compatibility strategy #1565

bjaglin · 2022-03-12T23:03:20Z

In the last few weeks, 2 dependency bumps broke backward compatibility of nightlies

metaconfig 0.10.0, at the source & binary level because of breaking changes in pprint, despite attempts to mitigate the problem
scalameta 4.5.0, at the functional level, by introducing a new node in the AST (detected thanks to the Scala 2 community build)

Since these changes haven't made it to a release yet, that seems like a good moment to either

clarify compatibility guarantees for rule authors ahead of a minor bump for the upcoming release (0.10.0), or
revert the bumps for now and
1. pin dependencies, or
2. contribute upstream to mitigate the impact

This PR takes the path of (1).

bjaglin · 2022-03-12T23:53:29Z

scalafix-cli/src/main/scala/scalafix/internal/interfaces/ScalafixArgumentsImpl.scala

+                s"please upgrade to ${dependency.getVersion} or later"
+            )
+          case Unknown =>
+            args.out.println(


sbt-scalafix sets the printstream very late, so this gets dumped on plain stdout. It would be possible to wire a fallback sbt logger from the beginning.

bjaglin · 2022-03-13T00:04:04Z

I'd really like your view on this @olafurpg (and I cannot assign you), thanks!

julienrf

Thank you @bjaglin for adding this!

I suggest also declaring the versioning scheme in your build definition (on the projects that are published, and for which you do perform binary compatibility checks):

versionScheme := Some("pvp")

That may help build tools (at least sbt) to correctly resolve conflicting versions of scalafix artifacts.

julienrf · 2022-03-16T09:21:02Z

docs/developers/api.md

+[`scalafix-core`](https://index.scala-lang.org/scalacenter/scalafix/scalafix-core) makes the packages
+below available to built-in and external rules.
+
+Its A.B.C scheme follows [PVP](https://pvp.haskell.org/) for syntactic and semantic binary compatiblity


I find the wording a bit confusing. It seems that you talk about “semantic binary compatibility” and “syntactic binary compatibility”. Although I guess you really talk about the binary compatibility of the API for syntactic and semantic rules?

Suggested change

Its A.B.C scheme follows [PVP](https://pvp.haskell.org/) for syntactic and semantic binary compatiblity

Its A.B.C version scheme follows [PVP](https://pvp.haskell.org/) for binary compatiblity

I agree this was confusing, particularly in the context of scalafix where we have syntactic and semantic rules - I'll amend with your wording.

For the record, what I meant was that a major version will be bumped even though no linkage error may happen, but just because the behavior/semantics changed. Binary compatibility, as described in https://docs.scala-lang.org/overviews/core/binary-compatibility-for-library-authors.html for example, seems to focus on link guarantees, so I wanted to highlight the silent functional changes as a factor.

a major version will be bumped even though no linkage error may happen, but just because the behavior/semantics changed

In such a case, would it be possible to not change the semantics of the existing methods but to add new methods instead?

That's what we aim for, but we cannot control dependencies that are part of the API: scalameta/scalameta#2601 is what resulted in a (silent) breaking change, identified by the community build.

docs/developers/api.md

bjaglin · 2022-03-19T22:23:15Z

project/ScalafixBuild.scala

@@ -179,6 +180,11 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
  private val PreviousScalaVersion: Map[String, String] = Map(
  )

+  override def buildSettings: Seq[Setting[_]] = List(
+    versionScheme := Some("pvp"),
+    versionPolicyIntention := Compatibility.None // TODO: harden after 0.10.0


I verified that with Compatibility.BinaryCompatible, we get

[error] Incompatibilities with dependencies of scalafix-core:0.9.34 [error] com.geirsson:metaconfig-core_2.13: incompatible version change from 0.9.15 to 0.10.0 (compatibility: package versioning policy) [error] com.geirsson:metaconfig-typesafe-config_2.13: incompatible version change from 0.9.15 to 0.10.0 (compatibility: package versioning policy) [error] com.lihaoyi:fansi_2.13: incompatible version change from 0.2.14 to 0.3.0 (compatibility: package versioning policy) [error] com.lihaoyi:pprint_2.13: missing dependency [error] org.scalameta:common_2.13: incompatible version change from 4.4.32 to 4.5.0 (compatibility: package versioning policy) [error] org.scalameta:parsers_2.13: incompatible version change from 4.4.32 to 4.5.0 (compatibility: package versioning policy) [error] org.scalameta:scalameta_2.13: incompatible version change from 4.4.32 to 4.5.0 (compatibility: package versioning policy) [error] org.scalameta:trees_2.13: incompatible version change from 4.4.32 to 4.5.0 (compatibility: package versioning policy) [error] Dependencies of module scalafix-core:0.9.34+56-5a4c18d3+20220319-2243-SNAPSHOT break the intended compatibility guarantees 'Compatibility.BinaryCompatible' (see messages above). You have to relax your compatibility intention by changing the value of versionPolicyIntention.

opened https://github.com/scalacenter/scalafix/pull/1568/commits as draft for a reminder

You probably also want to add:

versionPolicyIgnoredInternalDependencyVersions := Some("^\\d+\\.\\d+\\.\\d+\\+\\d+".r)

See https://github.com/scalacenter/sbt-version-policy#how-to-integrate-with-sbt-dynver

bjaglin · 2022-03-19T22:24:30Z

.github/workflows/release.yml

@@ -11,6 +11,9 @@ jobs:
      - uses: olafurpg/setup-scala@v13
      - uses: olafurpg/setup-gpg@v3
      - run: git fetch --unshallow
+      - name: Check that major was bumped if bin compat was broken
+        if: startsWith(github.ref, 'refs/tags/v')


don't block publishing of snapshots 🤞

bjaglin · 2022-03-19T22:25:55Z

@julienrf thanks for your feedback. I have just added sbt-version-policy in a second commit, your review is very much welcome on that one too.

julienrf · 2022-03-21T09:47:57Z

Unfortunately, sbt-version-policy expects early-semver instead of pvp as a versioning scheme. It’s behavior is undefined for pvp, so it’s better to not use it in that case (or consider using early-semver instead of pvp).

bjaglin · 2022-03-21T12:35:43Z

Unfortunately, sbt-version-policy expects early-semver instead of pvp as a versioning scheme. It’s behavior is undefined for pvp, so it’s better to not use it in that case

I had a quick look at the code, and it looks like the early-semver assumption is only in versionCheck, is that right? versionPolicyCheck seems to be fine. Following up in scalacenter/sbt-version-policy#123.

or consider using early-semver instead of pvp

I guess I could also go for that if the fix above is not trivial. I had picked pvp over early-semver mostly of out laziness when (re)implementing bjaglin@5a4c18d#diff-d96ec3acf34c2b31f8eefeb8bdd342bb1365fb47091c75b69298b7b49c928aafR14 (since I couldn't find a versioning lib cross-published for 2.11).

julienrf · 2022-03-21T12:42:47Z

it looks like the early-semver assumption is only in versionCheck, is that right? versionPolicyCheck seems to be fine

That might be the case, but using sbt-version-policy with anything else than early-semver is undertested. I recommend using early-semver, but I am willing to re-consider supporting other versioning schemes in sbt-version-policy (since you are not the only one asking for it).

bjaglin · 2022-03-21T22:07:48Z

@julienrf I amended to advertize/honor early-semver. One argument to stay on PVP would be that scalameta (the main dependency) is effectively on PVP and gets updated quite often. After scalafix reaches 1.0, each time scalameta will get bumped, we'll have to manually assess if it should result in a minor or patch release for scalafix-core. But I guess it will be more meaningful for Scalafix users?

julienrf · 2022-03-22T07:16:47Z

project/ScalafixBuild.scala

@@ -179,6 +180,11 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
  private val PreviousScalaVersion: Map[String, String] = Map(
  )

+  override def buildSettings: Seq[Setting[_]] = List(
+    versionScheme := Some("pvp"),
+    versionPolicyIntention := Compatibility.None // TODO: harden after 0.10.0


You probably also want to add:

versionPolicyIgnoredInternalDependencyVersions := Some("^\\d+\\.\\d+\\.\\d+\\+\\d+".r)

See https://github.com/scalacenter/sbt-version-policy#how-to-integrate-with-sbt-dynver

julienrf · 2022-03-22T07:27:30Z

One argument to stay on PVP would be that scalameta (the main dependency) is effectively on PVP and gets updated quite often. After scalafix reaches 1.0, each time scalameta will get bumped, we'll have to manually assess if it should result in a minor or patch release for scalafix-core.

I guess the defensive strategy would be to always bump the minor version number of scalafix. I don’t know if this is much an issue.

Also, why do you say that scalameta is on PVP? According to https://github.com/scalameta/scalameta/blob/main/COMPATIBILITY.md it seems to use semantic versioning.

bjaglin · 2022-03-22T08:05:34Z

I guess the defensive strategy would be to always bump the minor version number of scalafix. I don’t know if this is much an issue.

Indeed, my only concern was that there is no tooling to enforce that (since sbt-version-policy's versionPolicyReportDependencyIssues won't help if I understand correctly)

Also, why do you say that scalameta is on PVP? According to https://github.com/scalameta/scalameta/blob/main/COMPATIBILITY.md it seems to use semantic versioning.

Thanks for the pointer, I hadn't seen this. x.y.z1 must be compatible with x.y.z2 was not strictly honored as a lot of stuff got added in 4.4.x, but https://github.com/scalameta/scalameta/pull/2504/files explains the rationale. I'll follow-up with a PR to define versionScheme there.

Thanks for your insights!

julienrf · 2022-03-22T08:17:29Z

Indeed, my only concern was that there is no tooling to enforce that (since sbt-version-policy's versionPolicyReportDependencyIssues won't help if I understand correctly)

Now that you say it, versionPolicyReportDependencyIssues should fail when you bump scalameta, if versionPolicyIntention is set to BinaryAndSourceCompatible and libraryDependencySchemes contains "org.scalameta" %% "scalameta" % "pvp". If this is not the case, please report a bug in sbt-version-policy.

bjaglin force-pushed the compat branch from 81158f3 to 5ff6959 Compare March 12, 2022 23:38

bjaglin commented Mar 12, 2022

View reviewed changes

bjaglin force-pushed the compat branch from 5ff6959 to 9c8e356 Compare March 13, 2022 00:00

bjaglin requested a review from mlachkar March 13, 2022 00:03

bjaglin marked this pull request as ready for review March 13, 2022 00:03

mlachkar requested a review from julienrf March 16, 2022 08:32

julienrf reviewed Mar 16, 2022

View reviewed changes

bjaglin force-pushed the compat branch from 9c8e356 to 5a4c18d Compare March 19, 2022 21:17

bjaglin changed the title ~~define & document binary compatibility strategy~~ define & apply binary compatibility strategy Mar 19, 2022

bjaglin force-pushed the compat branch from 9ae86ba to 02acf6b Compare March 19, 2022 22:23

bjaglin commented Mar 19, 2022

View reviewed changes

bjaglin requested a review from julienrf March 19, 2022 22:32

bjaglin force-pushed the compat branch from 02acf6b to 0d418d6 Compare March 21, 2022 22:01

julienrf approved these changes Mar 22, 2022

View reviewed changes

bjaglin force-pushed the compat branch 2 times, most recently from 4a476c6 to 06866f1 Compare March 22, 2022 07:54

bjaglin added 2 commits March 22, 2022 22:29

define & apply binary compatibility strategy

c4c1f1e

fail when deps advertize breaking changes

9155107

bjaglin force-pushed the compat branch from 06866f1 to 9155107 Compare March 22, 2022 21:29

bjaglin mentioned this pull request Mar 22, 2022

remove deprecated keys scalacenter/sbt-scalafix#289

Merged

bjaglin merged commit 8330abd into scalacenter:main Mar 22, 2022

bjaglin mentioned this pull request Mar 22, 2022

support sbt 1.3.x and later only scalacenter/sbt-scalafix#291

Merged

bjaglin mentioned this pull request Apr 4, 2022

only warn when loading rules built with newer core #1588

Merged

bjaglin mentioned this pull request Sep 20, 2022

Fix binary compatibility check when bumping scala version #1673

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

define & apply binary compatibility strategy #1565

define & apply binary compatibility strategy #1565

bjaglin commented Mar 12, 2022 •

edited

Loading

bjaglin Mar 12, 2022

bjaglin commented Mar 13, 2022

julienrf left a comment •

edited

Loading

julienrf Mar 16, 2022

bjaglin Mar 19, 2022

julienrf Mar 21, 2022

bjaglin Mar 21, 2022 •

edited

Loading

bjaglin Mar 19, 2022

bjaglin Mar 19, 2022 •

edited

Loading

julienrf Mar 22, 2022

bjaglin Mar 19, 2022 •

edited

Loading

bjaglin commented Mar 19, 2022

julienrf commented Mar 21, 2022

bjaglin commented Mar 21, 2022

julienrf commented Mar 21, 2022

bjaglin commented Mar 21, 2022 •

edited

Loading

julienrf Mar 22, 2022

julienrf commented Mar 22, 2022

bjaglin commented Mar 22, 2022 •

edited

Loading

julienrf commented Mar 22, 2022

	Its A.B.C scheme follows [PVP](https://pvp.haskell.org/) for syntactic and semantic binary compatiblity
	Its A.B.C version scheme follows [PVP](https://pvp.haskell.org/) for binary compatiblity

define & apply binary compatibility strategy #1565

define & apply binary compatibility strategy #1565

Conversation

bjaglin commented Mar 12, 2022 • edited Loading

bjaglin Mar 12, 2022

Choose a reason for hiding this comment

bjaglin commented Mar 13, 2022

julienrf left a comment • edited Loading

Choose a reason for hiding this comment

julienrf Mar 16, 2022

Choose a reason for hiding this comment

bjaglin Mar 19, 2022

Choose a reason for hiding this comment

julienrf Mar 21, 2022

Choose a reason for hiding this comment

bjaglin Mar 21, 2022 • edited Loading

Choose a reason for hiding this comment

bjaglin Mar 19, 2022

Choose a reason for hiding this comment

bjaglin Mar 19, 2022 • edited Loading

Choose a reason for hiding this comment

julienrf Mar 22, 2022

Choose a reason for hiding this comment

bjaglin Mar 19, 2022 • edited Loading

Choose a reason for hiding this comment

bjaglin commented Mar 19, 2022

julienrf commented Mar 21, 2022

bjaglin commented Mar 21, 2022

julienrf commented Mar 21, 2022

bjaglin commented Mar 21, 2022 • edited Loading

julienrf Mar 22, 2022

Choose a reason for hiding this comment

julienrf commented Mar 22, 2022

bjaglin commented Mar 22, 2022 • edited Loading

julienrf commented Mar 22, 2022

bjaglin commented Mar 12, 2022 •

edited

Loading

julienrf left a comment •

edited

Loading

bjaglin Mar 21, 2022 •

edited

Loading

bjaglin Mar 19, 2022 •

edited

Loading

bjaglin Mar 19, 2022 •

edited

Loading

bjaglin commented Mar 21, 2022 •

edited

Loading

bjaglin commented Mar 22, 2022 •

edited

Loading