takari / polyglot-maven

Support alternative markup for Apache Maven POM files
Eclipse Public License 1.0
893 stars 101 forks source link

Update Scala to 2.12.18 #277

Closed lefou closed 1 year ago

lefou commented 1 year ago

Due to some security vulnerabilities in Scala 2.11 compiler, I switched polyglot-scala to Scala 2.12, which is the last version, for which there is a release of com.twitter:util-eval, which we use to compile and evaluate the pom.scala files.

Unfortunately, util-eval throws runtime exceptions when used as-is, which is mostly due to internal changes in the Scala compiler. Since util-eval doesn't work with Scala 2.12 and was even removed upstream for quite some time, I vendored the single file Eval.scala and applied some small refactorings to make it work. The imported file was licensed under the Apache Licence, version 2, which is identical to this project license.

Due to the version bump from Scala 2.11 to 2.12, I'd consider this pull request a breaking change. The next release number for polyglot(-scala) should be therefore 0.6.0 (if we apply early semantic versioning).

I think it's worth investigating, whether we can follow up with a bump to Scala 2.13. I haven't tested it yet, due to some dependencies not available. But it looks like com.googlecode.kiama has moved to https://github.com/inkytonik/kiama, so there should be no blockers. But the Eval might break again due to expected compiler changes.

cstamas commented 1 year ago

@headius we may consider splitting polyglots? We just had 0.5.0 (due "revive" when we changed to Maven 3.9.5 + JSR330 + more) and now due Scala upgrade we'd need 0.6.0 (to follow semantic versioning)... Any ideas? (or Ruby is just fine to get 0.6.0 as next)?

lefou commented 1 year ago

I updated the PR description. Before a release, I'd like to explore, if we can bump to Scala 2.13.

cstamas commented 1 year ago

+1, go for it (and ping me if we are good for release), and see what @headius have to say, but I think they will be fine with 0.6.0

lefou commented 1 year ago

Great. I'll do further changes in another PR. This one is IMHO good to merge. Maybe, we can make a 0.6.0-M1 available? There might be users, who can't directly use Scala 2.13 but only Scala 2.12, so they can progress in smaller steps.

cstamas commented 1 year ago

I'd merge and just do 0.6.0, am not a fan of "preview/M/alphas" :smile: Let's wait @headius response, but I think he will be fine with 0.6.0 as well.

cstamas commented 1 year ago

@lefou one question re Java8: do you insist on building polyglot with Java8 or having CI run test only?

As I just realized we could split the build just like MIMA does:

Like here: https://github.com/maveniverse/mima/blob/main/.github/workflows/ci.yml

Just asking, as "going back" with takari-lifecycle as seen with GPG signing, may bring or cause other issues as well, so I'd really really "move forward" with it, not "backward". This will require some changes, but IMHO is worth the cause...

lefou commented 1 year ago

I definitively don't insist to build with Java 8! I'm all for moving forward. I just insists in having an functional integration test that covers the Java 8 runtime. We claim to support it, so we should ensure it. As polyglot-scala has a working integration-test setup, which helps to cover essential use cases (and which helped me a lot in updating Scala support btw), it would be a great loss to not run them.

In previous discussions, I was under the impression, that testing on Java 8 - while building with a newer Java version - requires some special setup. As I'm not the guy who wanted to implement it, and I can't delegate this work to anybody else, I suggested to keep building with Java 8. If someone is implementing it however, I really appreciate it.

lefou commented 1 year ago

I'd merge and just do 0.6.0, am not a fan of "preview/M/alphas" 😄 Let's wait @headius response, but I think he will be fine with 0.6.0 as well.

That means, we will have a 0.7.0 shortly after. I already have a Scala 2.13 PR in the pipeline.

headius commented 1 year ago

Use whatever versions you want! Just make sure they go up instead of down!