Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Scala Native to 0.5.1 #45

Closed
wants to merge 3 commits into from

Conversation

xerial
Copy link

@xerial xerial commented Apr 19, 2024

Upgrade Scala Native to 0.5.1.

  • Upgrade to sbt 1.9.9 to pass compilation
  • [breaking] Dropped Scala 2.11.12, which is no longer supported in Scala Native and Scala.js
  • Upgraded sbt-scalajs-crossproject (-native) to the latest version 1.3.2

Ideally, Scala 3 should be supported. However, since the macro-based JVM code needs to be rewritten for Scala 3, it is not included in this PR.

@xerial
Copy link
Author

xerial commented Apr 20, 2024

CI failures include:

  • MiMA errors due to the missing first release for Scala Native 0.5.x. I think we can ignore these errors.
  • Scala.js 0.6.x support. I also feel it's safe to drop Scala.js 0.6.x support now that 1.x series has been used for years.

@ekrich
Copy link

ekrich commented May 9, 2024

* MiMA errors due to the missing first release for Scala Native 0.5.x. I think we can ignore these errors.

I think you can set a previous release missing somehow so that doesn't error - that might just be sbt-typelevel that has that feature but maybe you could copy.

@xerial
Copy link
Author

xerial commented May 9, 2024

@ekrich Thanks for the hint! Ignoring a specific version worked by setting empty to mimaPreviousArtifacts

@sideeffffect
Copy link
Contributor

Hello @sjrd @gzm0 , could we please get review on this PR? 🙏
It will unblock the rollout of Scala Native 0.5.x 🚀

@gzm0
Copy link
Contributor

gzm0 commented May 17, 2024

Sorry for the delay here: I'm insufficiently familiar with the relevant parts of the ecosystem to be sure to make the right version judgments here.

However: If we drop support for things, we need to bump the major version. Is this necessary to support Scala Native or can we do without? Because in the latter case, I do not think we should do it just because.

@sideeffffect
Copy link
Contributor

sideeffffect commented May 17, 2024

@gzm0 if I understand the PR correctly, we're dropping only support (cross-compilation target) for an old version of Scala (2.11). That shouldn't require a major bump release.

The code of the library is perfectly backwards compatible.

@gzm0
Copy link
Contributor

gzm0 commented May 17, 2024

The code of the library is perfectly backwards compatible.

That's irrelevant. If you do not publish it a Scala 2.11 cannot use it anymore. Scala 2 libraries need to be published for the minor Scala version. A library published for 2.11 cannot be used with 2.12 and vv.

@gzm0
Copy link
Contributor

gzm0 commented May 17, 2024

Actually, a similar comment applies to Scala native itself: How are libraries published for Scala native 0.5.x and Scala native 0.4.x published? If they are not compatible, we should probably cross compile & publish for Scala native 0.4.x and 0.5.x.

@sideeffffect
Copy link
Contributor

If you do not publish it a Scala 2.11 cannot use it anymore. Scala 2 libraries need to be published for the minor Scala version. A library published for 2.11 cannot be used with 2.12 and vv.

Yes. All of this is true, no disagreement there.

But at the same time, this doesn't warrant major bump of the library in question. At least it didn't warrant for any libraries which dropped Scala 2.11 and Native 0.4.x and adopted Native 0.5.x. E.g. com-lihaoyi has already migrated to 0.5.x: https://github.com/orgs/com-lihaoyi/discussions/1

No major bump.

we should probably cross compile & publish for Scala native 0.4.x and 0.5.x

Nobody does that from what I've seen. Library developers stop cross-compiling to 0.4.x and start cross-compiling to 0.5.x in one commit/PR.

@sjrd
Copy link
Contributor

sjrd commented May 17, 2024

I agree with @sideeffffect on the versioning aspect. We need a major version bump if we fragment the ecosystem. That is true only if the things which are still supported are incompatible with previous versions. A library that still supports 2.11 will always be able to stay on the old version, because it could not possibly depend on the third library requiring the new version of portable-scala-reflect (because such a new version would not exist for 2.11).

Here I think a minor version bump would be enough.

That said, I don't think I understand why it is even necessary to drop any support here? It seems we could keep expanding our build matrix instead, as we usually do?

@sideeffffect
Copy link
Contributor

sideeffffect commented May 17, 2024

why it is even necessary to drop any support here? It seems we could keep expanding our build matrix instead, as we usually do?

Of course, that's entirely possible. But it's just more load on library maintainers (which in this case is you guys ❤️ ). And in practice people don't do that. This library also isn't cross-compiled to Scala.js 0.6.x. And the consensus in the Scala Native community is also dropping 0.4.x immediately (see com-lihaoyi above).

But if that's what you'd prefer, I'm sure that wouldn't be a problem, the PR can accommodate this approach.

@kciesielski
Copy link

FYI sttp, tapir and other SoftwareMill libs are also switching directly to 0.5.1 without a major version bump.

@MateuszKubuszok
Copy link

MUnit took the approach where they moved to 0.5.x but manually released artifacts for 0.4.x (see: mvn and discussion) for a single version - it doesn't transitively force their users to update dependencies when they want to only want to change Scala Native version.

Not all libraries choose this, but if all of my dependencies did it, I would release 1 version for both 0.4 and 0.5 by hand, to make it easier on my users.

@gzm0
Copy link
Contributor

gzm0 commented May 27, 2024

