Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate AutoValue artifacts into an annotation and a processor #250

Closed
cantrowitz opened this issue Jun 5, 2015 · 29 comments
Closed

Separate AutoValue artifacts into an annotation and a processor #250

cantrowitz opened this issue Jun 5, 2015 · 29 comments

Comments

@cantrowitz
Copy link

Is there a good way to direct generated output to a different directory than [build_folder]/intermediates/classes.... This gets a bit messy from using it in the IDE and linking to a build output folder as a src directory.

@tbroyer
Copy link
Contributor

tbroyer commented Jun 5, 2015

This is dictated by the -s argument to javac, so it all depends on your build tool, and only it. Your not even forced to supply that folder and javac will keep things in memory (or temporary files).

@cantrowitz
Copy link
Author

A lot of android projects have an associated apt binary:
e.g.:

dependencies {
  apt 'com.google.dagger:dagger-compiler:2.0'
  compile 'com.google.dagger:dagger:2.0'
}

This makes it quick and easy for developers to integrate it into their project. Any plans on adding this support?

It does seem like AutoParcel does this, but might be good to have all developments in once location.

Thanks!

@tbroyer
Copy link
Contributor

tbroyer commented Jun 5, 2015

I don't get it. Auto works exactly the same as Dagger as far as declaring your dependencies goes: just add the dependency to the apt configuration (nothing to add to compile as there's no runtime dependency). There's nothing to add to Auto, or I didn't understand what you're asking for.

@cgruber
Copy link
Contributor

cgruber commented Jun 6, 2015

As @tbroyer pointed out, using the same strategy as dagger projects, using the plugins from http://code.neenbedankt.com/gradle-android-apt-plugin/ (also found here: https://bitbucket.org/hvisser/android-apt) will let you specify:

dependencies {
apt 'com.google.auto.value:auto-value:1.1'
}

and this should successfully configure gradle and intellij to use auto-values and put the generated files in a place where intellij can find them.

@cgruber cgruber closed this as completed Jun 6, 2015
@JakeWharton
Copy link
Contributor

The annotation is only on the processor path if you do that which means you
cannot actually leverage the library behavior.

On Fri, Jun 5, 2015, 9:02 PM Christian Edward Gruber <
[email protected]> wrote:

Closed #250 #250.


Reply to this email directly or view it on GitHub
#250 (comment).

@cgruber
Copy link
Contributor

cgruber commented Jun 6, 2015

I'm not sure I follow what you mean, jake? There is no runtime "library" -
just an annotation whose bytecode is not required at runtime.

On Fri, 5 Jun 2015 at 18:08 Jake Wharton [email protected] wrote:

The annotation is only on the processor path if you do that which means you
cannot actually leverage the library behavior.

On Fri, Jun 5, 2015, 9:02 PM Christian Edward Gruber <
[email protected]> wrote:

Closed #250 #250.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@JakeWharton
Copy link
Contributor

The annotation needs to be on the classpath for use.

On Fri, Jun 5, 2015, 9:33 PM Christian Edward Gruber <
[email protected]> wrote:

I'm not sure I follow what you mean, jake? There is no runtime "library" -
just an annotation whose bytecode is not required at runtime.

On Fri, 5 Jun 2015 at 18:08 Jake Wharton [email protected] wrote:

The annotation is only on the processor path if you do that which means
you
cannot actually leverage the library behavior.

On Fri, Jun 5, 2015, 9:02 PM Christian Edward Gruber <
[email protected]> wrote:

Closed #250 #250.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@cantrowitz
Copy link
Author

As @JakeWharton pointed out there should be one apt binary and one compile binary.

I think the AutoParcel project might be the easiest to illustrate (since it is pretty closely related to this project):

dependencies {
  compile 'com.github.frankiesardo:auto-parcel:0.3'
  apt 'com.github.frankiesardo:auto-parcel-processor:0.3'
}

The auto-parcel-processor and the actual annotation auto-parcel

@johncarl81
Copy link

I don't think the separation of the two libraries is necessary. The compile scoped dependency of auto-parcel only contains annotations that are @Retention(RetentionPolicy.SOURCE) (http://docs.oracle.com/javase/7/docs/api/java/lang/annotation/RetentionPolicy.html#SOURCE), which is "discarded by the compiler".

Also, I can confirm I use Autovalue under provided / apt scope successfully.

@tbroyer
Copy link
Contributor

tbroyer commented Jun 6, 2015

From the android-apt README:

Configuration of other annotation processors

For annotation processors that include the API and processor in one artifact, there's no special setup. You just add the artifact to the compile configuration like usual and everything will work like normal. Additionally, if code that is generated by the processor is to be referenced in your own code, Android Studio will now correctly reference that code and resolve references to it.

I suppose using provided would also work.

In https://github.com/tbroyer/gradle-apt-plugin (for non-android projects), I created a compileOnly configuration, similar to the android's provided one. Because apt does extends compileOnly (contrary to the android-apt plugin), you'd have to add Auto to both compileOnly (for the annotation) and apt (for the processor).

@JakeWharton
Copy link
Contributor

The big problem we face with provided is that dependencies show up on the IDE classpath but not at runtime. You an easily reference a Guava type without complaint from the compiler but only to find a runtime failure. Not only do we want this to fail at compile time, we never want Guava to show up in IDE auto-complete.

@tbroyer
Copy link
Contributor

tbroyer commented Jun 6, 2015

@JakeWharton Yup. This is why you want those dependencies of annotation processors to be shaded and "hidden". But indeed having separate artifacts for annotations and processor makes it easier.

@johncarl81
Copy link

@JakeWharton, ah, that's good point, I hadn't thought of that perspective. Makes the API much more clear and no extra non-runtime dependencies to reference in the IDE. I always figured the apt plugin was only meant for referencing generated code in Android Studio.

@cgruber
Copy link
Contributor

cgruber commented Jun 7, 2015

Hmm. We went deliberately with a one artifact approach because of a bias
for the Maven usage approach. We can revisit that upon consideration. Our
main fear is people using annotations and not configuring the processor
into their builds. Unlike bazel, there's no "exported plugin" metadata
where a dependency can vend processors along with it in a 2 artifact
scenario.

I'd like feedback from folks on what preferred deployment would look like,
if we just redid things.

On Sat, Jun 6, 2015, 17:09 John Ericksen [email protected] wrote:

@JakeWharton https://github.com/JakeWharton, ah, that's good point, I
hadn't thought of that perspective. Makes the API much more clear and no
extra non-runtime dependencies to reference in the IDE. I always figured
the apt plugin was only meant for referencing generated code in Android
Studio.


Reply to this email directly or view it on GitHub
#250 (comment).

@tbroyer
Copy link
Contributor

tbroyer commented Jun 7, 2015

I, for one, am fine with a single artifact approach iff dependencies are all shaded (and hidden from auto complete in IDEs), and of course if there are no runtime dependencies (which would mean the processor is deployed in the runtime classpath).
I think Jake's issue is more with Dagger 2 which doesn't shade Guava.

@cgruber
Copy link
Contributor

cgruber commented Jun 7, 2015

Dagger 2 is already a 2 artifact situation because there is a runtime,
albeit very minimal.

Autofactory shades everything I think, and autovalue is about to as we
strip out velocity with a micro replacement Éamonn wrote.

On Sun, Jun 7, 2015, 18:48 Thomas Broyer [email protected] wrote:

I, for one, am fine with a single artifact approach iff dependencies are
all shaded (and hidden from auto complete in IDEs), and of course if there
are no runtime dependencies (which would mean the processor is deployed in
the runtime classpath).
I think Jake's issue is more with Dagger 2 which doesn't shade Guava.


Reply to this email directly or view it on GitHub
#250 (comment).

@JakeWharton
Copy link
Contributor

Android (and I) prefer the Dagger 2 approach. This allows us to keep the
classpath free of Guava and other fat cow dependencies from making it into
the app (via processor path). It also puts the runtime (whether it's a
single annotation or proper runtime library ala Dagger 1 + JSR330) as a
normal, simple compile dependency.

On Sun, Jun 7, 2015, 7:48 PM Thomas Broyer [email protected] wrote:

I, for one, am fine with a single artifact approach iff dependencies are
all shaded (and hidden from auto complete in IDEs), and of course if there
are no runtime dependencies (which would mean the processor is deployed in
the runtime classpath).
I think Jake's issue is more with Dagger 2 which doesn't shade Guava.


