Bugzilla – Full Text Bug Listing
| Summary: | Pass automatically provided options to Java 6 processors | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Fabian Steeg <steeg> | ||||
| Component: | APT | Assignee: | Fabian Steeg <steeg> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | eclipse, gautier.desaintmartinlacaze, jarthana, mmc | ||||
| Version: | 4.7 | Flags: | jarthana:
review+
|
||||
| Target Milestone: | 4.7 M7 | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X | ||||||
| See Also: |
https://git.eclipse.org/r/91420 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=47f05c94e7ccf833a043c9e55a6d97f7d448bf8c |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Fabian Steeg
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. New Gerrit change created: https://git.eclipse.org/r/91420 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. 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? (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 See also Bug 512547 - might be relevant in this discussion (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() 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?
Jay, could you please have a look? (In reply to Fabian Steeg from comment #8) I'm okay with any approach that works for both Eclipse and javac. (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. (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! 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 Fabian, I pushed a new changeset to gerrit with some whitespace changes. Otherwise, the patch looks good to me. Great, thanks Jay, the blank lines make sense, I reviewed +1 in the gerrit. Gerrit change https://git.eclipse.org/r/91420 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=47f05c94e7ccf833a043c9e55a6d97f7d448bf8c Thanks Fabian! Verified for 4.7 M7 using I20170508-2000 build |