Bugzilla – Full Text Bug Listing
| Summary: | [xtext][logging] log appender no longer logs to eclipse error log | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Knut Wannheden <knut.wannheden> | ||||||
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | P3 | CC: | sebastian.zarnekow | ||||||
| Version: | 2.1.0 | Flags: | sebastian.zarnekow:
juno+
|
||||||
| Target Milestone: | M4 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Knut Wannheden
There is a bug in EclipseLogAppender which has the effect that no logging events are written to the Eclipse error log. Fix candidate for maintenance branch. The problem is the new guard in
private boolean isDoLog(Level level) {
return log!=null && (level.toInt() >= Priority.WARN_INT);
}
since the field _log_ is lazily initialized isDoLog will always return false thus never initialize the field.
Created attachment 206760 [details]
proposed patch
Patch looks good, please apply. One minor thing: Iff the bundle is not available, the error message will be repeated over and over again. If the bundle is not available an IllegalStateException would be thrown (every time) which would end up being logged as a warning to the error log. Additionally the build (in case this occurs during a build) will be marked as unsuccessful as an exception was thrown. Wouldn't it be better if the EclipseLogAppender would log a single error to the Eclipse error log in case the bundle isn't found? I.e. not throw an exception. But this is probably not a real issue anyways as org.eclipse.xtext.logging is a fragment with org.apache.log4j[1.2.15,1.2.16) as its host. So I assume this code would never run (in an OSGi environment) if the log4j bundle with version 1.2.15 is not available. (In reply to comment #6) > If the bundle is not available an IllegalStateException would be thrown (every > time) which would end up being logged as a warning to the error log. > Additionally the build (in case this occurs during a build) will be marked as > unsuccessful as an exception was thrown. > > Wouldn't it be better if the EclipseLogAppender would log a single error to the > Eclipse error log in case the bundle isn't found? I.e. not throw an exception. Agreed. > > But this is probably not a real issue anyways as org.eclipse.xtext.logging is a > fragment with org.apache.log4j[1.2.15,1.2.16) as its host. So I assume this > code would never run (in an OSGi environment) if the log4j bundle with version > 1.2.15 is not available. I stumbled accross this situation once: It was a plain Java process and the log4.properties from xtext.logging was used thus the EclipseLogAppender was intantiated but did not work. However, this is not a big deal. I will commit the patch as proposed to 2.1.x_Maintenance and work out something better for master (2.2). +1 Created attachment 206778 [details]
proposed patch for xtext 2.2
With the proposed patch Platform#isRunning() will be called once only to check if executing in OSGi or not. This makes the explicit bundle version check redundant as that is already covered by the MANIFEST.MF. Also the appender will no longer itself log to stdout as that should be covered by the default appender.
(In reply to comment #10) > Created attachment 206778 [details] > proposed patch for xtext 2.2 +1, looks good pushed first proposal to 2.1.x_Maintenance and second proposal to master. Thanks! Closing all bugs that were set to RESOLVED before Neon.0 Closing all bugs that were set to RESOLVED before Neon.0 |