Upgrade Kotlin to v1.4

[mikaello]

I am using this library in my application, but it prevents me from upgrading to Kotlin v1.4 since this library includes Kotlin v1.3.72. I can force my application to use Kotlin stdlib 1.3, but then some of the point of upgrading will be gone.

This PR is not merge ready, since it may break all Kotlin v1.3 projects (not verified/checked), but it is not possible to create an issue, so this PR will serve as a placeholder for this issue I guess?!

[mikaello]

This is not a nice solution, but I could honestly not figure out why this lambda returned Unit inside runInTrace when the return value was not assigned to a variable. So to make the test pass I needed to store the return value (which is 1 , the same as it returns inside runInTrace).

Looks like an undocumented (or at least very subtle, I tried to find any documentation) breaking change?

[rocketraman]

Hmm, you're right -- seems like a subtle change in behavior due to the new type inference.

We might as well assert on the returned count:

    var returnedCount = 0
    val f = withLevelFixture(Level.INFO, true) {
      returnedCount = it.runInTrace(entryMsg) {
        ++count
      }
    }
    assertTrue { count == 1 }
    assertTrue { returnedCount == 1 }

[rocketraman]

Thank you for the patch. Note that issues are created here: https://issues.apache.org/jira/projects/LOG4J2/issues.

According to Kotlin's documentation at https://kotlinlang.org/docs/kotlin-evolution.html#evolving-the-binary-format, the following things are true:

All binaries are backwards compatible, i.e. a newer compiler can read older binaries (e.g. 1.3 understands 1.0 through 1.2),

Therefore, a 1.4 project should be able to use kotlin logging API compiled with 1.3. I have been doing so with this library myself, but it does require a transitive exclude of the 1.3 stdlib. We should probably change the stdlib dependency to a runtime (or optional I guess in Maven) dependency to improve compat with projects using 1.4.

Preferably (but we can't guarantee it), the binary format is mostly forwards compatible with the next feature release, but not later ones (in the cases when new features are not used, e.g. 1.3 can understand most binaries from 1.4, but not 1.5).

This means that we should be able to upgrade to 1.4, and projects on 1.3 should continue to work, however that is not guaranteed by the Kotlin binary compatibility promise, and if we do the same thing with 1.5 in the future, we'll definitely be breaking projects still on 1.3.

Therefore, unless I'm misunderstanding something, we should probably stay on 1.3 but tweak the build to not publish the stdlib as a transitive dependency to improve compat with 1.4 projects.

@jvz What do you think?

[rocketraman]

Hmm, you're right -- seems like a subtle change in behavior due to the new type inference.

We might as well assert on the returned count:

    var returnedCount = 0
    val f = withLevelFixture(Level.INFO, true) {
      returnedCount = it.runInTrace(entryMsg) {
        ++count
      }
    }
    assertTrue { count == 1 }
    assertTrue { returnedCount == 1 }

[mikaello]

@rocketraman Thanks a lot for the thorough answer. And sorry for my comment on missing issue tracker, you even had a separate section about it in the Readme, my bad.

I think your summary indicating that this project should be on 1.3 and instead avoid distributing stdlib as a transitive dependency sounds like a reasonable solution.

[rocketraman]

I think your summary indicating that this project should be on 1.3 and instead avoid distributing stdlib as a transitive dependency sounds like a reasonable solution.

Care to submit a patch? :-)

[solaristhesun]

What ist the status on this? I am getting the following errors:

w: Runtime JAR files in the classpath should have the same version. These files were found in the classpath:
    C:/Users/sts/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.4.30/aeecfab2e6acbcb80fc6628e8882c98cbbd819cf/kotlin-stdlib-jdk8-1.4.30.jar (version 1.4)
    C:/Users/sts/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.4.30/cd81ed90fd79e43f587bd384c2bc6d1aed0bfe31/kotlin-stdlib-jdk7-1.4.30.jar (version 1.4)
    C:/Users/sts/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-reflect/1.3.0/6fd129fd9ba8581f2cb9c58bfd431dda4ee0457e/kotlin-reflect-1.3.0.jar (version 1.3)
    C:/Users/sts/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.4.30/d10d1e10f47006ee08162dde039e38ac487de4ac/kotlin-stdlib-1.4.30.jar (version 1.4)
    C:/Users/sts/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.4.30/bb9a3173350f55732416ee27956ea8f9b81f4dbb/kotlin-stdlib-common-1.4.30.jar (version 1.4)
w: Consider providing an explicit dependency on kotlin-reflect 1.4 to prevent strange errors
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath

So it looks to me as if it is not working in kotlin 1.4 despite someone saying it should. What steps are necessary to get it working in 1.4.

[rocketraman]

So it looks to me as if it is not working in kotlin 1.4 despite someone saying it should. What steps are necessary to get it working in 1.4.

See my last comment. You're having an issue since you are transitively pulling in kotlin-reflect 1.3. A temporary work-around is to depend explicitly on kotlin-reflect 1.4 in your project, until this (or another) pull can be updated with the changes in this project's POM to avoid sending the kotlin lib dependencies to downstream projects.

[mikaello]

Sorry for not replying! I tried to figure out how to avoid distributing the stdlib as a transitive dependency, but I struggled a bit, and after a while I surrendered and changed my project import to:

        <dependency>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-api-kotlin</artifactId>
            <version>1.0.0</version>
+           <exclusions>
+               <exclusion>
+                   <groupId>org.jetbrains.kotlin</groupId>
+                   <artifactId>kotlin-stdlib</artifactId>
+               </exclusion>
+           </exclusions>
        </dependency>

[rocketraman]

Closing this in favor of #17, to maintain compatibility with older versions of Kotlin for as long as possible.

[rocketraman]

Now that 1.6 is out, its time to resurrect this and move to 1.4.

[jvz]

I've opened #31 to track this. We can work on upgrading the minimum Kotlin version after I make a 1.3.x release.

[rocketraman]

Closing in favor of #14 which also updates coroutines and rebases on latest master.

[mikaello]

👍🏻

Closing in favor of #14 which also updates coroutines and rebases on latest master.

34, not 14: #34