Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 341298 - Pass automatically provided options to Java 6 processors
Summary: Pass automatically provided options to Java 6 processors
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: APT (show other bugs)
Version: 4.7   Edit
Hardware: PC Mac OS X
: P3 normal with 1 vote (vote)
Target Milestone: 4.7 M7   Edit
Assignee: Fabian Steeg CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-29 21:13 EDT by Fabian Steeg CLA
Modified: 2017-05-10 04:47 EDT (History)
4 users (show)

See Also:
jarthana: review+


Attachments
Return all options (makes them available to processor in ProcessingEnvironment) (1.55 KB, patch)
2011-03-29 21:13 EDT, Fabian Steeg CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Steeg CLA 2011-03-29 21:13:09 EDT
Created attachment 192152 [details]
Return all options (makes them available to processor in ProcessingEnvironment)

I've been looking into an issue with the Eclipse integration of the Contracts for Java project (see http://goo.gl/oZxc5). One particular feature mentioned in the JDT-APT UI (Project > Properties > Java Compiler > Annotation Processing) would be very useful to fix it:

"Note: options such as -classpath and -sourcepath are automatically passed to all processors, with values corresponding to the project's Java settings"

However, it seems this is only true for Java 5 processors, as I found this in the online help: "there are no automatically provided options for processors using the Java 6 annotation processing API, because the JavaFileManager methods provide the same functionality".

I've asked for help on how JavaFileManager replaces that feature in the JDT forum (see http://goo.gl/NsuGQ), but since nobody was able to help me I'd like to propose that the automatically provided options are also passed to Java 6 processors (see attached patch).
Comment 1 Morten Christensen CLA 2014-10-27 12:28:23 EDT
Yes, I need this also for VALJOGen (valjogen.41concepts.com). It would make configuration so much easier if "classpath" and "sourcepath" is passed automatically as options to the annotation processor as stated in the mentioned eclipse dialog.
Comment 2 Eclipse Genie CLA 2017-02-18 08:29:48 EST
New Gerrit change created: https://git.eclipse.org/r/91420
Comment 3 Fabian Steeg CLA 2017-02-18 10:26:32 EST
The Gerrit change linked from comment 2 contains my original patch and a test.

For manual reproduction see: https://github.com/fsteeg/eclipse-341298

The processor in these tests writes a source file, which references a class available on the project classpath. The processor then attempts to compile the file with the Java compiler API, using the passed options. Since the classpath is not passed as an option, compilation fails. According to the note in the project preferences (Java Compiler -> Annotation Processing), the classpath should be passed as an option to the processor.
Comment 4 Walter Harley CLA 2017-02-20 15:28:59 EST
I don't remember now, but the only reason I can think why I would have gone to the effort of explicitly excluding those options is if JDK6 javac also did not pass them.  

In general the effort with the Eclipse APT implementation was to try to be as similar to JDK6 as possible; in particular, the goal was that processors that worked in Eclipse should also work when executed in javac, and vice versa.  This goal was seen as very important because code that is written in the Eclipse IDE is often subsequently compiled in ant or Maven batch jobs.

