tony19 / logback-android

đź“„The reliable, generic, fast and flexible logging framework for Android
Apache License 2.0
1.2k stars 172 forks source link

logback-android and logback-classic ILoggingEvent binary incompatibility getMarkers vs getMarkerList #365

Open ulfandersson opened 6 months ago

ulfandersson commented 6 months ago

Describe the bug

As stated in the readme, one option for unit tests is to use logback-classic as a replacement for logback-android.

Since the upgrade to sl4j 2.0 (https://github.com/tony19/logback-android/pull/247) the ILoggingEvent class is no longer binary compatible with logback-classic.

Specifically, and where this matters, is with regards to markers.

logback-android has: List getMarkers

logback-classic has: Marker getMarker List getMarkerList

Reproduction

Call getMarkers() on a ILoggingEvent in a custom encoder (extending EncoderBase<ILoggingEvent>), use this custom encoder from a test where logback-android has been replaced with logback-classic

Example:

class CustomEncoder : EncoderBase<ILoggingEvent>() {

    override fun headerBytes(): ByteArray? = null

    override fun footerBytes(): ByteArray? = null

    override fun encode(event: ILoggingEvent): ByteArray {
        return "markers: ${event.markers}".toByteArray()
    }
}

Logs

No response

logback-android version

3.0.0

OS Version

14

What logback configuration are you using? (logback.xml or Java/Kotlin code)

No response

Validations

azabost commented 2 months ago

There are a few more differences compared to logback-classic 1.5.6, e.g. it added public default java.time.Instant getInstant() to ILoggingEvent.

What's worse, it requires Java 8 API for two reasons:

  1. because Date/Time API was added in Java 8
  2. because they now use default implementations in interfaces

Unfortunately, logback-android uses Java 6 compatibility level:

compileOptions {
    sourceCompatibility JavaVersion.VERSION_1_6
    targetCompatibility JavaVersion.VERSION_1_6
}

Additionally, logback-classic uses Instant.ofEpochMilli(getTimeStamp()) in the default implementation of getInstant(). When I changed the compatibility level of logback-android to Java 8 on my machine for testing purposes, I've got an error from IDE saying that

Call requires API level 26 (current min is 9): java.time.Instant#ofEpochMilli

I hope it could be implemented differently in logback-android but I'm not sure.

EDIT: it worked when I added coreLibraryDesugaringEnabled true and coreLibraryDesugaring "com.android.tools:desugar_jdk_libs:2.0.4" to build.gradle

By the way, Android Gradle Plugin sets Java 8 compatibility level by default since 4.2.0.

That being said, could logback-android also migrate to Java 8 and enable core library desugaring? @tony19 @mrcsxsiq

Oh, and one more thing I noticed recently. When I used JDK 21 to build a project, I noticed this warning:

Java compiler version 21 has deprecated support for compiling with source/target version 8. Try one of the following options:

  1. [Recommended] Use Java toolchain with a lower language version
  2. Set a higher source/target version
  3. Use a lower version of the JDK running the build (if you're not using Java toolchain)

For more details on how to configure these settings, see https://developer.android.com/build/jdks. To suppress this warning, set android.javaCompile.suppressSourceTargetDeprecationWarning=true in gradle.properties.

warning: [options] source value 8 is obsolete and will be removed in a future release warning: [options] target value 8 is obsolete and will be removed in a future release

That might be yet another reason not to stay with such a low compatibility level.