On Oct 31, 2008, at 9:09 PM, Adam Murdoch wrote:
Hi,
I really like how the stdout/err capture stuff looks. Some comments:
1. I wonder if we shouldn't always capture stdout and stderr, and
have captureStandardOutput() simply change the levels. Then, all
logging, whether done via a logging framework or stdout/stderr,
would always end up routed through logback, which would mean, for
example, that it would end up in the log file, or that a listener
could decorate it in some way.
You are right. At least it should be the default (and the users still
has the option to switch redirection off).
2. It would be good if the source of the logging were considered
when routing info/stdout messages.
For example, a call to logger.info() or println() in my
build.gradle is almost always interesting to me when I run Gradle,
and should be printed on the console when I run with default
options (ie should be treated as a lifecycle message).
Right.
However, a similar call in a java library used by an ant task used
by a plugin is not interesting to me at all, and should not be
shown when I run with default options (ie should be treated as an
info message). There's a spectrum of interestingness going on here,
and I'm not sure how we decide whether an info/stdout message is
interesting enough to treat as a lifecycle message.
The same thing probably applies to warning and error messages. Some
warnings are just noise to me when I run Gradle, and some are
interesting.
This is pretty task specific I guess. For example one could argue
that the groovyc output should be send to DEBUG (I have chosen INFO
for now). Or external code writes errors or warning not to System.err
but System.out. Or vice versa (see the Ant code: http://
www.krugle.org/kse/codespaces/BLCNL5 Line: 153). Ant is usually no
problem for us, as we inject our own AntLoggingAdapter. But sometimes
this injection is hard or impossible to achieve (See for example:
http://svn.codehaus.org/gradle/gradle-core/trunk/src/main/groovy/org/
gradle/api/internal/dependencies/maven/deploy/
DeployTaskWithVisibleContainerProperty.java)
Another issue is, that people will be often not aware of this
redirection. So they write a custom task and put some debug println
into it and, depending on our defaults, could have a very hard time
to figure out why they don't see it.
There is also the use case that you only want redirections for a
certain part of your code. For example the MavenUploader delegates to
the maven-ant-tasks (we can redirect the ant logging). But those
tasks use some Maven stuff which uses the logging of the Plexus
container (via a very modern inheritance based approach ;)). So we
have to catch standard out for the delegation call (See: http://
svn.codehaus.org/gradle/gradle-core/trunk/src/main/groovy/org/gradle/
api/internal/dependencies/maven/deploy/BaseMavenUploader.java method:
execute).
All in all I would say the default for task execution should be to
send System.out to HIGHLEVEL and System.err to ERROR. The Java Plugin
then knows about the behavior of specific tasks and can define less
verbose rules for certain tasks. And the user can always have the
last word.
3. Task.captureStandardOutput() and Project.captureStandardOutput()
have inconsistent semantics. For Task, a call to
captureStandardOutput() declares 'whenever I execute, capture
standard output'. For Project, a call to captureStandardOutput()
says 'capture standard output now until forever'.
I prefer the Task semantics. I think Project.captureStandardOutput
() should behave in a similar way. It should declare that when the
project is evaluated, standard output should be captured. If
captureStandardOutput() happens to be called as the project is
being evaluated, then is should take effect immediately. And when
evaluation completes, it should restore to what it was when
evaluation started.
This means, for example, that the logging for a project can be
configured from somewhere else (a parent project, say) without
affecting the logging of the configurer. It also means that the
logging for a project does not affect the logging of other projects
which are implicitly evaluated during the evaluation of the
project, eg by a call to dependsOn().
Good point. I haven't thought about multi-project builds and that
there is the configuration and execution phase. What about calling
project.captureStandardOutput from a task during execution time.
Should we accept this as a project wide global switch as well?
We also have to provide captureStandardOutput also to the settings
evaluation.
4. StandardOutputLoggingAdapter is not thread safe. Although Gradle
isn't multithreaded (yet), there's plenty of potential for things
to execute which are, and which assume it is safe to use System.out
from multiple threads. We really should make it thread safe. A
buffer per-thread would be kinda nice, but a big fat lock around
each method would do the trick too.
Good point. I have been thinking about that and though that we are
not multi-threaded yet, so this is not an issue. But I have forgotten
about StandardOutputLoggingAdapter. I'm not sure if a lock will do
the job here as this is a buffered writer. If different threads for
example use print additional to println, a logging line might have
mixed content from different threads. So I guess we have to use a
thread-local here.
- Hans
--
Hans Dockter
Gradle Project lead
http://www.gradle.org
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
