web apps has a similar use case - we generate java sources for jsps
(jsp compilation) and when there is an error we want to show the error
in jsp source

Note that good build scripts would report the correct source file to
begin with; this is a request for a workaround for build scripts that
cannot do so due to inadequate task support.

*** Issue 42653 has been marked as a duplicate of this issue. ***

Cf. also use case in issue #42653.

In issue 42653 (of which this is a duplicate) Jesse writes:

> The Ant module has hardcoded support for parsing *any*
> error message in the normal error message format, i.e.
> 
> <absolute-file-path>[:<line1>[:<col1>[:<line2>:<col2>]]][:<message>]
> 
> which applies to far more things than Java compilation. Any Ant task
> which produces error messages is expected to use this format, or the
> task is buggy.

I investigated the possibility of producing full path names in the
Tomcat JSP compiler, but unfortunately this won't be possible. See the
reply I got from Jan.Luehe@sun.com :

-----------
Hi Petr,

[...]

The reason we only provide the context-relative path in the error
message is because when compiling JSPs via the JspServlet in a real
deployment, you don't want Jasper to expose the filesystem structure
of the server (by providing JSP's absolute path). Doing so would be
a security hole.

I would be reluctant to changing Jasper to differentiate between
JspC and JspServlet modes, and exposing full pathname in one but
not the other mode.

[...]
--------

So I am investigating the possibility to get the web document base
some other way.

The following will be sufficient to address the use case described in
issue 42653:

Modules will be able to provide a hook that will be called by the Ant
build support. This class will be able to maintain state collected
between individual invocations of the hook. The hook will be called on
special build events (inspired by org.apache.tools.ant.BuildListener)
and when writing a line to the output window. In the latter case, the
hook will be able to return a (equivalent of) AntOutputParser.Result.

So specifically, for 42653, the WebApps hook will
- listen on taskStarted for the JspC task, and remember its 'uriroot'
attribute, which contains the docbase.
- remember the uriroot in its state
- listen on build messages. When a message comes that matches the JSP
compiler error regex pattern, it will use the remembered value of
uriroot to construct the absolute JSP page location, and return a
AntOutputParser.Result with the link to the source.

Jesse, how does this sound to you? Were you thinking of something like
this? Or did you have a different design in mind? Thanks.

I am also attaching a BuildListener that I successfully tested with
the JSP compiler task.

Created attachment 14819 [details]
BuildListener file for JspC

Yes, I was thinking something along these lines. Need to do some work
to make it a reasonable & maintainable API, and also to make it scale
well with a number of listeners (should preferably avoid calling every
listener on every single Ant event). Also need to write a
proof-of-concept impl for stack trace hyperlinking. Hope to have such
an API this week.

I'm working on this, but it is subtle... Ant provides the BuildLogger
interface which is useful, but there are complexities involving
subbuilds (<ant>, <subant>, etc.), <import>s, tasks outside targets,
debug messages logged before a project is fully configured,
<parallel>, etc.

Also to be scalable with a larger number of listening plugins it is
probably necessary to permit each plugin to specify some kind of mask
so as to be notified only of "relevant" events, e.g. only events
coming from certain scripts, or from certain tasks, etc.

More to come...

Created attachment 14924 [details]
Sketch of proposed SPI (ZIP file)

Please take a look at the attached ZIP of SPI sources (there is no
implementation yet). Read the Javadoc carefully since there are a
number of subtle points. I would like to produce a prototype
implementation, and if that goes well, submit this for fast-track review.

I believe this SPI would handle the following use cases:

- Permitting some project types or task providers to override
hyperlink handling, including the ability to hyperlink log messages
based on stateful information derived from runtime task configuration.
(As requested by J2ME & Web Apps.)

- Permit stack trace hyperlinking to be moved out of the Ant module
into a Java-specific module, including the special trick of monitoring
the classpath of executed applications. (Highly desirable for
architectural separation reasons.)

- Permit project types to override the display of messages coming from
build-impl.xml files so as to provide simpler or friendlier formatting
of output, while retaining full display of messages coming from
customized build.xml scripts or ungenerated scripts. (Desired for
stock Ant-based project types, according to usability study results.)

- Permit Ant script debugging functionality. (Potential future
feature; no concrete plans yet.)

The proposed API does not provide any means of replacing the
org.openide.io-based output with e.g. a tree view or tasklist display,
though I believe it is compatible with such a feature if desired in
the future. (Use of OutputListener should be OK, I think - you can
fake OutputEvents coming from other GUIs AFAIK.) That feature would
also require some SPI for supplying an Ant output display mechanism,
and perhaps also some kind of hierarchy support for nesting output.

Note that the SPI does not require client modules to link against
ant.jar at all; it abstracts all Ant runtime information into some
kind of safer form. This should also help insulate clients from
possible changes in Ant's runtime semantics or API signatures in a
future Ant release.

I will also attach the log output from a demo app which I wrote which
runs the "myapp" test project (and its "mylib" subproject, incl.
<import>s of build-impl.xml, running the two targets 'clean' and
'run'). The proposed SPI would make roughly the same kind of
information available to client logger, so you can get an idea of the
level of detail and kinds of events that can be received. Note that
task started events show the task structure with all attributes and
nested text evaluated.

Created attachment 14925 [details]
Sample log file showing what kind of information would be available to logger implementations

The SPI looks nice. 
Scenario for J2ME javac links redirection will be following (please
correct me if this is wrong):
- implement AntLogger that will listen on scripts under J2ME projects
only, javac task only, error and warning level only
- register this logger before the standard one
- it will detect links to source files, consume the events, analyze
them and log output with correct hyperlink

I think this will work fine but one thing can make our live more easy.
In case the AntEvent allows modifying the message we can just replace
part of the message text and let the event to be consumed by standard
logger next in the queue (just a supplication ;o). Thanks.

Your proposed J2ME logger sounds exactly like what I had in mind. The
actual sequence of the callback would be

- check if consumed, if so skip
- check if it's an error message w/ a relative source path
- if so, consume and process

Re. permitting the logger to replace the message and send it to the
next one in the queue - I was originally considering that but decided
the consume/isConsumed model should work just as well (and for all
event types, not just messageLogged).

After all, in your logger all you have to do is find the URL to the
source file and create an OutputListener hyperlink using the
AntSession method, then print it - pretty simple. You can print
whatever you like for the text of the line (an absolute path, or leave
the message in its original formatting).

If you were to pass the message on to the standard handler and expect
it to create the link for you, it would actually be a bit trickier.
You would need to take your absolute source file path and line number
and error message and construct a message "${source}:${line}:
${message}" and then modify the event's message to be that. Then it
would be picked up by the standard logger.

The disadvantages of doing this are that (1) you lose the ability to
control the precise formatting of the printed line of text
independently from the hyperlink semantics; (2) you waste a little bit
of processor time doing this replacement, then having other loggers
look at the message and ignore it too, then having the standard logger
reparse the string and link it.

Do you agree?

Re. Jesse's SPI proposal: I only glanced at this quickly, and the SPI
looks reasonable. Would be nice to have an example of a real listener,
e.g. the Ant module logger that the Javadoc mentions. I haven't yet
tried actually implementing the WebApps listeners with this API.

Thanks Jesse, would be fine.

Creating a branch:

cvs rtag -b -r BLD200405190604 ant_loggers_42525 ant_nowww_1 java_nowww_1

I am considering removing o.a.t.a.m.spi.AntOutputStream since it does
not fit in well with the new SPI and I believe its functionality can
be much better handled by the new SPI - assuming what you want is to
customize Ant formatting rather than actually sending it somewhere
other than the OW. Jan L., is this OK for you (latex module)?

Created attachment 15059 [details]
Test script for experimentation

Moving base tag to BLD200405221753.

Moving base tag to BLD200406061800.

Seems to be basically working on the branch...

I succeeded in making the error message redirection work for errors in
JSP pages. I was getting errors from CVS when trying to create a
branch, so I am attaching the fix in a zip file. The patch in the zip
file is against BLD200406061800.

I found one issue, though. I couldn't make this work when calling JspC
using <taskdef> (that's how it's done in BLD200406061800). My logger
was not getting the message for some reason. The message was appearing
in the output though - it was consumed by the default logger, which
displayed it as a link to build-impl.xml.

When I switch to a different way of calling JspC (using <java>, see
issue 43585) that is used in newer builds, things started to work.

One other thing I'll want to do is to catch error messages from the
compilation of servlets generated from JSPs, and link them back to JSP
source using JSR 45 information.

Jesse, BTW, what is your target for merging this implementation to
trunk? Thanks.

Created attachment 15612 [details]
Implementation of JSP logger that links from JspC errors to JSP source

Re. not getting events for <jspc> - not sure. Try running with
-J-Dorg.apache.tools.ant.module.bridge.impl.NbBuildLogger=0 to see
what the exact event that is getting generated looks like. Might have
something to do with runtime wrappers for taskdef's tasks, I don't know.

Created attachment 15623 [details]
The logger API (ZIP of Javadoc); look at AntLogger, AntSession, AntEvent, & TaskStructure

Could I get a really fast fast-track review of this? At least someone
from DevRev to take a quick look at it? I would like to put it into
the trunk this week and I would like some sanity check on the API
(beyond Petr & Adam's comments).

Unlike most of the Ant module public interfaces, this interface is
intended to be used from several other modules outside of ant and the
ant/* submodules: definitely java/project, possibly java/j2seproject,
probably web/project, probably the J2ME project type.

The attached Javadoc shows the public interfaces (one abstract class,
three interfaces). Should be easy to make compatible extensions to;
only AntLogger is intended to be implemented by clients, and it is
just looked for in lookup, nothing else refers to it.

From a performance standpoint, the interestedIn* methods are intended
to make it possible to have a larger number of logger implementations
without having to pass every event to every logger. I haven't done a
perf check yet but I will take a look in OptimizeIt.

Unit tests are in progress - will try to test NbBuildLogger (the
dispatch of events to interested loggers) and StandardLogger (the
default impl with basic printing and hyperlinking support).

I believe the Javadoc is complete enough to not need separate overview
documentation, but if you are looking for examples of usage,
JavaAntLogger is a good example of listening to certain events
(filtered by some criteria), keeping per-(logger,session) state, and
producing hyperlinks. It is in java/project in the branch.

This API fits J2ME Project requirements.
Our J2MEAntLogger is ready in ant_loggers_42525 branch of
kjava/j2meproject module.

Thanks to Jesse.

On behalf of apireviews@netbeans.org: We discussed that today
(mmetelka, tpavek, jtulach) and we are fine with methods introduced by
the API. We however do not understand the reasons TaskStructure,
AntEvent and AntSession are interfaces. If there are no implementation
reasons for keeping them as interface we want to turn them into final
classes without externally available constructor.

There was discussion whether AntLogger as abstract class is correct,
whether that should not be interface, but we agreed that abstract
class is fine.

Making TaskStructure, AntSession, and AntEvent into final classes
would certainly complicate the implementation. They are implemented
not only in a different package but in a different compilation unit
(bridge.jar). Furthermore, there are two independent impls of AntEvent
in bridge.jar. Making these interfaces into classes would necessitate
introducing nonpublic interfaces (in ant.jar, impl in bridge.jar) to
provide the actual behavior, with the class delegating all methods to
the backend interface, plus a trampoline for constructing the class
instances based on an interface instance. I could do this, but it
would add some complexity that I would prefer to avoid unless there is
some particular reason why the current state poses a problem to
comprehensibility or evolvability of the SPI.

Re. AntLogger as an abstract class vs. interface - could be an
interface but it is plausible that new event callbacks or
interestedIn* criteria could be needed later, in which case it would
be easier if it is left as an abstract class. The default behavior of
each method should be clear: do nothing for callbacks and ask not to
be notified of anything; i.e. behave as if the logger did not exist.

(The SPI would be a bit easier to use if the default for interestedIn*
methods were 'true' rather than 'false', but I feared that this would
encourage lazy implementors to not override any of those methods,
leading to loss of scalability.)

Have added partial unit tests (more are needed); checks some problem
areas such as locating targets using Ant 1.6, which does not provide
such an API; a workaround is available which supplies some basic
information anyway.

Re. taskdef problem - still don't know what might have been wrong
there, but I added a unit test confirming that you can taskdef a task
and run it, and get messages from it, labelled with the selected task
name. If you find anything wrong here, let me know (or write a unit test).

I did a quick performance check. Looks reasonable. Some things that
could be optimized but nothing that appeared urgent.

1. Sample j2seproject that prints 99999 lines of output, stressing the
callbacks (since each line produces a message logged event that has to
be processed). Total time (cca. 8 seconds on my laptop) pretty much
the same as in a pre-42525 dev build. OptimizeIt (limited to the Ant
task group, to block out AWT repaints etc.) shows about 30% of the
time just consumed by the output window appending to the output
file... remainder split among various things (incl. Ant itself). Looks
like using raw string processing rather than regexps in a few places
(NbBuildLogger as well as AntLogger impls) would improve speed a bit.
Also NbBL should use a lighterweight algorithm to calculate loggers to
dispatch to; the current algorithm would be fairly fast for a large
number of loggers but is probably overkill for the handful that there
are; should limit use of Collections in this part of the code.

2. Building /space/src/r36/nbbuild/build.xml#build-nozip (until it
gets a 1.5-related compilation error). This looks fine; speed looks
comparable to normal; OI shows no significant hotspots in Ant logging
code at all, everything of interest is in javac.

Of course when other loggers are added, the authors need to check
their performance individually, to make sure that

- they are not expressing an interest in a category of events that
they do not need to receive (use interestedIn* wisely)

- the callbacks they do receive are fast and allocate little memory

A note: I fixed it so that in the dynamic body of a logger callback
you can freely print to stderr (or use ErrorManager etc.) and it will
not loop trying to send the output back to Ant. System I/O redirection
gets suspended while in callbacks.

committed 1.97  ant/build.xml
committed 1.70  ant/manifest.mf
committed 1.14  ant/api/doc/changes/apichanges.xml
committed 1.2   ant/nbproject/.cvsignore
committed 1.8   ant/nbproject/project.properties
committed 1.10  ant/nbproject/project.xml
committed 1.18 
ant/src-bridge/org/apache/tools/ant/module/bridge/impl/BridgeImpl.java
committed 1.6  
ant/src-bridge/org/apache/tools/ant/module/bridge/impl/Bundle.properties
committed 1.10 
ant/src-bridge/org/apache/tools/ant/module/bridge/impl/NbBuildLogger.java
committed 1.2  
ant/src/META-INF/services/org.apache.tools.ant.module.spi.AntLogger
committed 1.13  ant/src/org/apache/tools/ant/module/AntModule.java
committed 1.26  ant/src/org/apache/tools/ant/module/AntSettings.java
committed 1.17 
ant/src/org/apache/tools/ant/module/AntSettingsBeanInfo.java
committed 1.23  ant/src/org/apache/tools/ant/module/Bundle.properties
committed 1.17  ant/src/org/apache/tools/ant/module/bridge/AntBridge.java
committed 1.6  
ant/src/org/apache/tools/ant/module/bridge/AuxClassLoader.java
committed 1.6  
ant/src/org/apache/tools/ant/module/bridge/BridgeInterface.java
committed 1.8  
ant/src/org/apache/tools/ant/module/bridge/DummyBridgeImpl.java
removed   1.14 
ant/src/org/apache/tools/ant/module/run/AntOutputParser.java
committed 1.22  ant/src/org/apache/tools/ant/module/run/Bundle.properties
committed 1.2   ant/src/org/apache/tools/ant/module/run/Hyperlink.java
removed   1.20 
ant/src/org/apache/tools/ant/module/run/OutputWriterOutputStream.java
committed 1.2  
ant/src/org/apache/tools/ant/module/run/StandardLogger.java
committed 1.44 
ant/src/org/apache/tools/ant/module/run/TargetExecutor.java
committed 1.2   ant/src/org/apache/tools/ant/module/spi/AntEvent.java
committed 1.2   ant/src/org/apache/tools/ant/module/spi/AntLogger.java
committed 1.8  
ant/src/org/apache/tools/ant/module/spi/AntOutputStream.java
committed 1.2   ant/src/org/apache/tools/ant/module/spi/AntSession.java
removed   1.3   ant/src/org/apache/tools/ant/module/spi/Bundle.properties
committed 1.2   ant/src/org/apache/tools/ant/module/spi/TaskStructure.java
committed 1.2   ant/test/unit/data/antlogger/imported.xml
committed 1.2   ant/test/unit/data/antlogger/importing.xml
committed 1.2   ant/test/unit/data/antlogger/taskdefs.xml
committed 1.2   ant/test/unit/data/antlogger/trivial.xml
committed 1.2  
ant/test/unit/src/org/apache/tools/ant/module/spi/AntLoggerTest.java
committed 1.9   java/j2seproject/nbproject/project.xml
committed 1.2  
java/j2seproject/src/META-INF/services/org.apache.tools.ant.module.spi.AntLogger
committed 1.2  
java/j2seproject/src/org/netbeans/modules/java/j2seproject/J2SEAntLogger.java
committed 1.4   java/project/build.xml
committed 1.11  java/project/nbproject/project.xml
committed 1.2  
java/project/src/META-INF/services/org.apache.tools.ant.module.spi.AntLogger
committed 1.2  
java/project/src/org/netbeans/modules/java/project/JavaAntLogger.java
committed 1.515 nbbuild/build.xml

"Making these interfaces into classes would necessitate
introducing nonpublic interfaces (in ant.jar, impl in bridge.jar) to
provide the actual behavior, with the class delegating all methods to
the backend interface, plus a trampoline for constructing the class
instances based on an interface instance. I could do this, but it
would add some complexity that I would prefer to avoid unless there is
some particular reason why the current state poses a problem to
comprehensibility or evolvability of the SPI."

There is a hidden dependency between various modules (the one defining
the interfaces and second implementing them). By introducing
non-public API and expressing it by the implementation dependency one
clearly states to the world (module system, people assembling apps,
developers evolving the api) that there are some packaging/assembly
contstrains. E.g. I as a someone unaware of ant module specifics,
would easily find out that the bridge.jar has special releation than
just pure API usage.

Using "final interfaces" is option, but usually we suggest people to
invest more work and do final classes in case there are no performance
implications. Take it as advice.

But based on first paragraph of this comment, I think the we should at
least emphatize the dependency between the api module and implementor
of the "final interfaces". I suggest to define implementation version
"finalIEs1" and establish implementation dependency between the
modules. If other reviews agree, I would see this as a request.

Maybe I wasn't clear on this - the interfaces are implemented within
the same module, just from a different JAR and class loader. There is
no exposed intermodule semi-API.

Jesse,
 we have discussed this again on today's DevRev meeting and generally
we would like you to use the final classes instead of the "final
interfaces". This is an approach that we recommend in all reviews.
It is not required to follow that rule if there are difficulties with
that approach - e.g. many instances (so it would be costly to create
an extra delegate class instance) or technical difficulties (possibly
regarding your special setup with an extra jar - not sure whether that
applies here).

As you like. See issue #45491.