search

lightbend-labs / jardiff

A tool for comparing JAR files, including method bodies and Scala 2 pickled signatures
Apache License 2.0
170 stars 23 forks source link

Set JDK to 1.8 #146

Closed mdedetrich closed 10 months ago

mdedetrich commented 10 months ago

The intention of this PR is to test if this project can compile with JDK 8. The reasons for this are

  1. At least with Pekko where we use this project to validate releases (i.e. we check if there is a bytecode difference between staging and locally built artifacts) and since Pekko is built with JDK 1.8 we constantly have to switch JDK's when testing with jardiff

  2. Given https://github.com/lightbend-labs/jardiff/pull/52 it makes sense to target JDK 1.8 since almost all sbt plugins target 1.8 (given that its built with Scala 2.12).

mdedetrich commented 10 months ago

@retronym Do you by any chance know what the reason behind the JDK 11 requirement is? At least sbt test works fine with JDK 8 so I don't think there is a hard line technical JDK 8 requirement but I may be missing something?

SethTisue commented 10 months ago

120 says something about logback? is that plausible? cc @Philippus

mdedetrich commented 10 months ago

120 says something about logback? is that plausible? cc @Philippus

Anything against downgrading Logback to the latest one that supports 1.8? iirc That version of logback does work with newer versions assuming you just do standard logging.

SethTisue commented 10 months ago

No objection here, but that's just my ignorance on the subject talking. I'll try to rope in Jason.

retronym commented 10 months ago

I'm happy to widen support to JDK 8

SethTisue commented 10 months ago

@mdedetrich okay, happy to merge a logback downgrade if you add one here. would you mind also adding a Scala Steward exemption so we don't accidentally re-upgrade it?

it would be nice to have some kind of smoke test for logging that would fail on JDK 8 without the upgrade, but I don't insist that you improve the status quo, there

mdedetrich commented 10 months ago

@mdedetrich okay, happy to merge a logback downgrade if you add one here. would you mind also adding a Scala Steward exemption so we don't accidentally re-upgrade it?

it would be nice to have some kind of smoke test for logging that would fail on JDK 8 without the upgrade, but I don't insist that you improve the status quo, there

I'll get onto this, will also do a smoke test

som-snytt commented 10 months ago

as a footnote, I would like to structure my utility to run on whichever JDK with whichever required dependencies. This came up with REPL and jline dropping JDK8. In the Age of Scala-CLI, this ought to be a non-issue. (Dropping JDK8 is not a simplification, because currently, one might want to support JDK11, 17, 21.)

mdedetrich commented 10 months ago

@SethTisue @retronym I have done everything that you have asked for. There were actually 2 dependencies I had to downgrade (logback/jgit) and a smoke test was added for both and I can confirm that this works locally by switching out JDK's. Scala steward ignore rules were also added.

Interestingly even though logback/slf4j is added as a dependency the project doesn't seem to do any actual logging (there may however be some indirect logging from transitive dependencies going on).

SethTisue commented 10 months ago

thank you @mdedetrich !

Interestingly even though logback/slf4j is added as a dependency the project doesn't seem to do any actual logging

@retronym curious if you remember anything about that. slf4j is already there even in your initial commit.