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: migrate to org.apache.logging.log4j:log4j-core:2.13.0 #3202

Merged
merged 5 commits into from
Jan 10, 2020

Conversation

nharrand
Copy link
Collaborator

@nharrand nharrand commented Jan 9, 2020

log4j:log4j is stuck to 1.2.7, which contains a security vulnerability.
The new artifact for log4j is now org.apache.logging.log4j:log4j-core.
The api has slightly changed for version 2.x, see this page for migration information.

@monperrus monperrus changed the title refactor: migrate to org.apache.logging.log4j:log4j-core:2.13.0 refactor: migrate to org.apache.logging.log4j:log4j-core:2.13.0 Jan 10, 2020
@monperrus monperrus merged commit 99775fc into INRIA:master Jan 10, 2020
@monperrus
Copy link
Collaborator

Thanks a lot!

@monperrus
Copy link
Collaborator

FYI, this change breaks some usages, starting with using Spoon in Groovy: https://travis-ci.org/SpoonLabs/spoon-ci-external/builds/635334772

It seems that it's because of dependency shadowing, but I'm not sure.

@monperrus
Copy link
Collaborator

Also breaks the Nopol build: https://ci.inria.fr/sos/job/nopol/719/display/redirect

log4j-core seems not usable as is at runtime.

@nharrand
Copy link
Collaborator Author

nharrand commented Jan 13, 2020

For Nopol:

It seems that these lines use classes from the old dependency, while not declaring it. (L59 could simply be replacde by factory.getEnvironment().setLevel("OFF");

https://github.com/SpoonLabs/nopol/blob/2bcc7dc685a1e66189363aa2a6be082868440106/nopol/src/main/java/fr/inria/lille/commons/spoon/SpoonedFile.java#L5
https://github.com/SpoonLabs/nopol/blob/2bcc7dc685a1e66189363aa2a6be082868440106/nopol/src/main/java/fr/inria/lille/commons/spoon/SpoonedFile.java#L59

Proposed fix SpoonLabs/nopol#193

(By the way, it is not so much that log4j-core is not usable at runtime, but rather that the package name changed from org.apache.log4j to org.apache.logging.log4j.)

@nharrand
Copy link
Collaborator Author

nharrand commented Jan 13, 2020

For Groovy, you might be right. I am not sure of what's happening. But maybe just updating log4j was not the smartest move. Maybe we should move to slf4j and avoid logger compatibility issues.
WDYT?

@monperrus
Copy link
Collaborator

monperrus commented Jan 13, 2020

I confirm that Nopol is fixed: https://ci.inria.fr/sos/job/nopol/722/

Thanks a lot!

@monperrus
Copy link
Collaborator

Maybe we should move to slf4j and avoid logger compatibility issues.

that's a great idea.

@monperrus
Copy link
Collaborator

really want to get Groovy working again

#3208 is an option

MartinWitt pushed a commit to MartinWitt/spoon that referenced this pull request Feb 11, 2020
@monperrus monperrus mentioned this pull request Mar 20, 2020
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.

2 participants