Reply to this email directly or view it on GitHub
#250 (comment).

@cgruber
Copy link
Contributor

cgruber commented Jun 8, 2015

Even when the contents of the "dependency" is a source only annotation?
Why would you even want that?

Though given that you have to go through the motions in gradle, it seems no
worse. I wonder if everything were shaded (avoiding ide collisions) and
the apt plugin for gradle added the dependency as a build time only dep,
whether that would change your view?

On Sun, Jun 7, 2015, 18:58 Jake Wharton [email protected] wrote:

Android (and I) prefer the Dagger 2 approach. This allows us to keep the
classpath free of Guava and other fat cow dependencies from making it into
the app (via processor path). It also puts the runtime (whether it's a
single annotation or proper runtime library ala Dagger 1 + JSR330) as a
normal, simple compile dependency.

On Sun, Jun 7, 2015, 7:48 PM Thomas Broyer [email protected]
wrote:

I, for one, am fine with a single artifact approach iff dependencies are
all shaded (and hidden from auto complete in IDEs), and of course if
there
are no runtime dependencies (which would mean the processor is deployed
in
the runtime classpath).
I think Jake's issue is more with Dagger 2 which doesn't shade Guava.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@cgruber
Copy link
Contributor

cgruber commented Jun 8, 2015

Not arguing strongly here, just trying to hone in on the best policy to
support everyone., if possible.

On Sun, Jun 7, 2015, 19:04 Christian Gruber [email protected] wrote:

Even when the contents of the "dependency" is a source only annotation?
Why would you even want that?

Though given that you have to go through the motions in gradle, it seems
no worse. I wonder if everything were shaded (avoiding ide collisions) and
the apt plugin for gradle added the dependency as a build time only dep,
whether that would change your view?

On Sun, Jun 7, 2015, 18:58 Jake Wharton [email protected] wrote:

Android (and I) prefer the Dagger 2 approach. This allows us to keep the
classpath free of Guava and other fat cow dependencies from making it into
the app (via processor path). It also puts the runtime (whether it's a
single annotation or proper runtime library ala Dagger 1 + JSR330) as a
normal, simple compile dependency.

On Sun, Jun 7, 2015, 7:48 PM Thomas Broyer [email protected]
wrote:

I, for one, am fine with a single artifact approach iff dependencies are
all shaded (and hidden from auto complete in IDEs), and of course if
there
are no runtime dependencies (which would mean the processor is deployed
in
the runtime classpath).
I think Jake's issue is more with Dagger 2 which doesn't shade Guava.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@johncarl81
Copy link

Correct me if I'm wrong, Shading only obfuscates the package, it would be easy to accidentally autocomplete a Guava class by just class name.. right?

@cgruber
Copy link
Contributor

cgruber commented Jun 8, 2015

Yes, though I believe naming the package starting with $ helps fix that, in
some cases.

On Sun, Jun 7, 2015, 19:06 John Ericksen [email protected] wrote:

Correct me if I'm wrong, Shading only obfuscates the package, it would be
easy to accidentally autocomplete a Guava class by just class name.. right?


Reply to this email directly or view it on GitHub
#250 (comment).

@johncarl81
Copy link

I'll throw a vote in for the Dagger 2 approach. Pros: API usability in the IDE around the APT scope, Cons: the user might miss the compiler dependency.

If a user misconfigures the compiler dependency wouldn't it result in a failed build?

