You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2020/12/21 09:58:43 UTC
[GitHub] [logging-log4j-kotlin] mikaello opened a new pull request #14: Upgrade Kotlin to v1.4
mikaello opened a new pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14
I am using this library in my application, but it prevents me from upgrading to v1.4 since this library includes Kotlin 1.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 v1.3 projects, but it is not possible to create an issue, so then this PR will serve as a placeholder for this issue I guess?!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] solaristhesun edited a comment on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
solaristhesun edited a comment on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-777325037
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] mikaello commented on a change in pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
mikaello commented on a change in pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#discussion_r546624688
##########
File path: log4j-api-kotlin/src/test/kotlin/org.apache.logging.log4j.kotlin/LoggerTest.kt
##########
@@ -173,7 +173,8 @@ class LoggerTest {
fun `Run in trace with result`() {
var count = 0
val f = withLevelFixture(Level.INFO, true) {
- it.runInTrace(entryMsg) {
+ @SuppressWarnings("unused")
+ val mustBeHere = it.runInTrace(entryMsg) {
Review comment:
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?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] rocketraman commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
rocketraman commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-749624621
> 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? :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] rocketraman commented on a change in pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
rocketraman commented on a change in pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#discussion_r546721176
##########
File path: log4j-api-kotlin/src/test/kotlin/org.apache.logging.log4j.kotlin/LoggerTest.kt
##########
@@ -173,7 +173,8 @@ class LoggerTest {
fun `Run in trace with result`() {
var count = 0
val f = withLevelFixture(Level.INFO, true) {
- it.runInTrace(entryMsg) {
+ @SuppressWarnings("unused")
+ val mustBeHere = it.runInTrace(entryMsg) {
Review comment:
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 }
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] rocketraman commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
rocketraman commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-996528405
Now that 1.6 is out, its time to resurrect this and move to 1.4.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] mikaello commented on a change in pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
mikaello commented on a change in pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#discussion_r546624688
##########
File path: log4j-api-kotlin/src/test/kotlin/org.apache.logging.log4j.kotlin/LoggerTest.kt
##########
@@ -173,7 +173,8 @@ class LoggerTest {
fun `Run in trace with result`() {
var count = 0
val f = withLevelFixture(Level.INFO, true) {
- it.runInTrace(entryMsg) {
+ @SuppressWarnings("unused")
+ val mustBeHere = it.runInTrace(entryMsg) {
Review comment:
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 stored. 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?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] rocketraman commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
rocketraman commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-777474336
> 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] rocketraman commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
rocketraman commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-837386965
Closing this in favor of https://github.com/apache/logging-log4j-kotlin/pull/17, to maintain compatibility with older versions of Kotlin for as long as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] mikaello commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
mikaello commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-749392389
@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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] mikaello commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
mikaello commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-777526127
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:
```diff
<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>
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] solaristhesun commented on pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
solaristhesun commented on pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14#issuecomment-777325037
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j-kotlin] rocketraman closed pull request #14: Upgrade Kotlin to v1.4
Posted by GitBox <gi...@apache.org>.
rocketraman closed pull request #14:
URL: https://github.com/apache/logging-log4j-kotlin/pull/14
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org