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

log4j usage in activators #487

Open
crapo opened this issue Jul 23, 2020 · 7 comments
Open

log4j usage in activators #487

crapo opened this issue Jul 23, 2020 · 7 comments

Comments

@crapo
Copy link
Collaborator

crapo commented Jul 23, 2020

@kittaakos , Jena has changed how it does its logging between 2.x and 3.x. It used to use slf4j with a log4j12 backend, now it does slf4j with a log4j2 backend. Consequently we have removed org.apache.log4j.Logger from our code. However, it appears automatically in the generated activators, e.g., SadlActivator. We are wondering if we should add log4j to the ui Bundle-ClassPath or add it to the third party jars in com.ge.research.jena. What is your advice?

@kittaakos
Copy link
Contributor

We are wondering if we should add log4j to the ui Bundle-ClassPath or add it to the third party jars in com.ge.research.jena.

Why is it required to change anything wrt the logging? Did the Jena update break anything?

if we should add log4j to the ui Bundle-ClassPath

How do you want to add log4j to the classpath? Do you already have a branch with such changes?

As far as I know, the preferred Xtext logging way is to contribute a fragment to org.apache.log4j and make sure that no other (logger) fragments exist in the application. Then you can have org.apache.log4j as a required-bundle in both core and UI plug-ins.

@crapo
Copy link
Collaborator Author

crapo commented Jul 27, 2020

The Jena update seemed to break a couple of things. It was difficult to resolve all of the compiler issues. JenaBasedSadlModelProcessor failed to be resolve even though other classes in the same package did resolve. That proved to be related to logging issues. Everything has been merged to development as all tests pass. However, the current situation is that while content assist works when running from the Plugin Development Perspective, once the update zip is installed in Eclipse content assist does not work. Currently there is an audible sound when trying to do content assist. See #486 .

@crapo
Copy link
Collaborator Author

crapo commented Jul 27, 2020

@kittaakos , to be more specific, in order to get JenaBasedSadlModelProcessor to resolve, I had to comment out the line "private static final Logger logger = Logger.getLogger(SadlmodelProcessor.class)" in SadlModelProcessor.java, along with the associated import of org.apache.log4j.Logger. These changes were replaced by using org.slf4j classes for logging. @tuxji made some changes to loggin in #481 . Some logging changes were made to test cases to get things to compile (7ab50d4).

@crapo
Copy link
Collaborator Author

crapo commented Jul 27, 2020

There was an update build in there in which content assist worked, which lead me to close #486. Then it stopped. Maybe that coincides with replacing rather than commenting out the logging in SadlModelProcessor. Investigating now.

@kittaakos
Copy link
Contributor

There was an update build

Do you know which one was the last commit that worked after installing the SADL p2 into 2020-03?

Investigating now.

OK, please ping me if I have to take it over.

@kittaakos
Copy link
Contributor

However, the current situation is that while content assist works when running from the Plugin Development Perspective, once the update zip is installed in Eclipse content assist does not wor

It works for me. See #486 (comment)

What is missing for this issue?

@crapo
Copy link
Collaborator Author

crapo commented Jul 28, 2020

@kittaakos , the content assist issue is resolved. I can install the latest SADL p2 into an Eclipse IDE for Java Developers 2020-03 and CA works.
The only outstanding question is have we done something with the various logging systems what looks like it might be an issue with the Xtext logging. As soon as you give your approval of the logging I'll close this.

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

No branches or pull requests

2 participants