Skip to content

Commit

Permalink
Improved logging infrastructure and plugged Log4J in the scala-ide.sd…
Browse files Browse the repository at this point in the history
…t.core

project.

We used to have too many alternatives to create log items (Eclipse Log
framework, our ad-hoc naive internal Logger and println statements originating
from scalac). In short, we did not have an ideal logging infrastructure.

Now, I plugged Log4J as the backend logger (I'll refer to it as the "default
logger" in the rest of this commit message), which replaces our naive ad-hoc
Logger. Further, all log events occurring in the Eclipse Log Framework are now
*forwarded* to the default logger. Also, the Standard Output and Standard Error
are *redirected* to the default logger.  The  goal is to have a single place
to consult the log, and that is now the *scala-ide.log* file that is produced
by the default logger, located in

  ${workspace}/.metadata/.plugin/org.scala-ide.sdt.core/

It's easy to consult the log from within Eclipse, just open the Eclipse
Preferences, then Scala > Logging, and there is a link to open the
*scala-ide.log* file in the editor (though, mind that it is not automatically
refreshed while it's open).

From the same preference's page, you can also control the amount of produced
log, i.e., you can set the log level that better suits you (the default level
is WARNING).  If needed, you can also enable a console appender to print all
produced log items in the console (this is quite handy when doing development
on the Scala IDE sources).

To get a handle on the loggers you simply need to mix-in the HasLogger trait,
which contains a reference to both the default logger and the Eclipse Log. You
may wonder why we have two loggers. The reason is simple, if you want to
communicate a message to the user, you should use the Eclipse Log. This becayse
messages sent to the Eclipse Log are shown in the Eclipse Log View (remember
that all messages sent to the Eclipse Log are also forwarded to the default
logger, so you don't need to log the same message twice). In all other cases,
you should use th default logger.

Finally, I removed the "plugininfo" option, which it was used to enable debug
information in the ScalaIndexBuilder. As a matter of fact, the amount of logged
information can now be easily controlled through Log Levels.

One element I'm not happy with is that I plugged Log4J within the
scala-ide.sdt.core project and not as an external plug-in. I've tried to do so
but failed due to my lack of understanding of OSGi.  This is something that I
believe we should do at some point (maybe someone can help with this?!).

Fixes #1000880.
  • Loading branch information
dotta committed Feb 3, 2012
1 parent f266bcc commit 0ff7e44
Show file tree
Hide file tree
Showing 56 changed files with 698 additions and 190 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package pc
import scala.tools.eclipse.javaelements.ScalaCompilationUnit
import scala.tools.eclipse.javaelements.ScalaSourceFile
import scala.tools.eclipse.testsetup.SDTTestUtils
import scala.tools.eclipse.util.Logger
import scala.tools.eclipse.logging.Logger
import scala.tools.nsc.interactive.InteractiveReporter
import org.eclipse.jdt.core.ICompilationUnit
import org.junit.Assert._
Expand All @@ -31,7 +31,7 @@ class PresentationCompilerTest {
compiler.withStructure(sourceFile, keepLoaded = true) { tree =>
compiler.askOption { () =>
val overrideIndicatorBuilder = new compiler.OverrideIndicatorBuilderTraverser(unit, new java.util.HashMap) {
override val logger = mockLogger
override val eclipseLog = mockLogger
}
// if the unit is not kept loaded (i.e., `keepLoaded = false`), then a message
// "Error creating override indicators" is reported. That is why this test checks
Expand All @@ -43,7 +43,7 @@ class PresentationCompilerTest {
}()

// verify
verify(mockLogger, times(0)).error(any(), any())
verify(mockLogger, times(0)).error(any())
}

@Test
Expand Down
1 change: 1 addition & 0 deletions org.scala-ide.sdt.core/.classpath
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
<classpathentry kind="con" path="org.scala-ide.sdt.launching.SCALA_CONTAINER"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="lib" path="lib/miglayout-3.7.4.jar"/>
<classpathentry kind="lib" path="lib/log4j-1.2.16.jar" sourcepath="lib/log4j-1.2.16-sources.jar"/>
<classpathentry kind="output" path="target/classes"/>
</classpath>
3 changes: 2 additions & 1 deletion org.scala-ide.sdt.core/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,6 @@ Export-Package:
scala.tools.eclipse.wizards
Bundle-RequiredExecutionEnvironment: J2SE-1.5
Bundle-ClassPath: .,
lib/miglayout-3.7.4.jar
lib/miglayout-3.7.4.jar,
lib/log4j-1.2.16.jar

9 changes: 9 additions & 0 deletions org.scala-ide.sdt.core/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@
class="scala.tools.eclipse.properties.ImplicitsPreferencePage"
id="org.scala-ide.sdt.core.properties.Implicits"
name="Implicits">
</page>
<page
category="org.scala-ide.sdt.core.preferences"
class="scala.tools.eclipse.logging.ui.properties.LoggingPreferencePage"
id="org.scala-ide.sdt.core.properties.Logging"
name="Logging">
</page>
</extension>

Expand Down Expand Up @@ -745,6 +751,9 @@
<initializer
class="scala.tools.eclipse.properties.ImplicitsPagePreferenceInitializer">
</initializer>
<initializer
class="scala.tools.eclipse.logging.ui.properties.LoggingPreferencePageInitializer">
</initializer>
</extension>

<extension point="org.eclipse.core.expressions.propertyTesters">
Expand Down
38 changes: 38 additions & 0 deletions org.scala-ide.sdt.core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

<properties>
<miglayout.version>3.7.4</miglayout.version>
<log4j.version>1.2.16</log4j.version>
</properties>

<dependencies>
Expand All @@ -23,6 +24,30 @@
<type>jar</type>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<version>${log4j.version}</version>
<scope>compile</scope>
<exclusions>
<exclusion>
<groupId>javax.mail</groupId>
<artifactId>mail</artifactId>
</exclusion>
<exclusion>
<groupId>javax.jms</groupId>
<artifactId>jms</artifactId>
</exclusion>
<exclusion>
<groupId>com.sun.jdmk</groupId>
<artifactId>jmxtools</artifactId>
</exclusion>
<exclusion>
<groupId>com.sun.jmx</groupId>
<artifactId>jmxri</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<build>
Expand Down Expand Up @@ -96,6 +121,19 @@
</goals>
<configuration>
<artifactItems>
<artifactItem>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<version>${log4j.version}</version>
<outputDirectory>${basedir}/lib</outputDirectory>
</artifactItem>
<artifactItem>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<version>${log4j.version}</version>
<outputDirectory>${basedir}/lib</outputDirectory>
<classifier>sources</classifier>
</artifactItem>
<artifactItem>
<groupId>com.miglayout</groupId>
<artifactId>miglayout</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import org.eclipse.jdt.internal.core.JavaModelManager
import org.eclipse.jdt.internal.core.builder.{ JavaBuilder, State }
import scala.tools.eclipse.javaelements.JDTUtils
import scala.tools.eclipse.util.{ FileUtils, ReflectionUtils }
import util.HasLogger
import scala.tools.eclipse.logging.HasLogger
import scala.tools.nsc.interactive.RefinedBuildManager

class ScalaBuilder extends IncrementalProjectBuilder with HasLogger {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import org.eclipse.jdt.internal.ui.javaeditor.{ EditorUtility, JavaElementHyperl
import org.eclipse.jdt.ui.actions.OpenAction
import org.eclipse.jdt.internal.ui.javaeditor.JavaEditor
import javaelements.{ ScalaCompilationUnit, ScalaSelectionEngine, ScalaSelectionRequestor }
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

class ScalaHyperlinkDetector extends AbstractHyperlinkDetector with HasLogger {
def detectHyperlinks(viewer: ITextViewer, region: IRegion, canShowMultipleHyperlinks: Boolean): Array[IHyperlink] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ import scala.tools.eclipse.util.OSGiUtils.pathInBundle
import scala.tools.eclipse.templates.ScalaTemplateManager
import org.eclipse.jdt.ui.PreferenceConstants
import org.eclipse.core.resources.IResourceDelta
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger
import org.osgi.framework.Bundle
import scala.tools.eclipse.util.Utils
import org.eclipse.jdt.core.ICompilationUnit
import scala.tools.nsc.io.AbstractFile
import scala.tools.eclipse.util.EclipseResource
import scala.tools.eclipse.logging.PluginLogConfigurator

object ScalaPlugin {
var plugin: ScalaPlugin = _
Expand All @@ -46,7 +47,7 @@ object ScalaPlugin {
def getShell: Shell = getWorkbenchWindow map (_.getShell) orNull
}

class ScalaPlugin extends AbstractUIPlugin with IResourceChangeListener with IElementChangedListener with IPartListener with HasLogger {
class ScalaPlugin extends AbstractUIPlugin with PluginLogConfigurator with IResourceChangeListener with IElementChangedListener with IPartListener with HasLogger {
ScalaPlugin.plugin = this

final val HEADLESS_TEST = "sdtcore.headless"
Expand Down Expand Up @@ -135,7 +136,7 @@ class ScalaPlugin extends AbstractUIPlugin with IResourceChangeListener with IEl
val bundles = Option(Platform.getBundles(ScalaPlugin.plugin.libraryPluginId, null)).getOrElse(Array[Bundle]())
logger.debug("[scalaLibBundle] Found %d bundles: %s".format(bundles.size, bundles.toList.mkString(", ")))
bundles.find(b => b.getVersion().getMajor() == scalaCompilerBundleVersion.getMajor() && b.getVersion().getMinor() == scalaCompilerBundleVersion.getMinor()).getOrElse {
logger.error("Could not find a match for %s in %s. Using default.".format(scalaCompilerBundleVersion, bundles.toList.mkString(", ")), null)
eclipseLog.error("Could not find a match for %s in %s. Using default.".format(scalaCompilerBundleVersion, bundles.toList.mkString(", ")), null)
Platform.getBundle(ScalaPlugin.plugin.libraryPluginId)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import scala.tools.nsc.util.{ BatchSourceFile, Position, SourceFile }
import scala.tools.eclipse.javaelements.{
ScalaCompilationUnit, ScalaIndexBuilder, ScalaJavaMapper, ScalaMatchLocator, ScalaStructureBuilder,
ScalaOverrideIndicatorBuilder }
import scala.tools.eclipse.util.{ Cached, EclipseFile, EclipseResource, HasLogger }
import scala.tools.eclipse.util.{ Cached, EclipseFile, EclipseResource }
import scala.tools.eclipse.logging.HasLogger
import scala.tools.nsc.util.FailedInterrupt
import scala.tools.nsc.symtab.Flags
import scala.tools.eclipse.completion.CompletionProposal
Expand Down Expand Up @@ -137,11 +138,11 @@ class ScalaPresentationCompiler(project : ScalaProject, settings : Settings)
None

case e =>
logger.error("Error during askOption", e)
eclipseLog.error("Error during askOption", e)
None
}
case e =>
logger.error("Error during askOption", e)
eclipseLog.error("Error during askOption", e)
None
}

Expand Down Expand Up @@ -197,7 +198,7 @@ class ScalaPresentationCompiler(project : ScalaProject, settings : Settings)
}

override def logError(msg : String, t : Throwable) =
logger.error(msg, t)
eclipseLog.error(msg, t)

def destroy() {
logger.info("shutting down presentation compiler on project: " + project)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import scala.tools.eclipse.properties.IDESettings
import util.SWTUtils.asyncExec
import EclipseUtils.workspaceRunnableIn
import scala.tools.eclipse.properties.CompilerSettings
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger
import scala.collection.mutable.ListBuffer
import scala.actors.Actor
import org.eclipse.jdt.core.IJarEntryResource
Expand Down Expand Up @@ -75,14 +75,14 @@ class ScalaProject private (val underlying: IProject) extends HasLogger {
} catch {
case ex @ MissingRequirementError(required) =>
failedCompilerInitialization("could not find a required class: " + required)
logger.error(ex)
eclipseLog.error(ex)
None
case ex =>
logger.info("Throwable when intializing presentation compiler!!! " + ex.getMessage)
ex.printStackTrace()
if (underlying.isOpen)
failedCompilerInitialization("error initializing Scala compiler")
logger.error(ex)
eclipseLog.error(ex)
None
}
}
Expand Down Expand Up @@ -227,7 +227,7 @@ class ScalaProject private (val underlying: IProject) extends HasLogger {
}

case _ =>
logger.warning("Classpath computation encountered unknown entry: " + cpe)
logger.warn("Classpath computation encountered unknown entry: " + cpe)
}
}
computeClasspath(javaProject, false)
Expand Down Expand Up @@ -361,7 +361,7 @@ class ScalaProject private (val underlying: IProject) extends HasLogger {
try {
cntnr.delete(true, monitor) // try again
} catch {
case t => logger.error(t)
case t => eclipseLog.error(t)
}
}
} else
Expand All @@ -370,7 +370,7 @@ class ScalaProject private (val underlying: IProject) extends HasLogger {
try {
file.delete(true, monitor)
} catch {
case t => logger.error(t)
case t => eclipseLog.error(t)
}
case _ =>
}
Expand Down Expand Up @@ -565,7 +565,7 @@ class ScalaProject private (val underlying: IProject) extends HasLogger {
setting.tryToSetFromPropertyValue(value)
}
} catch {
case t: Throwable => logger.error("Unable to set setting '" + setting.name + "' to '" + value0 + "'", t)
case t: Throwable => eclipseLog.error("Unable to set setting '" + setting.name + "' to '" + value0 + "'", t)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import org.eclipse.jdt.core.search.SearchDocument
import org.eclipse.jdt.internal.core.search.indexing.AbstractIndexer
import scala.tools.eclipse.javaelements.ScalaSourceFile
import scala.tools.eclipse.contribution.weaving.jdt.indexerprovider.IIndexerFactory
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

class ScalaSourceIndexerFactory extends IIndexerFactory {
override def createIndexer(document : SearchDocument) = new ScalaSourceIndexer(document);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import org.eclipse.ui.IWorkbenchPart
import scala.tools.eclipse.util.ReflectionUtils
import scala.tools.eclipse.javaelements.ScalaClassElement
import scala.tools.eclipse.javaelements.ScalaSourceTypeElement
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

class ScalaToggleBreakpointAdapter extends ToggleBreakpointAdapter with HasLogger { self =>
import ScalaToggleBreakpointAdapterUtils._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import scala.tools.eclipse.{EclipseBuildManager, TaskScanner, ScalaProject}
import scala.tools.nsc.Settings
import scala.tools.nsc.reporters.Reporter
import scala.tools.nsc.util.{ Position, NoPosition }
import scala.tools.eclipse.util.{ EclipseResource, FileUtils, HasLogger }
import scala.tools.eclipse.util.{ EclipseResource, FileUtils}
import scala.tools.eclipse.logging.HasLogger

import scala.collection.mutable.ListBuffer

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import scala.tools.nsc.io.AbstractFile
import scala.tools.nsc.reporters.Reporter
import scala.tools.eclipse.util.{ EclipseResource, FileUtils }
import org.eclipse.core.runtime.{ SubMonitor, IPath, Path }
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

class EclipseRefinedBuildManager(project: ScalaProject, settings0: Settings)
extends RefinedBuildManager(settings0) with EclipseBuildManager with HasLogger {
Expand Down Expand Up @@ -82,7 +82,7 @@ class EclipseRefinedBuildManager(project: ScalaProject, settings0: Settings)
case e =>
hasErrors = true
project.buildError(IMarker.SEVERITY_ERROR, "Error in Scala compiler: " + e.getMessage, null)
logger.error("Error in Scala compiler", e)
eclipseLog.error("Error in Scala compiler", e)
}
if (!hasErrors)
pendingSources.clear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import scala.tools.eclipse.util.EclipseResource
import java.io.File
import org.eclipse.jdt.launching.JavaRuntime
import org.eclipse.jdt.core.{ JavaCore, IJavaProject }
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger
import scala.tools.eclipse.contribution.weaving.jdt.jcompiler.BuildManagerStore

class AnalysisCompile (conf: BasicConfiguration, bm: EclipseSbtBuildManager, contr: Controller) extends HasLogger {
Expand Down Expand Up @@ -195,7 +195,7 @@ class AnalysisCompile (conf: BasicConfiguration, bm: EclipseSbtBuildManager, con
null

case ex =>
logger.error("Crash in the build compiler.", ex)
eclipseLog.error("Crash in the build compiler.", ex)
reporter.log(SbtConverter.convertToSbt(NoPosition), "The SBT builder crashed while compiling your project. This is a bug in the Scala compiler or SBT. Check the Erorr Log for details. The error message is: " + ex.getMessage(), xsbti.Severity.Error)
null

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import sbt.compiler.{JavaCompiler}
import sbt.{Process, ClasspathOptions}
import scala.tools.eclipse.util.{ EclipseResource, FileUtils }
import org.eclipse.core.resources.IResource
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

// The following code is based on sbt.AggressiveCompile
// Copyright 2010 Mark Harrah
Expand Down Expand Up @@ -340,7 +340,7 @@ class EclipseSbtBuildManager(val project: ScalaProject, settings0: Settings)
//ScalaPlugin.plugin.sbtScalaLib.get.toFile
val e = new Exception("Cannot find Scala library on the classpath. Verify your build path!")
project.buildError(IMarker.SEVERITY_ERROR, e.getMessage(), null)
logger.error("Error in Scala SBT builder", e)
eclipseLog.error("Error in Scala SBT builder", e)
return
}
//val compJar = ScalaPlugin.plugin.sbtScalaCompiler
Expand Down Expand Up @@ -391,7 +391,7 @@ class EclipseSbtBuildManager(val project: ScalaProject, settings0: Settings)
case e =>
hasErrors = true
project.buildError(IMarker.SEVERITY_ERROR, "Error in Scala compiler: " + e.getMessage, null)
logger.error("Error in Scala compiler", e)
eclipseLog.error("Error in Scala compiler", e)
}
if (!hasErrors)
pendingSources.clear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import sbt.{ ScalaInstance, Path }
import xsbt.boot.{Launcher, Repository }
import java.io.File
import org.eclipse.core.resources.ResourcesPlugin
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

object ScalaCompilerConf {
val LIBRARY_SUFFIX = "scala-library.jar"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import org.eclipse.jdt.core.search.{SearchEngine, IJavaSearchConstants, SearchPa
import org.eclipse.jdt.core.IJavaElement
import scala.collection.mutable
import org.eclipse.core.runtime.NullProgressMonitor
import scala.tools.eclipse.util.HasLogger
import scala.tools.eclipse.logging.HasLogger

/** Base class for Scala completions. No UI dependency, can be safely used in a
* headless testing environment.
Expand Down
Loading

0 comments on commit 0ff7e44

Please sign in to comment.