all: prepare for Scala3 #36

ahjohannessen · 2023-04-14T13:27:38Z

adjust tests to use mockito instead of specs2 mock wrappers.
cross compile scala 2 and scala 3

ndeverge

@honeycomb-cheesecake could you please check and merge the changes? Thanks!

mkurz · 2023-10-23T08:51:14Z

@ahjohannessen @ndeverge please check my message in the discord play-user channel, thanks!

mkurz · 2023-11-20T15:28:59Z

I do have an almost working version locally, just I got in some conversations/chats this afternoon 😉 We will be done soon here 😉

ndeverge · 2023-11-20T15:42:45Z

@mkurz oh sorry, I was working on it too this afternoon :-(
Don't worry, I leave you to it, let me know if you have a testable version, I'll test the integration with my own project and Play 2.9

mkurz · 2023-11-20T15:45:16Z

@ndeverge Please push your work either here or in a different branch, so it's not lost, we can incorporate what we both have done.

mkurz · 2023-11-20T15:45:33Z

I can rebase onto yours, no problem

mkurz · 2023-11-20T16:07:30Z

@ndeverge Did you run into this E is not a legal path error?

mkurz · 2023-11-20T16:12:46Z

project/plugins.sbt

@@ -10,3 +10,5 @@ addSbtPlugin(dependency = "org.scalariform" % "sbt-scalariform" % "1.8.3")
 addSbtPlugin(dependency = "org.scoverage" % "sbt-scoverage" % "2.0.7")
 addSbtPlugin(dependency = "org.scoverage" % "sbt-coveralls" % "1.3.11")
 addSbtPlugin(dependency = "org.xerial.sbt" % "sbt-sonatype" % "3.10.0")
+
+ThisBuild / libraryDependencySchemes += "org.scala-lang.modules" %% "scala-xml" % VersionScheme.Always


@ndeverge The problem here is that sbt-scapegoat pulls in scala-xml verson 1, but every other library already upgraded to scala-xml version 2. Given that scapegoat also does not come with Scala 3 artifacts yet (see scapegoat-scala/scapegoat#521), I think we should just drop it... We don't use it in any other Play repos anyway.

Also then this workaround is not necessary anymore.

@mkurz Right, let's get rid of it then!

ndeverge · 2023-11-20T17:02:24Z

@ndeverge Did you run into this E is not a legal path error?

I was on the main codebase, now I need to make a test against this branch to check if I still get this error.

ndeverge · 2023-11-21T08:30:22Z

@mkurz good news! I tested this branch with my project, migrated to Play 2.9.0 and so far so good.
I'll clean this branch today, eg making it compile and I'll remove scapegoat.

coveralls · 2023-11-21T09:36:41Z

Pull Request Test Coverage Report for Build 7140244048

2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.07%) to 94.187%

Totals
Change from base Build 6942082710:	0.07%
Covered Lines:	1361
Relevant Lines:	1445

💛 - Coveralls

mkurz · 2023-11-21T09:39:55Z

@ndeverge Let you me know when you are done or stuck

- Avoiding automatic dependency on akka http server makes it easier for dependant projects to use Scala 3 and netty instead. - adjust tests to use mockito instead of specs2 mock wrappers. - drop jdk8 from CI

mkurz · 2023-11-21T15:32:59Z

@ahjohannessen @MathisGuillet1 @ndeverge Can you help out here? Please check out this branch and in sbt run

++3.x
compile

You will see errors, related to

I think this needs more work to port to Scala 3...

@SethTisue maye you can help out here as well?

ndeverge · 2023-11-22T08:37:17Z

Here is shorter snippet of the issue:

trait Identity

trait Env {
  type I <: Identity
}

trait IdentityService[T <: Identity]

trait Environment[E <: Env] {
  def identityService: IdentityService[E#I]
}

The Scala 3 compiler fails with:

[error] 11 |  def identityService: IdentityService[E#I]
[error]    |                                       ^
[error]    |                                       E is not a legal path
[error]    |                                       since it is not a concrete type
[error] one error found

You can try directly on https://scastie.scala-lang.org/IScKT1QfStKC3ILH47LnQw

Maybe @DmytroMitin could help too?

ahjohannessen · 2023-11-22T11:31:42Z

@mkurz I think an equivalent encoding is needed for:

trait Identity

trait Env {
  type I <: Identity
}

trait IdentityService[T <: Identity]

trait Environment[E <: Env] {
  def identityService: IdentityService[E#I]
}

In Scala 3 this is a no go.

SethTisue · 2023-11-22T20:59:36Z

(not volunteering to get involved here, sorry)

ndeverge · 2023-11-28T05:34:05Z

Only 6 errors left in test sources, but I am a bit stuck now understanding the meaning of some errors.

Thanks @MathisGuillet1 I'll have a look in a couple of days, and I'll try to do my best.

MathisGuillet1 · 2023-11-29T08:56:23Z

Boom, all ✔️ with scala 3
Just need to work a bit on cross compile now and we will be good to go!

ndeverge · 2023-11-29T10:08:39Z

Boom, all ✔️ with scala 3 Just need to work a bit on cross compile now and we will be good to go!

@MathisGuillet1 Congrats, thank you SO much!

I'll give it a try by integrating the Scala 3 version with my project to make sure that some features are not broken.
And I'll help you on the cross compile, but hardly this week.

mkurz · 2023-11-30T08:24:08Z

Great! I will review this PR and after it got merged I will quickly set up sbt-ci-release so we can cut the release.

IMHO we should publish it under

"org.playframework.silhouette" % "play-silhouette" % "x.x.x"

Any objections?

However, after the release process is set up you, @MathisGuillet1 and @ndeverge, will responsible for this repo. You can of course ping me or someone else if you run into problems in future, but the goal is that you take care of this repo.
Also, you can do releases on your own; after sbt-ci-release is set up you can just push tags and the release should happen automatically.

I want to set up things until tomorrow evening.

MathisGuillet1 · 2023-11-30T08:55:32Z

That's perfect for me @mkurz
We will take care of it once it is set up.
Thanks for your help the ci

MathisGuillet1 · 2023-11-30T12:46:29Z

@mkurz I discussed with @ndeverge this morning, it's also fine for him.
I also initiated a PR to update groupId based on this branch.
See the changes here : https://github.com/playframework/play-silhouette/pull/72/files/df8b5f39f07229391898028cb3faaa873f57e010..2f658d9bb1d673579f1c0d0e28f7706eef5ed3f4

ndeverge · 2023-11-30T15:41:59Z

@mkurz also, similar to Play 2.9 and 3.0, we think that we could use the 9.x version for Play 2.9 compatibility, and 10.x version for Play 3.0.
If you are ok with that, I think we can merge both PRs into the main branch.

MathisGuillet1 · 2023-12-04T15:14:30Z

Great! I will review this PR and after it got merged I will quickly set up sbt-ci-release so we can cut the release.

Any update on this @mkurz ?

mkurz · 2023-12-07T14:48:11Z

This is absolutely next on my list, I guess it should be really now done by tomorrow (if the PR is ok and everything)

Because Around relies on DelayedInit which was dropped in Scala 3

mkurz · 2023-12-08T10:41:18Z

silhouette/test/Helpers.scala

-  abstract override def around[T: AsResult](t: => T): Specs2Result = super.around {
-    try { before; t } finally { after }
-  }
-}


I removed these helpers, because they will not work in Scala 3, because Around relies on DelayedInit which is not doing anything anymore in Scala 3:
https://dotty.epfl.ch/docs/reference/dropped-features/delayed-init.html
Just to safe you from future headaches, why things are not working if you make use of them...

mkurz

I took a look and in general should be OK.
Didn't dive in too much in the Scala 3 fixes, but I guess it should work. Also tests I just scrolled over and not much changed as far as I can see.

The only thing which I think could be improved in future is to avoid too much duplicated code between the app-2 and app-3 folders, now it's just copy and paste. Maybe it would be possible to break out just the code that needs to be Scala 3 specific and put that in a class or something and then import/extend it from general Scala code.
Now you might run into troubles if you need to change code twice.
But not for now 😉

mkurz · 2023-12-08T10:46:13Z

Actually wait a moment...

mkurz · 2023-12-08T10:50:14Z

All good, tests are running for Scala 3 I see 😉

MathisGuillet1 · 2023-12-08T10:53:22Z

Maybe it would be possible to break out just the code that needs to be Scala 3 specific and put that in a class or something and then import/extend it from general Scala code.

You are right, I went straight to the point here but in the future, I'll try to isolate type projection only

ahjohannessen force-pushed the wip-prepare-scala3-remove-specs2-mock branch 5 times, most recently from 7666bef to 62b6281 Compare April 14, 2023 14:39

ndeverge approved these changes May 15, 2023

View reviewed changes

brkaisin mentioned this pull request Oct 18, 2023

Scala 3 support #41

Closed

mkurz force-pushed the wip-prepare-scala3-remove-specs2-mock branch from 62b6281 to 3cbc915 Compare November 20, 2023 12:27

mkurz reviewed Nov 20, 2023

View reviewed changes

ahjohannessen and others added 5 commits November 21, 2023 14:36

all: prepare for Scala3

89fbc64

- Avoiding automatic dependency on akka http server makes it easier for dependant projects to use Scala 3 and netty instead. - adjust tests to use mockito instead of specs2 mock wrappers. - drop jdk8 from CI

Activate Scala 3

2f856a2

Some imports needed by Scala 3

7a576f5

Import needed by Scala 3

8f81fbf

WIP Comment some flags Scala 3 does not like

71c6813

mkurz force-pushed the wip-prepare-scala3-remove-specs2-mock branch 2 times, most recently from e373023 to 71c6813 Compare November 21, 2023 15:28

mathis guillet added 7 commits November 28, 2023 10:27

Fix overloading errors

4a9c6ed

Fix imports

950dc39

Fix guice implicit conversions

88780fd

Add running wrapper for play specs with scala 3

f25f378

Fix some providers tests

4cab39b

Fix tests for scala 3

ddae063

Fix tests JWTAuthenticatorSpecs

4ab8fc9

Enable cross compile

9fcb3e3

MathisGuillet1 added 5 commits November 29, 2023 11:17

Fully working cross compiled version

76234e4

Build cleanup

7fe2b61

Disable Xlint-all flag

a63b0bb

Switch default to scala 3

b56e1d3

Switch back default to scala 2

df8b5f3

Remove (unused) helpers that rely on Around

eed5074

Because Around relies on DelayedInit which was dropped in Scala 3

mkurz reviewed Dec 8, 2023

View reviewed changes

mkurz approved these changes Dec 8, 2023

View reviewed changes

mkurz merged commit ab77893 into playframework:main Dec 8, 2023
9 checks passed

mkurz mentioned this pull request Feb 25, 2024

[play-]slick now comes with Scala 3 artifacts playframework/play-samples#368

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

all: prepare for Scala3 #36

all: prepare for Scala3 #36

ahjohannessen commented Apr 14, 2023 •

edited by MathisGuillet1

Loading

ndeverge left a comment

mkurz commented Oct 23, 2023

mkurz commented Nov 20, 2023

ndeverge commented Nov 20, 2023

mkurz commented Nov 20, 2023

mkurz commented Nov 20, 2023

mkurz commented Nov 20, 2023

mkurz Nov 20, 2023

mkurz Nov 20, 2023

ndeverge Nov 20, 2023

ndeverge commented Nov 20, 2023

ndeverge commented Nov 21, 2023

coveralls commented Nov 21, 2023 •

edited

Loading

mkurz commented Nov 21, 2023

mkurz commented Nov 21, 2023

ndeverge commented Nov 22, 2023 •

edited

Loading

ahjohannessen commented Nov 22, 2023

SethTisue commented Nov 22, 2023

ndeverge commented Nov 28, 2023

MathisGuillet1 commented Nov 29, 2023

ndeverge commented Nov 29, 2023

mkurz commented Nov 30, 2023

MathisGuillet1 commented Nov 30, 2023

MathisGuillet1 commented Nov 30, 2023 •

edited

Loading

ndeverge commented Nov 30, 2023

MathisGuillet1 commented Dec 4, 2023

mkurz commented Dec 7, 2023

mkurz Dec 8, 2023

mkurz left a comment

mkurz commented Dec 8, 2023

mkurz commented Dec 8, 2023

MathisGuillet1 commented Dec 8, 2023

all: prepare for Scala3 #36

all: prepare for Scala3 #36

Conversation

ahjohannessen commented Apr 14, 2023 • edited by MathisGuillet1 Loading

ndeverge left a comment

Choose a reason for hiding this comment

mkurz commented Oct 23, 2023

mkurz commented Nov 20, 2023

ndeverge commented Nov 20, 2023

mkurz commented Nov 20, 2023

mkurz commented Nov 20, 2023

mkurz commented Nov 20, 2023

mkurz Nov 20, 2023

Choose a reason for hiding this comment

mkurz Nov 20, 2023

Choose a reason for hiding this comment

ndeverge Nov 20, 2023

Choose a reason for hiding this comment

ndeverge commented Nov 20, 2023

ndeverge commented Nov 21, 2023

coveralls commented Nov 21, 2023 • edited Loading

Pull Request Test Coverage Report for Build 7140244048

💛 - Coveralls

mkurz commented Nov 21, 2023

mkurz commented Nov 21, 2023

ndeverge commented Nov 22, 2023 • edited Loading

ahjohannessen commented Nov 22, 2023

SethTisue commented Nov 22, 2023

ndeverge commented Nov 28, 2023

MathisGuillet1 commented Nov 29, 2023

ndeverge commented Nov 29, 2023

mkurz commented Nov 30, 2023

MathisGuillet1 commented Nov 30, 2023

MathisGuillet1 commented Nov 30, 2023 • edited Loading

ndeverge commented Nov 30, 2023

MathisGuillet1 commented Dec 4, 2023

mkurz commented Dec 7, 2023

mkurz Dec 8, 2023

Choose a reason for hiding this comment

mkurz left a comment

Choose a reason for hiding this comment

mkurz commented Dec 8, 2023

mkurz commented Dec 8, 2023

MathisGuillet1 commented Dec 8, 2023

ahjohannessen commented Apr 14, 2023 •

edited by MathisGuillet1

Loading

coveralls commented Nov 21, 2023 •

edited

Loading

ndeverge commented Nov 22, 2023 •

edited

Loading

MathisGuillet1 commented Nov 30, 2023 •

edited

Loading