Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: upgrade slf4j to 2.0.3 #247

Merged
merged 13 commits into from Dec 4, 2022
Merged

feat!: upgrade slf4j to 2.0.3 #247

merged 13 commits into from Dec 4, 2022

Conversation

compscidr
Copy link
Contributor

@compscidr compscidr commented Oct 14, 2022

Not sure if this is quite the right way of going about it, but was noticing if I updated slf4j to 2.0.x I wasn't getting logs showing up in Android any longer.

It looks like the only breaking change was in a few places Marker turned to List<Marker> so I updated throughout the code to take this wherever it was needed.

Tried it out against an android project and logging is now working again, so seems to work.

I have a feeling this will probably make things not work any longer with older slf4j though, so you may also not want to merge it, or make a different version for slf4j 2.x or something.

fix #249

@compscidr

This comment was marked as outdated.

@compscidr compscidr closed this Oct 14, 2022
@compscidr
Copy link
Contributor Author

compscidr commented Oct 14, 2022

Okay, realized it was not working previously because it needed an implementation of org.slf4j.impl.LoggerServiceProvider which I have now added, and also added a file to the META-INF/services which tells the service loader of slf4j which implementation to try to load as a logging provider. Seems to be working now by building the jar with your script and including it in an android project using slf4j 2.0.3.

If you want to try it out, you can use:

repositories {
    maven { url 'https://jitpack.io' }
}

dependencies {
    implementation 'com.github.compscidr:logback-android:slf4f-2.0.3-SNAPSHOT'
}

@compscidr compscidr reopened this Oct 14, 2022
@sorinirimies
Copy link

Hi, @compscidr, do you need some further testing support? I was wondering if this PR could afterwards be merged as our project relies on this library as well.
I am also asking myself whether in general, this library is still maintained or not.

@compscidr
Copy link
Contributor Author

compscidr commented Oct 24, 2022

Hi, @compscidr, do you need some further testing support? I was wondering if this PR could afterwards be merged as our project relies on this library as well. I am also asking myself whether in general, this library is still maintained or not.

So far I've been using the changes from the PR using the jitpack build since I pushed them and it seems to be working well. It would be great to hear how others are making out with it. I did some pretty janky quickfixes to just make it api compatible - I'm curious about the markers and whether they will work well.

I think merging directly into this repo now will likely break backwards compatibility issues with pre-2.x slf4j, so it might take some careful thought if both are to be maintained together. I'm not sure on the status of this repo / whether it is being maintained (it seems like there were some recent pushes in the last year or so), but if its not, I can make a fork that's compatible with 2.x and start to maintain it. I'd likely drop older slf4j support entirely and focus only on the new stuff.

In the meantime, I just discovered the test suite errors, I'll at least get it passing those things and get the green check when I get a chance.

@compscidr compscidr changed the title Upgrade slf4f to 2.0.3 Upgrade slf4j to 2.0.3 Oct 24, 2022
@tony19

This comment was marked as outdated.

@compscidr

This comment was marked as outdated.

@MNCloudwerksTechnology
Copy link

Hey @tony19,
If this branch works and compatibility with slf4j 2.0.3 is established, would mind merging this PR? We would really like to use this library again in our Android project.

Cheers

@MNCloudwerksTechnology
Copy link

MNCloudwerksTechnology commented Nov 3, 2022

Just tested the implementation with:
implementation 'com.github.compscidr:logback-android:slf4f-2.0.3-SNAPSHOT'

It still prints out:

2022-11-03 18:38:10.225 4584-4584/com.random.package W/System.err: SLF4J: No SLF4J providers were found. 2022-11-03 18:38:10.226 4584-4584/com.random.package W/System.err: SLF4J: Defaulting to no-operation (NOP) logger implementation 2022-11-03 18:38:10.226 4584-4584/com.random.package W/System.err: SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.

No logs in either appender. Seems to me, that the provider is still not loading properly. Was that part of your fix, @compscidr?

@compscidr
Copy link
Contributor Author

Just tested the implementation with: implementation 'com.github.compscidr:logback-android:slf4f-2.0.3-SNAPSHOT'

It still prints out:

2022-11-03 18:38:10.225 4584-4584/com.random.package W/System.err: SLF4J: No SLF4J providers were found. 2022-11-03 18:38:10.226 4584-4584/com.random.package W/System.err: SLF4J: Defaulting to no-operation (NOP) logger implementation 2022-11-03 18:38:10.226 4584-4584/com.random.package W/System.err: SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.

No logs in either appender. Seems to me, that the provider is still not loading properly. Was that part of your fix, @compscidr?

Hm that's weird, I have it working in a pretty complicated project atm - when I get a chance I'll see if I can make it work with a minimal example or something to verify there isn't some other interaction happening.

@MNCloudwerksTechnology
Copy link

MNCloudwerksTechnology commented Nov 3, 2022

Thanks that would be great.

Are you also using the following?

  • Target SDK 33
  • target & source-Compatibility VERSION_11

Also, thanks for your fast reply.

Cheers

@compscidr
Copy link
Contributor Author

Thanks that would be great.

Are you also using the following?

  • Target SDK 33
  • target & source-Compatibility VERSION_11

Also, thanks for your fast reply.

Cheers

Okay here's a pretty minimal sample project with it working: https://github.com/compscidr/hello-kotlin-android - it does use target sdk 33 and source-compat VERSION_11

This is what it output to androids logcat:

2022-11-04 14:44:13.107 22021-22021 c.e.m.MainActivity      com.example.myapplication            D  [main                ] logger test
2022-11-04 14:44:13.107 22021-22021 c.e.m.TestComponent     com.example.myapplication            D  [main                ] testComponent: someFunction()
2022-11-04 14:44:13.107 22021-22021 c.e.l.MyClass           com.example.myapplication            D  [main                ] In the display function!

And then it also logged to papertrail which I also often use alongside.

@tony19 tony19 added this to the 3.0.0 milestone Nov 5, 2022
@tony19 tony19 self-assigned this Nov 6, 2022
tony19
tony19 previously approved these changes Nov 20, 2022
@tony19
Copy link
Owner

tony19 commented Nov 20, 2022

Thanks for the updates! I'll merge this in 3.x, since it's technically a breaking change (as you pointed out).

@tony19 tony19 changed the title Upgrade slf4j to 2.0.3 feat!: upgrade slf4j to 2.0.3 Dec 4, 2022
@tony19 tony19 merged commit f7be379 into tony19:main Dec 4, 2022
tony19 added a commit that referenced this pull request Dec 4, 2022
This reverts commit f7be379, reversing
changes made to f09aeb6.
@tony19
Copy link
Owner

tony19 commented Dec 4, 2022

@compscidr For some reason, this change breaks the build. I'm not sure why the CI failure didn't mark the PR. I'm reverting your changes for now in #296.


Found the issue: slf4jVersion was re-defined at the app level, and was pointing to an old version. Addressed that problem in #297.

tony19 added a commit that referenced this pull request Dec 4, 2022
tony19 added a commit that referenced this pull request Dec 4, 2022
tony19 added a commit that referenced this pull request Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

No log is printed with Slf4j 2.x.x
4 participants