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

Add gradle instructions for Android projects #452

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cypressf
Copy link

Let me know if this is not correct. I assume we should be using provided and annotationProcessor?

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@eamonnmcmanus
Copy link
Member

Hopefully someone more familiar with Gradle on Android than me can review this?

I also think we should update the AutoValue version number, but that's a problem that's already there.

@JakeWharton
Copy link
Contributor

This is fine as far as an official recommendation.

The use of provided still incorrectly leaks all of the transitive dependencies of AV onto the IDE and compilation classpath such that you can write code that uses things from Guava and it will compile fine but explode at runtime. Implementing #268 would solve that, but for now I've created https://github.com/JakeWharton/AutoValueAnnotations to avoid this problem, although I don't expect an official recommendation of it.

@eamonnmcmanus
Copy link
Member

@JakeWharton can you explain more? AutoValue shades Guava, so I don't see how an IDE could pick up classes from it.

@JakeWharton
Copy link
Contributor

Well it'd pick up com.google.auto.value.shaded.guava or whatever the package is in autocomplete when you typed "ImmutableList". The goal is such that the only class that is on the classpath for compilation and in the IDE is the annotation such that you don't get accidental imports of things that won't be present at runtime.

@eamonnmcmanus
Copy link
Member

The shaded name is $ImmutableList, so it won't be picked up by autocomplete. The only things you could accidentally import are classes from the AutoValue processor itself. Not that I disagree that separating these artifacts would have advantages, but the current situation is not as bad as you suggest.

@JakeWharton
Copy link
Contributor

Yeah that's really great. Not all processors are as considerate to their downstream consumers!

Also I'd think shading is something that becomes less of a pressing of an issue with a separate annotations artifact since build systems should resolve the classpath and processorpath (and any conflicts therein) entirely separately.

@eamonnmcmanus
Copy link
Member

@cypressf can you sign the CLA? It looks as if the recommendation itself is fine (apart from the version number, which we'll fix).

Google's processes mean that we won't actually be merging the PR, but will make the same change (duly credited) internally and sync it out.

@cypressf
Copy link
Author

Yup, I signed it.

@googlebot
Copy link

CLAs look good, thanks!

@tlaundal
Copy link

tlaundal commented Jun 6, 2017

Using provided and annotationProcessor is not sufficient anymore, as the generated Factory classes import com.google.auto.factory.internal.Preconditions.

The only way to avoid shading in the processing code is to split the annotations and other classes needed at runtime into its own module, like #268 suggests, and include this module as a compile dependency, I think.

EDIT: It seems 1.0-beta5 fixes runtime dependencies for the Factorys.

@JakeWharton
Copy link
Contributor

JakeWharton commented Jun 6, 2017 via email

@kluever kluever added P2 type=documentation Documentation that is other than for an API labels Oct 28, 2019
@netdpb netdpb added P3 and removed P2 labels May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes P3 type=documentation Documentation that is other than for an API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants