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

Use api instead of compile for Android #3154

Closed
wants to merge 2 commits into from
Closed

Use api instead of compile for Android #3154

wants to merge 2 commits into from

Conversation

andrzejressel
Copy link
Contributor

No description provided.

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@JakeWharton
Copy link

The direct analogous is api. implementation is merely an optimization that should be opted into.

@andrzejressel
Copy link
Contributor Author

I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

@jbduncan
Copy link
Contributor

@JakeWharton I thought it was the other way round? Isn't implementation the same as compile, with api being the optimisation? :/

@jbduncan
Copy link
Contributor

@JakeWharton Wait, I'm challenging myself now. I realise that I'm not 100% clear on the difference between api and implementation, so never mind. :)

@JakeWharton
Copy link

JakeWharton commented May 23, 2018 via email

@andrzejressel andrzejressel changed the title Use implementation instead of compile for Android Use api instead of compile for Android May 23, 2018
@andrzejressel
Copy link
Contributor Author

api it is then.

@cpovirk
Copy link
Member

cpovirk commented May 23, 2018

@cpovirk cpovirk self-assigned this May 23, 2018
@cpovirk
Copy link
Member

cpovirk commented May 23, 2018

Hmm, so should we change the JRE one to api, as well? It sounds like compile may be deprecated.

@JakeWharton
Copy link

JakeWharton commented May 23, 2018 via email

@jbduncan
Copy link
Contributor

@cpovirk compile is indeed deprecated. :)

@cpovirk
Copy link
Member

cpovirk commented May 23, 2018

Huh. So is the idea that Android projects are more likely to use the java-library plugin? If anything, I might have naively guessed they were less likely :) Is it just that the default Android build setup suggests java-library or something?

@JakeWharton
Copy link

JakeWharton commented May 23, 2018 via email

@cpovirk
Copy link
Member

cpovirk commented May 23, 2018

Ah! Thanks.

@jbduncan
Copy link
Contributor

Hmm. It sounds to me that implementation is the most sensible default, and api should be used only if the java-library plugin is imported and the given project wants to expose Guava types to its users. Is my understanding correct @JakeWharton?

@JakeWharton
Copy link

JakeWharton commented May 23, 2018 via email

@jbduncan
Copy link
Contributor

@JakeWharton Cool! So you're suggesting api here because it's the most liberal option and most equivalent to the older compile, am I right?

@cpovirk
Copy link
Member

cpovirk commented May 24, 2018

(Even for people who don't expose Guava in their APIs, I'd be a little nervous about excluding transitive dependencies from the classpath, given problems with static analyzers and perhaps annotation processors discussed in #2824 and #2721. (There could also be problems if people use Guava annotations but don't consider that "exposing an API," but that seems less likely.) I suppose that those problems are relatively unlikely to affect people, though.)

@cpovirk
Copy link
Member

cpovirk commented May 24, 2018

Anyway, I do like the idea of suggesting the thing that is more likely to work (even if it's slower in some cases -- and I suspect only marginally so, since Guava isn't going to be changing between one build and the next). It is a little sad that this exports Guava to downstream targets, but that seems like the lesser evil (and no worse than what people see today).

One final thought: We could decide to give instructions for each of...

  • Java 8+
  • Java 7
  • Android

...where the Java 7 instructions look like the current Android instructions and the Android instructions look like your new ones. But I'm not sure that's worth the potential confusion.

@ronshapiro ronshapiro mentioned this pull request May 25, 2018
ronshapiro pushed a commit that referenced this pull request May 25, 2018
Fixes #3154

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197947742
ronshapiro pushed a commit that referenced this pull request May 25, 2018
Fixes #3154

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197947742
@jbduncan
Copy link
Contributor

@cpovirk I think it's worth at least changing the remaining compile instruction for "guava-jre" to api, so that the Gradle installation instructions are consistent. :)

@cpovirk
Copy link
Member

cpovirk commented May 29, 2018

@jbduncan, @JakeWharton said above:

I believe 'api' only exists for JRE projects if you're using the
'java-library' plugin. If you're using the regular 'java' plugin you only
get 'compile' and 'implementation' (the idea being you're a leaf node and
don't have anywhere to expose dependencies to).

So I worry that api won't exist for some people, and they'll be confused.

Does Gradle warn about compile in JRE projects like it does in Android projects?

@jbduncan
Copy link
Contributor

@cpovirk Good point about @JakeWharton's comment!

Gradle does warn about compile in JRE projects too. :(

Taking all this into consideration, I think the best advice to give Gradle users is something like the following:

To add a dependency using Gradle:

// For Java applications or Java libraries which use Guava internally:
implementation 'com.google.guava:guava:25.1-jre'
// Or, for Java libraries which use the "java-library" plugin and expose Guava types to users
api 'com.google.guava:guava:25.1-jre'
// Or, for Android applications & libraries which use Guava internally:
implementation 'com.google.guava:guava:25.1-android'
// Or, for Android libraries which use the "java-library" plugin and expose Guava types to users
api 'com.google.guava:guava:25.1-android'

But having said this, the example advice I give above is quite verbose. I wonder if there's a better way of presenting it to Gradle&Guava users...

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

Successfully merging this pull request may close these issues.

None yet

5 participants