So: Fabian, have you confirmed what the present behavior of javac is?  Does it pass these options to the processor?
Comment 5 Fabian Steeg CLA 2017-02-21 08:26:56 EST
(In reply to Walter Harley from comment #4)

Thanks for the explanation Walter, I now understand the original motivation of the implementation. I tested it with the 341298_Annotation project I linked to in comment 3, using javac 1.8.0_121:

javac -classpath src -processorpath processor.jar src/com/test/Annotated.java

And the classpath is not passed as an option to the processor. I see how getting it in Eclipse could lead to people building processors that only work in Eclipse, and not with javac.

From a usage perspective however, when calling javac I have the classpath string, so I can easily pass it as an option explicitly. In Eclipse however, I currently have to manually create it, and manually keep it in sync with the actual Java project classpath. This is where this bug originates from (the link in comment 0 now redirects to a different page, it originally pointed to the discussion starting at [1]). Also the 'Java Compiler' -> 'Annotation Processing' project preference page explicitly says that -classpath is passed to all processors.

How about we prepend "when running in Eclipse" to that note to make clearer what to expect? It would read: "Note: when running in Eclipse, options such as -classpath and -sourcepath are automatically passed to all processors, with values corresponding to the project's Java settings."

[1] https://fsteeg.wordpress.com/2011/02/07/setting-up-contracts-for-java-in-eclipse/#comment-184
Comment 6 Walter Harley CLA 2017-02-22 11:29:08 EST
See also Bug 512547 - might be relevant in this discussion
Comment 7 Fabian Steeg CLA 2017-02-25 18:22:53 EST
(In reply to Walter Harley from comment #4)

> So: Fabian, have you confirmed what the present behavior of javac is?  Does
> it pass these options to the processor?

I looked into that a little more and in cofoja [1] they get the classpath and other options that are passed to javac from the (non-API) JavacProcessingEnvironment via reflection.

So it is possible to get these options with javac, but it's not possible when running the processor in Eclipse. Passing through the options would make the same thing possible. It would allow processors that compile generated code to integrate much better in Eclipse.

I added that approach to the sample project [2], providing a use case that works with javac, does not currently work in Eclipse, but works with my proposed Gerrit change [3].

Varying behavior in different implementations also seems in line with the ProcessingEnvironment#getOptions() documentation [4]:

"See documentation of the particular tool infrastructure being used for details on how to pass in processor-specific options. For example, a command-line implementation may distinguish processor-specific options by prefixing them with a known string like "-A"; other tool implementations may follow different conventions or provide alternative mechanisms. A given implementation may also provide implementation-specific ways of finding options passed to the tool in addition to the processor-specific options."

[1] https://github.com/nhatminhle/cofoja
[2] https://github.com/fsteeg/eclipse-341298
[3] https://git.eclipse.org/r/#/c/91420/
[4] https://docs.oracle.com/javase/7/docs/api/javax/annotation/processing/ProcessingEnvironment.html#getOptions()
Comment 8 Fabian Steeg CLA 2017-03-04 18:33:01 EST
A different approach for passing classpath etc to the processor would be to support variables in processor options, so that users can set up options like classpath=${classpath} in the Annotation Processing preferences page, and we'd insert the removed -classpath value into the value passed to the processor.

While less convenient than simply passing all options, this makes the classpath passing explicit, and would also make this work with existing processors that expect the classpath in a processor-specific option.

Walter, what do you think? Should I implement a proposal and update my Gerrit?
Comment 9 Noopur Gupta CLA 2017-03-10 03:47:08 EST
Jay, could you please have a look?
Comment 10 Walter Harley CLA 2017-03-12 15:53:26 EDT
(In reply to Fabian Steeg from comment #8)

I'm okay with any approach that works for both Eclipse and javac.
Comment 11 Jay Arthanareeswaran CLA 2017-03-13 04:57:30 EDT
(In reply to Fabian Steeg from comment #8)
> A different approach for passing classpath etc to the processor would be to
> support variables in processor options, so that users can set up options
> like classpath=${classpath} in the Annotation Processing preferences page,
> and we'd insert the removed -classpath value into the value passed to the
> processor.

Sounds like some UI work involved? Not sure if we want to do that, at least in 4.7 time frame. I can still look at the current gerrit if you want.
Comment 12 Fabian Steeg CLA 2017-03-14 05:05:32 EDT
(In reply to Jay Arthanareeswaran from comment #11)

> Sounds like some UI work involved? Not sure if we want to do that, at least
> in 4.7 time frame.

I think we'd only have to update the note in the preferences, and the documentation (which contains a screenshot with the note).

> I can still look at the current gerrit if you want.

Based on Walter's feedback I now think that the variables approach would be better, as it encourages creating processors that don't depend on Eclipse specific features. I'll implement a proposal for that and update the Gerrit.

Jay, Walter, thanks for your feedback!
Comment 13 Fabian Steeg CLA 2017-03-18 09:47:18 EDT
The variable approach is ready for review in Gerrit [1].

Unlike I wrote in comment #8, I used the %variable% syntax (which is already supported for classpath variables in annotation options, see bottom of [2]).

I'll look into contributing the UI note and documentation updates next.

I also updated the GitHub repo [3] (I removed the javac reflection approach, checkout 273836c if anyone wants to reproduce the behavior from comment #7).

[1] https://git.eclipse.org/r/#/c/91420/
[2] http://help.eclipse.org/neon/index.jsp?topic=%2Forg.eclipse.jdt.doc.isv%2Fguide%2Fjdt_apt_getting_started.htm
[3] https://github.com/fsteeg/eclipse-341298
Comment 14 Jay Arthanareeswaran CLA 2017-03-22 07:03:28 EDT
Fabian, I pushed a new changeset to gerrit with some whitespace changes. Otherwise, the patch looks good to me.
Comment 15 Fabian Steeg CLA 2017-03-22 10:06:57 EDT
Great, thanks Jay, the blank lines make sense, I reviewed +1 in the gerrit.
Comment 17 Jay Arthanareeswaran CLA 2017-03-22 11:09:18 EDT
Thanks Fabian!
Comment 18 Sasikanth Bharadwaj CLA 2017-05-10 04:47:18 EDT
Verified for 4.7 M7 using I20170508-2000 build