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

Google Cloud components should not depend on -android versions #4440

Closed
stevenschlansker opened this issue Feb 4, 2019 · 9 comments
Closed
Assignees
Labels
type: question Request for information or clarification. Not an issue.

Comments

@stevenschlansker
Copy link

Currently, the google-cloud-logging-logback component depends on Guava version 26.0-andriod. This is not appropriate for Google Cloud projects which are running on a full JDK in e.g. GKE. The two different versions are not considered compatible by Maven and version checker plugins.

It looks like this issue was introduced in #3980

Environment details

  • OS: Fedora Linux 29
  • Java version: JDK 11
  • google-cloud-java version(s): 0.79.0-alpha

Steps to reproduce

  1. Use Guava
  2. Import google-cloud-logging-logback
  3. Run a build checker that ensures dependency convergence

Stacktrace

[INFO] --- maven-dependency-versions-check-plugin:2.0.2:check (default) @ mycorp-server ---
[INFO] Checking dependency versions
[ERROR] Found a problem with the direct dependency com.google.guava:guava of the current project
  Expected version is 26.0-jre
  Resolved version is 26.0-jre
  Version 26.0-android was expected by artifact: com.google.cloud:google-cloud-logging-logback
  Version 26.0-jre was expected by artifacts: com.mycorp.components:mycorp-jaxrs-client, com.mycorp.components:mycorp-spring
@stevenschlansker
Copy link
Author

stevenschlansker commented Feb 4, 2019

Additionally, the docs specify that Google Cloud components aren't even compatible with Android:

Note: Cloud Java client libraries do not currently support Android.

(https://cloud.google.com/logging/docs/setup/java)
so it's even stranger to depend on 26.0-android version.

@sduskis
Copy link
Contributor

sduskis commented Feb 4, 2019

We base our guava version based on grpc, which currently uses 26.0-android.

@ejona86, do you have any comments about the history of the selection of the guava version?

@sduskis
Copy link
Contributor

sduskis commented Feb 4, 2019

Would it make sense for us to exclude grpc's dependency on -android and use -jre instead?

@sduskis
Copy link
Contributor

sduskis commented Feb 4, 2019

Oh... I see. @andreamlin commented:

We chose the android flavor because it supports JDK 7. (See guava README).

Our libraries need to support JDK 7, but the -jre version supports 8+ only.

@ejona86
Copy link

ejona86 commented Feb 4, 2019

The android version can be used for two things: 1) android support, 2) JDK 7 support. google-cloud-java supports JDK 7, so the -android version is required.

Note that Maven's output looks like what I'd expect. There's no bug here. It is complaining that the version was downgraded from 26.0-jre to 26.0-android (because 'j' in jre is greater than 'a' in android). So this is the same sort of error you get with 25.0 vs 26.0. You need to add the higher version of Guava to your dependencies, or reorder your dependencies.

@ejona86
Copy link

ejona86 commented Feb 4, 2019

Wait a minute. That output does look strange. Why is Maven complaining? "Expected version is 26.0-jre" and "Resolved version is 26.0-jre"... so what's the issue?

@stevenschlansker, oh, you're using dependencyConvergence? Dependency Convergence adds pain for no benefit. I strongly recommend requireUpperBoundDeps instead. In any case when using dependencyConvergence these sorts of errors will crop up and you have to manually address it (via exclusions which is part of why dependencyConvergence should not be used).

@stevenschlansker
Copy link
Author

stevenschlansker commented Feb 4, 2019

I understand that it can be resolved in each project by explicitly resolving the conflict -- however pushing this onto every downstream user is not great. It's not quite the same as 25 vs 26 as both jars really are version 26. (Would jdk vs android be more appropriate as a classifier?)

I guess ideally Guava would be split into guava-api and then guava-impl-jdk and guava-impl-android but that's probably asking far too much.

I can play around with requireUpperBoundDeps but my previous experience with Maven version checkers was that they generally understand -alpha -beta and -rc like version, but basically anything else it gives up and says "these versions aren't compatible".

Per https://maven.apache.org/ref/3.3.3/maven-artifact/apidocs/org/apache/maven/artifact/versioning/ComparableVersion.html it should be alphabetically sorted, but relying on this behavior over making the versioning more sane seems like a really unfortunate decision - inherently one is "greater" than the other, but which one you want is context dependent. If android > jre, then your android build correctly sees android as the upper bound, but jre does not. If jre > android vice versa.

@ejona86
Copy link

ejona86 commented Feb 5, 2019

android is a subset of the jre API (so no, guava-api wouldn't work), so it is correct that android < jre. I agree there can be problems in cases like 25.0-jre < 26.0-android. But there's also not many alternatives (and no, classifier doesn't work).

Complaints about Guava's versioning system should be filed at https://github.com/google/guava and not here. It's already been made clear that -android is the appropriate and necessary version for this project.

requireUpperBoundDeps should work much better than dependencyConvergence. For example, there is no need to <exclude> dependencies (which can break future upgrades) and it properly checks even if <dependencyManagement> is in use.

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Feb 5, 2019
@sduskis sduskis added type: question Request for information or clarification. Not an issue. and removed triage me I really want to be triaged. labels Feb 5, 2019
@sduskis
Copy link
Contributor

sduskis commented Feb 5, 2019

I don't think that we can do much on this repo. I'm going to close this issue since there isn't anything actionable for this repo. @ejona86 suggested alternative repos in which this investigation can proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

4 participants