typelevel / sbt-typelevel

Let sbt work for you.
https://typelevel.org/sbt-typelevel/
Apache License 2.0
163 stars 44 forks source link

Defaults for Xsource are not binary compatible between 2.13.12 and 2.13.14 #717

Open jenshalm opened 1 week ago

jenshalm commented 1 week ago

The changes associated with this flag have the unfortunate effect that for some projects (like Laika) both sticking with the old Xsource:3 or switching to the new Xsource:3-cross can lead to breaking bincompat which will be flagged my mima. The reason is that the old flag does less now than it used to do in 2.13.12 while the new flag does more and the respective differences in feature support are both bincompat-breaking for any source that contains the affected features. The solution for Laika was to use the new flag -Xsource-features to selectively enable those features from Xsource:3-cross which used to live under Xsource:3 in 2.13.12 (see https://github.com/typelevel/Laika/commit/0d88951cd10d6b777c4f055f017d942d8a27f09e#diff-5634c415cd8c8504fdb973a3ed092300b43c4b8fc1e184f7249eb29a55511f91R54).

Now given that sbt-tl sets Xsource:3 by default it effectively means that it sets a default that is potentially bin-compat breaking for projects going from 2.13.12 to 2.13.14.

The behaviour of the Xsource flag is described in detail in this page: https://docs.scala-lang.org/scala3/guides/migration/tooling-scala2-xsource3.html - however, it unfortunately lacks a migration guide for projects which used Xsource:3 in 2.13.12 or earlier. It explicitly lists 3 features affecting bincompat: case-apply-copy-access, case-companion-function, infer-override. For Laika I validated that infer-override needs to be on, while case-companion-function needs to be off, which can be accomplished by keeping Xsource:3 (which does not contain any of those 3 features) and additionally set -Xsource-features:infer-override to add the removed feature back in. I am not sure about the 3rd feature which would need to be investigated (case-apply-copy-access).

The risk for projects using sbt-tl to publish artifacts that are not bincompat is fairly low, as mima seems to reliably flag all those issues. However, it would be more convenient and consistent if the plugin would ensure compatibility under the hood. The ideal solution imho would be:

som-snytt commented 1 week ago

To avoid silent language changes when upgrading to a new Scala 2.13 version, it is recommended to enable features explicitly or use a group (e.g., -Xsource-features:v2.13.14).

I guess without looking that there is no -Xsource-features:v2.13.12.

The mission of -Xsource3 changed from "I'm migrating my code so do everything to get me closer to Scala 3" to "I want to keep cross-compiling as long as possible."

For stability, I think it's reasonable to ask scalac to provide the useful aliases; I don't know how far back.

jenshalm commented 1 week ago

@som-snytt - for me the main issue is mostly lack of a migration guide for existing projects going from 2.13.12 to 2.13.14, everything else is manageable imho.

The last paragraph of this page: https://docs.scala-lang.org/scala3/guides/migration/tooling-scala2-xsource3.html gives instructions for how to avoid binary breaking changes, but the way I understand it this is solely for users who were not using any of the flags in 2.13.12. If you'd also add a paragraph for those who used Xsource:3 in 2.13.12 and the relevant flags those projects need to set for compatibility, then everything would be fine imho.

So, yes, having -Xsource-features:v2.13.12 would be even better (but I haven't actually tried whether it exists), but given that's probably some work to implement if not (and lead to users expecting this to be available for all 2.13 versions), featuring compatibility more prominently in the docs and in the release notes might be the quicker and easier fix.