Skip to content

Lift jetty11#2017

Merged
farmdawgnation merged 5 commits into
lift:lift-4.0from
pabloazul:lift-jetty11
Aug 25, 2024
Merged

Lift jetty11#2017
farmdawgnation merged 5 commits into
lift:lift-4.0from
pabloazul:lift-jetty11

Conversation

@pabloazul

@pabloazul pabloazul commented Aug 22, 2024

Copy link
Copy Markdown

https://groups.google.com/g/liftweb/c/FhAM0ZB-JpI

Updates sbt, scala, jetty, javax.servlet -> jakarta.servlet, and fixes some of the deprecated scala syntax in preperation for moving to scala 3.

Fixed:
type Traversable in package scala is deprecated (since 2.13.0): Use Iterable instead of Traversable
object JavaConverters in package collection is deprecated (since 2.13.0): Use scala.jdk.CollectionConverters instead
view bounds are deprecated; use an implicit parameter instead.
Implicit definition should have explicit type
Adding a number and a String is deprecated. Use the string interpolation s"$num$str"

Dropped EnumWithDescription which seems to not be used and contained a cyclic reference that no longer compiles.

Removed the cross compiling for now and dropped the persistence library from build.sbt and did not attempt to update it.

There are still more warnings that need attention, but I fixed all of the trivial rewrites.

@pabloazul

Copy link
Copy Markdown
Author

I accidentally got some bloop cruft in the commit, so would redo it before it could be viable for merge.

@frmrm

frmrm commented Aug 23, 2024

Copy link
Copy Markdown

Do you think we target these changes for the mainline 3.x, the semi-compatible 4.x line, or the break the world 5.x line? The sheer number of changes and removal of attention to persistence biases me a bit in the direction of 5.x.

@andreak

andreak commented Aug 23, 2024

Copy link
Copy Markdown
Member

I vote for "clean it up" in 4, then Scala3 in 5, so I'm infavor of removing persistence etc. in 4.

@pabloazul

Copy link
Copy Markdown
Author

I would agree with Andreak. 4 is a straight-up breaking change wrt to persistence, but otherwise basically just getting to most recent dependencies. Then 5 is scala3 + whatever other features anyone is motivated to support, such as the json macros.

@farmdawgnation

Copy link
Copy Markdown
Member

I'm supportive of this approach. I'm going to change the target branch to the lift-4.x line for now.

@farmdawgnation farmdawgnation changed the base branch from master to lift-4.0 August 24, 2024 23:16

@farmdawgnation farmdawgnation left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once the bloop changes are cleaned up I think this will be g2g.

@pabloazul

Copy link
Copy Markdown
Author

Once the bloop changes are cleaned up I think this will be g2g.

deblooped.

@pabloazul

pabloazul commented Aug 25, 2024 via email

Copy link
Copy Markdown
Author

@pabloazul pabloazul changed the title Lift jetty11 - not ready for merge Lift jetty11 Aug 25, 2024
@farmdawgnation

Copy link
Copy Markdown
Member

I believe you just need to push a commit adding your name and email as a signature on this document: https://github.com/lift/framework/blob/master/contributors.md

@farmdawgnation farmdawgnation left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good. Going to give this a run on my machine to see if I can find any kinks in the sbt config before merge.

@farmdawgnation

Copy link
Copy Markdown
Member

Confirming all the tests are passing as long as I run it in Java 11 - however given that it's 2024, we have a few LTS releases ahead of that, and this is a major version bump anyhow - I'm fine with that.

Merging!

@andreak

andreak commented Aug 26, 2024

Copy link
Copy Markdown
Member

+1 for java21 as the minimum for Lift4.

@pabloazul

pabloazul commented Aug 26, 2024 via email

Copy link
Copy Markdown
Author

@fanf

fanf commented Aug 26, 2024

Copy link
Copy Markdown
Contributor

Oups, late on the game but for some contexte: I'm extremelly interested by that PR and a Lift 4 version that is just upgrading to Java 17 and servlet 4/5.
I was the one asking for it on the mailing list because we are currently blocked in our Spring Security upgrade due to the servlet namespace. But we would like to not have other breaking changes in the core/web parts. So the proposition by @pabloazul seems good with us.
I'm going to try that version and report any worthy feedback.
In the meantime, THANK YOU SO MUCH for that PR. It was more involved than what I was thinking it would be. (the type ascription part is not the one I love the much with scala 3)

@pabloazul

pabloazul commented Aug 26, 2024 via email

Copy link
Copy Markdown
Author

@farmdawgnation

Copy link
Copy Markdown
Member

Let's try from the right account this time!

Unless folks feel suuuuper strongly, I'd prefer we aim for Java 11+ for Lift 4 and Java 17+ for Lift 5. Mostly just because Lift 3 supports all the way back to Java 8 and I feel a bit bad for moving multiple LTS releases of Java in a single Lift major bump. 😅

@fanf

fanf commented Aug 27, 2024

Copy link
Copy Markdown
Contributor

@farmdawgnation : no big feeling on jdk11 vs jdk17. It looks from my point of view that JDK8 was a specificity, but once people migrated away from it, they somehow followed JDK updates.
And so why not JDK21 directly cc @andreak ? Just because we're still in the "around 2 years of transition for ecosystem to migrate to the next version when out". Or just "support the last 2 LTS". Typically in Rudder, we need to support Linux distrib that were released before JDK21 (and make things complicated to have it).
But if lift want to support JDK11 in Lift4, and given the goal of that version to break as little things as possible while modernizing the stack, it looks good to me too.

@pabloazul : the good new: it works perfectly and I was triavialy able to update Rudder to servlet 5 / jetty11.
The bad news: above report is only based on compilation and unit tests, because updating to Spring 6 is an other story, and totally broken for now, so the app doesn't start (well, it starts but fail as soon as we reach the authentication bootstrapping).
So it seems OK like that, and I'm going to sacrifices things to make Spring God happy.

@fanf

fanf commented Aug 28, 2024

Copy link
Copy Markdown
Contributor

OK, after a bit more testing, everything is working fine on my side. We would be extremely glad to have a version alpha1 of lift 4 published as it stands now, it would greatly facilitate migration and more testing.

Thank you all for the amazing work ! It's unbloking a major topic for us.

@farmdawgnation

Copy link
Copy Markdown
Member

Hey @fanf - Sorry for not replying sooner, but acknowledged. I can probably get an M1 build spin up sometime this week. Going to try for today but my kids may have other plans. 😂

@fanf

fanf commented Sep 1, 2024 via email

Copy link
Copy Markdown
Contributor

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.

6 participants