SemanticApplicationDesignLanguage / sadl

Semantic Application Design Language (SADL) Open Source Code
http://semanticapplicationdesignlanguage.github.io/sadl/
Eclipse Public License 1.0
30 stars 12 forks source link

log4j usage in activators #487

Open crapo opened 4 years ago

crapo commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 (https://github.com/crapo/sadlos2/commit/7ab50d4e0a6c5c72c9b1fb114621c4a765ebf06e).

crapo commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 https://github.com/crapo/sadlos2/issues/486#issuecomment-664823231

What is missing for this issue?

crapo commented 4 years ago

@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.