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

Add Scala 2.13.x support #1916

Merged
merged 75 commits into from Nov 12, 2020
Merged

Conversation

WojciechMazur
Copy link
Contributor

@WojciechMazur WojciechMazur commented Oct 5, 2020

This PR adds support for 2.13 Scala version on top of not yet merged work of Ergys on #1877
Resolves #1629

CI was temporally disabled for this PR but it passed all tests when run locally

Before merging this PR following changes should be addressed:

  • Merging Add Scala 2.12 support #1877 and rebasing this PR on top of incoming changes
  • Adjusting CI process to allow less time consuming testing - with current pipeline it may take up to 10 hours of CI uptime (~3-4h real time) even if it would fail on scalafmt check. This may cause DoS for other pipelines in case if there is no one to control CI. Travis allows using stages to describe dependencies between jobs, while current setup runs all jobs even if any of them failed.

Changes include:

  • Allowing cross-building across project (includes fixing deprecation originated from 2.13 changes)
  • Adding overrides for 2.13, removal of redundant file overrides for other versions and using overrides dir for common changes
  • Adding scala-collection-compat sources to scalalib overrides for support of new collection API, eg. java.Stream uses underyling scala.Stream/scala.LazyList
  • Updating auxlib runtime.Statics to changes introduced in 2.13
  • Symbol changes compat across 2.13.0 - 2.13.2 and later revisions (changed toString method)
  • Partially porting Hashtable implementation from Scala.js due to cyclic dependency on Properties with underyling scala.Hashmap and new collections implementation using Properties to tune underlying Vector
  • Added support for non-Integer match cases which are translated to if-else chain.
  • Test fixes, eg. HashingSuite for case classes which yields different values since 2.13

@ekrich
Copy link
Member

ekrich commented Oct 5, 2020

Wow, this is great but now we will have to cross build our Scala Native libraries. 😄

I have a couple of questions.

  1. What is the plan for updating the scala-collections-compat? Will it be relatively easy to keep up to date?
  2. Since you ported part of the Scala.js collections across, is there a plan or an Issue that can keep track of which parts have been ported so someone can finish or it can be done a little at a time?

@WojciechMazur
Copy link
Contributor Author

@ekrich I don't think there is any plan for updating this lib but it should be quite easy to keep up to date. Although we also can consider removing it and replacing all usages by some other cross-version safe alternative. For now it looked as easiest way to fix this problem and move with migration.
I don't think there is any issue for that right now. In long term it would be great to use common javalib implementation between SN and Scala.js (at least in most parts)

@ekrich
Copy link
Member

ekrich commented Oct 5, 2020

Right now, I use scala-collections-compat in downstream projects as it supports 0.4.0-M2 and all the other cross versions and platforms. Now I'm wondering if that is going to cause problems.

@ekrich
Copy link
Member

ekrich commented Oct 5, 2020

Also, for collections, I added ScalaOps which allows us to remove the scala.collection.JavaConverters. I used that in java.util.Properties. Not sure it that is used elsewhere in Scala.js.

@sjrd
Copy link
Collaborator

sjrd commented Oct 5, 2020

ScalaOps is used in virtually every file of java.util.* in Scala.js ;)

build.sbt Outdated Show resolved Hide resolved
@WojciechMazur WojciechMazur marked this pull request as ready for review Oct 14, 2020
@WojciechMazur WojciechMazur force-pushed the feature/2.13.x-support branch 2 times, most recently from b7f9733 to 521b12a Compare Oct 15, 2020
@WojciechMazur WojciechMazur force-pushed the feature/2.13.x-support branch 3 times, most recently from 4f07a25 to 5ed7bc7 Compare Nov 6, 2020
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Show resolved Hide resolved
build.sbt Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
Copy link
Collaborator

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few drive-by comments to hopefully entirely get rid of scala-collections-compat.

util/src/main/scala/scala/scalanative/util/package.scala Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
tools/src/main/scala/scala/scalanative/build/LLVM.scala Outdated Show resolved Hide resolved
tools/src/main/scala/scala/scalanative/build/IO.scala Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
Copy link
Collaborator

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I only have nitpicks left.

javalib/src/main/scala/java/io/BufferedOutputStream.scala Outdated Show resolved Hide resolved
javalib/src/main/scala/java/math/BigDecimal.scala Outdated Show resolved Hide resolved
javalib/src/main/scala/java/net/PlainSocketImpl.scala Outdated Show resolved Hide resolved
javalib/src/main/scala/java/lang/System.scala Outdated Show resolved Hide resolved
project/build.sbt Outdated Show resolved Hide resolved
scalalib/overrides-2.13/scala/package.scala Outdated Show resolved Hide resolved
unit-tests/src/test/scala/scala/HashCodeTest.scala Outdated Show resolved Hide resolved
sjrd
sjrd approved these changes Nov 12, 2020
@sjrd sjrd merged commit 252d9b7 into scala-native:master Nov 12, 2020
@ruurtjan
Copy link

So happy to see this merged into master! Thanks for all the effort :)

Is there anything you can say on when this will be released?

@sjrd
Copy link
Collaborator

sjrd commented Nov 12, 2020

Several weeks if we're lucky.

vicopem pushed a commit to vicopem/scala-native that referenced this pull request Nov 19, 2020
vicopem added a commit to vicopem/scala-native that referenced this pull request Nov 19, 2020
vicopem added a commit to vicopem/scala-native that referenced this pull request Nov 19, 2020
@WojciechMazur WojciechMazur deleted the feature/2.13.x-support branch Feb 15, 2021
ekrich pushed a commit to ekrich/scala-native that referenced this pull request May 21, 2021
WojciechMazur added a commit to WojciechMazur/scala-native that referenced this pull request Aug 25, 2021
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.

Scala 2.13 support
5 participants