Bug 160773 - [jsr269] Need interfaces between jdt compiler and jsr269 impl
Summary: [jsr269] Need interfaces between jdt compiler and jsr269 impl
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.3 M5   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-12 20:08 EDT by Walter Harley CLA
Modified: 2007-02-06 05:28 EST (History)
2 users (show)

See Also:


Attachments
Proposed patch (11.40 KB, patch)
2006-11-15 19:50 EST, Olivier Thomann CLA
no flags Details | Diff
Proposed patch (24.51 KB, patch)
2006-12-14 13:31 EST, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Walter Harley CLA 2006-10-12 20:08:17 EDT
We need interfaces between the JDT batch compiler and the JSR269 implementation in o.e.jdt.compiler.apt.  This bug is a forum for discussion.


At present, o.e.jdt.internal.compiler.batch.Main supports command line invocation by parsing the command line and then constructing and invoking an o.e.jdt.internal.compiler.Compiler.   The JSR199 implementation works by extending Main to implement the new javax.tools.JavaCompiler interface.

To add APT support, APT will subclass o.e.jdt.internal.compiler.Compiler.  The subclass will be named o.e.jdt.compiler.apt.internal.AptCompiler.  Instead of just instantiating a new Compiler, the command-line and JSR199 batch compiler invocations will need to either create a Compiler if annotation processing is not required, or gets a Compiler from APT via reflection if it is.  

Note that according to the Java documentation, javac defaults to running annotation processors unless the -noproc option is explicitly specified.  If no -processors or -processorPath option is explicitly specified, it searches the classpath to find processors.  So we cannot rely on the presence of these options to indicate the need for annotation processing.  This suggests that, to decide whether to instantiate the Compiler from APT or from JDT, we would need to not only look at the command line options but also (in the default case) crack every jar on the classpath and look at the files in its META-INF/services folder to see if it offers a Processor.  Whether that's the right thing to do, or whether we should deviate from the javac spec to be more efficient, is Philippe's call, I think.

To support the JSR199 CompilationTask.setProcessors() method, Compiler will need an additional setProcessors() method itself; on the JDT Compiler this will be implemented with UnsupportedOperationException but in APT it will actually set processors.

In batch mode, AptCompiler needs a filesystem proxy to read files (e.g. processor jars) and create resource files and Java source files.  This could be a JavaFileManager, except that's a JSR199 class and thus not available within base JDT.  It needs to have most or all of the functionality of JavaFileManager; in particular it needs to be able to honor getFileAsInput(StandardLocation.ANNOTATION_PROCESSOR_PATH, ...) and getJavaFileAsOutput(StandardLocation.SOURCE_OUTPUT, ...) based on the -processorPath and -s options of the command line, or else it needs to provide broad enough filesystem access to let the APT command-line code implement that functionality. In the JSR199 invocation case, callers can pass in their own JavaFileManager, which must then be passed to APT.  

APT also needs access to additional command line options: -A, -processorPath, -processor, -proc:only, -s, -XprintProcessorInfo, and -XprintRounds.

I suggest that JDT create an interface CompilerFactory, which would be implemented by APT as a singleton object AptBatchCompilerFactory obtained through reflection, and which would have one method, newCompiler( existing Compiler constructor params, plus filesystem proxy ).

In IDE mode, Compiler is created by AbstractImageBuilder.  Here again, this needs to create an AptCompiler instead if annotation processing is required.  However, the processing context mostly comes from the platform (reading preferences, loading processors from plug-ins, etc.) rather than being passed in.  This object needs to be created via the extension mechanism, rather than reflection.  So I suggest an extension point o.e.jdt.core.extendedCompiler, whose implementation interface is CompilerFactory.  In the IDE, AbstractImageBuilder would use this extension point to get an AptIdeCompilerFactory from APT.  This version of CompilerFactory would ignore the filesystem proxy and the extended options, instead getting them from the platform.
Comment 1 Olivier Thomann CLA 2006-10-27 11:56:09 EDT
We propose the following ideas:
1) If a processor is detected (this will need to be clarified), we would insert a loop after the actual beginToCompile(...) call and before the process units.
This loop would call a ProcessorManager (or another name like that). The processor manager would know how to process the known units and collects the new units created while processing them. If new units have been created, the lookup environment would be reset and the beginToCompile loop called again with old units and new units.
Once the second beginToCompile is done, the ProcessorManager would not be called again. I believe we agreed that the creation of the new units is done in one pass.