OK. I think I understand now why we do not need a major version bump to drop support for cross versioned things: The version is mostly relevant on the artifacts and not on the git repository itself. So for things we drop support, an artifact with the new version simply doesn't exist.

  • So whether to cross publish to native 0.4.x is probably best left up to the native core maintainers.
  • IMO we should not drop support for things we don't have to in this PR.
  • Does the sbt version bump force a downstream bump? If yes, do we need a minor bump?

@sideeffffect
Copy link
Contributor

Does the sbt version bump force a downstream bump?

Not at all.

@sideeffffect
Copy link
Contributor

Is there any think blocking this PR? If yes, how can I help moving it forward? There's a lot of interest in getting this merged and released, because of lot of transitive dependents this project has.

@gzm0
Copy link
Contributor

gzm0 commented May 30, 2024

IMO we should not drop support for things we don't have to in this PR.

Basically this PR changes too much. If you do not want to wait on @xerial , I suggest opening a new, minimal PR.

@sjrd
Copy link
Contributor

sjrd commented May 30, 2024

I'll try making an alternative tomorrow that only adds support without breaking anything.

@sideeffffect
Copy link
Contributor

@gzm0 what do you mean? I see only changes in sbt setting to enable Scala Native 0.5.x. What changes do you think should be left out?

@xerial
Copy link
Author

xerial commented May 31, 2024

@sjrd FYI. The only reason I needed to drop Scala.js 0.6.x support is due to this missing dependency error:

[warn] 	Note: Unresolved dependencies path:
[error] sbt.librarymanagement.ResolveException: Error downloading org.scala-js:scalajs-compiler_2.12.19:0.6.33
[error]   Not found
[error]   Not found
[error]   not found: /home/runner/.ivy2/localorg.scala-js/scalajs-compiler_2.12.19/0.6.33/ivys/ivy.xml
[error]   not found: https://repo1.maven.org/maven2/org/scala-js/scalajs-compiler_2.12.19/0.6.33/scalajs-compiler_2.12.19-0.6.33.pom
[error] 	at lmcoursier.CoursierDependencyResolution.unresolvedWarningOrThrow(CoursierDependencyResolution.scala:344)
[error] 	at lmcoursier.CoursierDependencyResolution.$anonfun$update$38(CoursierDependencyResolution.scala:[31](https://github.com/portable-scala/portable-scala-reflect/actions/runs/8760907044/job/24050014845#step:5:32)3)
[error] 	at scala.util.Either$LeftProjection.map(Either.scala:573)
[error] 	at lmcoursier.CoursierDependencyResolution.update(CoursierDependencyResolution.scala:313)
[error] 	at sbt.librarymanagement.DependencyResolution.update(DependencyResolution.scala:60)
[error] 	at sbt.internal.LibraryManagement$.resolve$1(LibraryManagement.scala:60)
[error] 	at sbt.internal.LibraryManagement$.$anonfun$cachedUpdate$12(LibraryManagement.scala:134)
[error] 	at sbt.util.Tracked$.$anonfun$lastOutput$1(Tracked.scala:74)
[error] 	at sbt.internal.LibraryManagement$.$anonfun$cachedUpdate$20(LibraryManagement.scala:147)
[error] 	at scala.util.control.Exception$Catch.apply(Exception.scala:228)
[error] 	at sbt.internal.LibraryManagement$.$anonfun$cachedUpdate$11(LibraryManagement.scala:147)
[error] 	at sbt.internal.LibraryManagement$.$anonfun$cachedUpdate$11$adapted(LibraryManagement.scala:128)
[error] 	at sbt.util.Tracked$.$anonfun$inputChangedW$1(Tracked.scala:220)
[error] 	at sbt.internal.LibraryManagement$.cachedUpdate(LibraryManagement.scala:161)
[error] 	at sbt.Classpaths$.$anonfun$updateTask0$1(Defaults.scala:3801)
[error] 	at scala.Function1.$anonfun$compose$1(Function1.scala:49)
[error] 	at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:63)
[error] 	at sbt.std.Transform$$anon$4.work(Transform.scala:69)
[error] 	at sbt.Execute.$anonfun$submit$2(Execute.scala:283)
[error] 	at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:24)
[error] 	at sbt.Execute.work(Execute.scala:292)
[error] 	at sbt.Execute.$anonfun$submit$1(Execute.scala:283)
[error] 	at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[error] 	at sbt.CompletionService$$anon$2.call(CompletionService.scala:65)
[error] 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[error] 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
[error] 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[error] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[error] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[error] 	at java.lang.Thread.run(Thread.java:748)
[error] (portable-scala-reflectJS / update) sbt.librarymanagement.ResolveException: Error downloading org.scala-js:scalajs-compiler_2.12.19:0.6.[33](https://github.com/portable-scala/portable-scala-reflect/actions/runs/8760907044/job/24050014845#step:5:34)
[error]   Not found
[error]   Not found
[error]   not found: /home/runner/.ivy2/localorg.scala-js/scalajs-compiler_2.12.19/0.6.33/ivys/ivy.xml
[error]   not found: https://repo1.maven.org/maven2/org/scala-js/scalajs-compiler_2.12.19/0.6.33/scalajs-compiler_2.12.19-0.6.33.pom

https://github.com/portable-scala/portable-scala-reflect/actions/runs/8760907044/job/24050014845

@sjrd
Copy link
Contributor

sjrd commented May 31, 2024

Superseded by #49 and other PRs that came before it. Thanks for pushing us in the right direction.

Version 1.1.3 is now on Maven Central with Scala Native 0.5.x support (and everything else that was supported in 1.1.2).

@sjrd sjrd closed this May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants