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

refactor: use slf4j API instead of log4j-api #3755

Merged
merged 5 commits into from
Jan 19, 2021

Conversation

andrewbwogi
Copy link
Contributor

@andrewbwogi andrewbwogi commented Jan 16, 2021

closes #3686

@MartinWitt
Copy link
Collaborator

Nice work! 2 Points from me:

  • How about replacing the Logger initialization with private static final Logger LOGGER = Logger.getLogger(MethodHandles.lookup().lookupClass());. If we have a convention for using this, no one starts to use the Logger.getLogger(String) with non class names.
  • Replace the star imports with there correct imports?

@monperrus
Copy link
Collaborator

Really nice, thanks a lot @andrewbwogi LGTM.

Why does CI fail?

@MartinWitt
Copy link
Collaborator

MartinWitt commented Jan 18, 2021

3 Checkstyle errors

Failed during checkstyle execution: There are 3 errors reported by Checkstyle 8.29 with checkstyle.xml ruleset. -> [Help 1]

@monperrus
Copy link
Collaborator

Great, thanks, will merge tomorrow.

@monperrus monperrus changed the title refactor: use slf4j instead of log4j-api refactor: use slf4j API instead of log4j-api Jan 19, 2021
@monperrus monperrus merged commit cd339e2 into INRIA:master Jan 19, 2021
@monperrus
Copy link
Collaborator

Thanks a lot @andrewbwogi, that's very valuable.

@seintur
Copy link
Contributor

seintur commented Jan 19, 2021

Hi all,

Since this PR has been merged, it seems that several projects that use Spoon fail with an java.lang.NoClassDefFoundError: org/slf4j/event/Level

For those with access to the Inria CI, see for instance Casper (the same for Juliac although since the log is much larger, the failure is less obvious to spot). Certainly an issue with slf4j then. Any idea or pointer to the solution?

@MartinWitt
Copy link
Collaborator

By merge there were problems in our CI see https://github.com/INRIA/spoon/runs/1726213633. At first glance it looks like a race condition in ParallelProcessor on windows(argh)

@monperrus
Copy link
Collaborator

monperrus commented Jan 20, 2021 via email

@MartinWitt
Copy link
Collaborator

MartinWitt commented Jan 20, 2021

Can someone try to build a failing product with the following steps?

  1. add https://mvnrepository.com/artifact/org.slf4j/slf4j-simple to spoon
  2. rebuild spoon
  3. rebuild $project with the local built spoon?

@slarse
Copy link
Collaborator

slarse commented Jan 20, 2021

@monperrus
Copy link
Collaborator

org/slf4j/event/Level comes from slf4j-api-1.7.30.jar (verified), which is included in this PR.

How come it would it not be found by clients?

That may be due to those jobs being executed with JDK8.

@seintur
Copy link
Contributor

seintur commented Jan 20, 2021

org/slf4j/event/Level comes from slf4j-api-1.7.30.jar (verified), which is included in this PR.

How come it would it not be found by clients?

The weirdness is that this not a ClassNotFoundException but a NoClassDefFoundError. AFAIK this means that the class was present at compile-time but no longer at load/run-time.

@slarse
Copy link
Collaborator

slarse commented Jan 29, 2021

The weirdness is that this not a ClassNotFoundException but a NoClassDefFoundError. AFAIK this means that the class was present at compile-time but no longer at load/run-time.

That's correct. It's not all that strange, really, just means that for some reason the required class is not present on the classpath at runtime.

Now every other build is failing with this error message when running Spoon's Maven plugin:

Failed to execute goal fr.inria.gforge.spoon:spoon-maven-plugin:3.1:generate (default) on project : Execution default of goal fr.inria.gforge.spoon:spoon-maven-plugin:3.1:generate failed: A required class was missing while executing fr.inria.gforge.spoon:spoon-maven-plugin:3.1:generate: org/slf4j/event/Level

Going to try to start replicating some errors locally and see if I can figure it out.

@slarse
Copy link
Collaborator

slarse commented Jan 29, 2021

Can't replicate on my system. I'll try spinning up a few Ubuntu VMs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: move from Log4j API to SLF4J API
5 participants