Any thoughts?

I said that we need to clarify how a processor is detected. I guess we can do it by processing the different apt options. I need to get information on them in order to know what they should do.

Do you see any missing part in this compilation loop?
Comment 2 Philipe Mulet CLA 2006-10-27 12:45:01 EDT
ProcessorManager --> AnnotationProcessorManager
#process(...) --> #processAnnotations(...)
Comment 3 Walter Harley CLA 2006-11-01 17:52:48 EST
Olivier asked:
> What kind of information do you need to "process" the 
> annotations of the units?
> I have read that as soon as each annotation has a "matching" 
> processor, there is no more lookup for processors. Will you 
> take care of that part inside the apt code?

Mainly, in addition to the annotated units themselves, what we need is the information necessary to discover and load processors.  And, as noted above, we need access to a filesystem, such as what JavaFileManager provides.

From the command-line entry point, if there is a -processorPath option, or -processor options, we need those; if not, we need the -classpath and (if it already exists and is convenient) a classloader for it.

If the JSR199 entry point is being used, it seems more involved.  Some aspects of JSR199 that I don't understand include:

 - JavaCompiler.getTask() takes as an argument "Iterable<String> classes".  The javadoc says "class names (for annotation processing)" but I have no idea what that's supposed to mean, nor what classloader is supposed to be used to load these classes.  It does not seem to correspond to any javac command line option.

 - ToolProvider.getSystemClassLoader() is supposed to return "the class loader for tools provided with this platform".  I don't think that classloader needs to be able to load processor classes, but I'm not sure.  If it does, that implies that either JDT needs to create the processor class loader, or else APT needs to be able to create it and then hand it back to JDT on demand.

 - JavaFileManager.getClassLoader() is supposed to be able to provide a class loader for, among other things, annotation processors.  Since the caller can specify a JavaFileManager in their call to JavaCompiler.getTask(), I assume that we are supposed to use that class loader to load our annotation processors.  I guess that in javac, they have a command-line option parser that creates a JavaFileManager that includes a class loader based on the -classpath or-processorPath option, and then the compiler calls that method to load its annotation processors.  I assume the idea is that from the JSR199 entry point, if CompilationTask.setProcessors() has been called then the JFM classloader will not be used; and that at the command line, if -processor options are detected, they translate into calls to CompilationTask.setProcessors().


So, taking those things into account, one possibility is for you guys to provide us (regardless of whether the entry point is command line or JSR199) with a JavaFileManager that has a class loader that honors the Location switch and that includes the annotation processor path.  However, that still leaves open the question of how APT is supposed to actually *discover* the processors on that classpath.  Possibly the answer is for JDT to implement java.util.ServiceLoader.  (Or APT could implement it; but to do that we will need to look at the manifests of all the jars on the processor classpath and/or factory classpath, which might be duplicated effort.)

I think there is a lot there that might be unnecessary for users - I don't know whether it will be part of the Sun JCK, though.

As mentioned above, APT also needs access to some additional command line options: -A, -proc:only, -s, -XprintProcessorInfo, and -XprintRounds, or the corresponding options from the JSR199 mechanism.
Comment 4 Olivier Thomann CLA 2006-11-15 19:50:59 EST
Created attachment 53949 [details]
Proposed patch

This adds the looping strategy, but we would still need a good way to initialize the annotation processor manager.
Walter, I think we should talk together about a good way to process the command line and add new options.
Comment 5 Olivier Thomann CLA 2006-12-14 13:31:00 EST
Created attachment 55694 [details]
Proposed patch

This patch includes updated BatchCompilerTests.
Comment 6 Olivier Thomann CLA 2006-12-14 13:34:39 EST
Released for 3.3M5.
Comment 7 Eric Jodet CLA 2007-02-06 05:19:43 EST
Verified for 3.3 M5 using build I20070205-0009