With the added push of at least one Android extension (#202) puts some emphasis on the Android side of things.

@cgruber
Copy link
Contributor

cgruber commented Jun 8, 2015

Fair point about compilation failures.

On Sun, Jun 7, 2015, 19:15 John Ericksen [email protected] wrote:

I'll throw a vote in for the Dagger 2 approach. Pros: API usability in the
IDE around the APT scope, Cons: the user might miss the compiler dependency.

If a user miss-configures the compiler dependency wouldn't it result in a
failed build?

With the added push of at least one Android extension (#202
#202) puts some emphasis on the
Android side of things.


Reply to this email directly or view it on GitHub
#250 (comment).

@JakeWharton
Copy link
Contributor

Yes, even if it is a single source-only annotation. It needs to be on the
classpath for IDE completion. It is useless on the processor path.

On Sun, Jun 7, 2015, 8:17 PM Christian Edward Gruber <
[email protected]> wrote:

Fair point about compilation failures.

On Sun, Jun 7, 2015, 19:15 John Ericksen [email protected] wrote:

I'll throw a vote in for the Dagger 2 approach. Pros: API usability in
the
IDE around the APT scope, Cons: the user might miss the compiler
dependency.

If a user miss-configures the compiler dependency wouldn't it result in a
failed build?

With the added push of at least one Android extension (#202
#202) puts some emphasis on the
Android side of things.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@cgruber
Copy link
Contributor

cgruber commented Jun 8, 2015

Sorry... The question included the premise that we fix the plugin to make
the apt scope export the plugin to the class path of the ide.

But yeah. It's probably just easier to extract the annotation. Sigh. The
more I think about the android studio use case the more I think you're
right.

On Sun, Jun 7, 2015, 19:57 Jake Wharton [email protected] wrote:

Yes, even if it is a single source-only annotation. It needs to be on the
classpath for IDE completion. It is useless on the processor path.

On Sun, Jun 7, 2015, 8:17 PM Christian Edward Gruber <
[email protected]> wrote:

Fair point about compilation failures.

On Sun, Jun 7, 2015, 19:15 John Ericksen [email protected]
wrote:

I'll throw a vote in for the Dagger 2 approach. Pros: API usability in
the
IDE around the APT scope, Cons: the user might miss the compiler
dependency.

If a user miss-configures the compiler dependency wouldn't it result
in a
failed build?

With the added push of at least one Android extension (#202
#202) puts some emphasis on the
Android side of things.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@cgruber
Copy link
Contributor

cgruber commented Jun 8, 2015

I'm just bummed as it adds boilerplate to the Maven case.

On Sun, Jun 7, 2015, 20:28 Christian Gruber [email protected] wrote:

Sorry... The question included the premise that we fix the plugin to make
the apt scope export the plugin to the class path of the ide.

But yeah. It's probably just easier to extract the annotation. Sigh. The
more I think about the android studio use case the more I think you're
right.

On Sun, Jun 7, 2015, 19:57 Jake Wharton [email protected] wrote:

Yes, even if it is a single source-only annotation. It needs to be on the
classpath for IDE completion. It is useless on the processor path.

On Sun, Jun 7, 2015, 8:17 PM Christian Edward Gruber <
[email protected]> wrote:

Fair point about compilation failures.

On Sun, Jun 7, 2015, 19:15 John Ericksen [email protected]
wrote:

I'll throw a vote in for the Dagger 2 approach. Pros: API usability in
the
IDE around the APT scope, Cons: the user might miss the compiler
dependency.

If a user miss-configures the compiler dependency wouldn't it result
in a
failed build?

With the added push of at least one Android extension (#202
#202) puts some emphasis on
the
Android side of things.


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).


Reply to this email directly or view it on GitHub
#250 (comment).

@JakeWharton
Copy link
Contributor

It's not strictly the Maven case. The build system is irrelevant. It's the case where you don't care about your classpath or what's in your final artifact. Not sure how you'd classify it, but for the purposes of discussion it's the JVM service case.

@frankiesardo
Copy link

Separating the annotation from the processing is a very sounded choice in my opinion.

It's not impossible that somebody would like to depend just on ˋgoogle.auto.value.annotationˋ and provide his own implementation for the processor (to do something that we can't forsee at the moment).

In the early days of my fork having the annotation as a separate dependency would have been great: the users that now are using auto-parcel would have been using the same annotation with a different processor and migrating to this new version would have been seemless without them even noticing.

Shipping a repo with just the annotation means that you can mark your classes with AutoValue and maybe package them as a library and let the consumer decide what processor to use (another far fetched example, but still).

I just see so many potentials wins with this move with relatively little cost. Plus, since ˋgoogle.auto.value.processorˋ would depend on the annotation, if you're not using Android Studio you just need to add the processor as a provided dependency and you're done, no reason to know the other artefact exists.

@eamonnmcmanus eamonnmcmanus reopened this Aug 12, 2015
@eamonnmcmanus eamonnmcmanus changed the title Generated output folder Separate AutoValue artifacts into an annotation and a processor Sep 15, 2015
@eamonnmcmanus
Copy link
Member

Seems to have ended up as a duplicate of #268.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants