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

Guava for Android and Guava for the JRE need separate Maven namespaces #2914

Closed
shs96c opened this issue Aug 15, 2017 · 54 comments
Closed

Guava for Android and Guava for the JRE need separate Maven namespaces #2914

shs96c opened this issue Aug 15, 2017 · 54 comments
Assignees
Labels

Comments

@shs96c
Copy link

shs96c commented Aug 15, 2017

Guava for Android is lacking classes that are provided in Guava for the JRE (notably around support for Java 8 in the immutable collections). The two are not interchangeable.

Maven's dependency resolution looks at group and project IDs, and then attempts to find the most recent version. Because the only difference between the android and JRE versions of guava is the version number, this can result in Java 8 projects getting the Android version of Guava (and, presumably, vice versa).

For reference, at the time of writing search.maven.org believes the most recent guava release is the one for Android.

guava-oops

The correct thing to do is give the Android version a different project ID.

@JakeWharton
Copy link

Seems like a classifier would be far more appropriate.

@shs96c
Copy link
Author

shs96c commented Aug 15, 2017

That would also work, so long as the JRE version retained the default classifier (which I think is jar).

@kashike
Copy link

kashike commented Aug 15, 2017

See #2839, as well.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

#2839 could also be resolved by adding a classifier. Expecting just the version string to work won't work.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

Can we please get a 23.1 release that just adds the classifiers and stops projects breaking?

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

Our understanding was that classifiers do not solve the problem. I'll see if I can dig up past conversations or test it out.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

Despite what Maven's docs say about classifier...

As a motivation for this element, consider for example a project that offers an artifact targeting JRE 1.5 but at the same time also an artifact that still supports JRE 1.4. The first artifact could be equipped with the classifier jdk15 and the second one with jdk14 such that clients can choose which one to use.

...we found that Maven will happily permit a project to depend on both com.google.guava:guava:jar:22.0 and com.google.guava:guava:jar:someclassifier:22.0.

[edit: see also tbroyer on classifier]

As best we can tell, the only reliable solution is for users to manually request <max-version-needed> or, if they're running on Android/Java7, <max-version-needed>-android.

If there's another scheme that could work, we'd be happy to hear about it.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

Then pick a different project ID :) The problem right now is that more and more projects are potentially being broken.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

A different project ID has the same effect as a different classifier, as far as we can tell.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

Using the right terminology now :)

So the maven namespace is segmented by group id (which can be com.google.guava), artifact id (which is currently guava in both cases) and version (23 or 23-android right now). The correct solution, as pointed out, is to use a classifier, but apparently this is hopelessly broken.

The next best solution is to change the artifact id. This will lead to a situation where people might end up depending on both guava and guava-android. A group id can have many different artifact ids without them clashing --- if they couldn't then maven as we know it couldn't work.

@jbduncan
Copy link
Contributor

@cpovirk @shs96c I'm not sure I understand what the problem is with a project potentially depending on both guava and guava-android. Can you elaborate further for me?

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

@jbduncan, it depends on what you're targeting, and what gets put into your classpath:

  • An app targeting android might end up with the JRE version of guava. In that case, someone might mistakenly depend on methods that aren't available in the Android SDK (eg. the Stream APIs)

  • An app targeting desktop guava might not compile properly because they depend on classes that aren't present in the android flavour.

Because the packages are the same, whichever is first on the classpath will be selected. In some cases, this will work as expected. In some cases it won't. Because maven's dependency resolution is "novel" at best, which you get is a bit of a crapshoot. Unless you use version pinning, the version dependency appears to be advisory at best.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

Our thinking was that it was pretty bad for people to be able to depend on both guava and guava-android, for the reasons @shs96c lays out.

What I probably need to understand better are the drawbacks of the current versioning scheme. It sounds like you want to be able to tell Maven "depend on the most recent version?" If we could use a version like 23.0-server to make it "more recent" than 23.0-android, as discussed in #2839, I think we could solve that problem[*]. Are there other problems?

[*] mostly: It still requires that you have at least one dep on 23.0-server. If you don't but you do have deps on 22.0-server and 23.0-android, Maven is still going to do the wrong thing. Maybe you can ensure that you have that dep (or maybe there's a way to tell Maven "ignore which version everyone specifies, and just give me the max?"). But at that point, I'm wondering if it's easier to just tell Maven to use that specific version directly.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

@cpovirk yes there's a problem. Doing that will break android projects.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

@cpovirk, I think your understanding of maven dependency resolution isn't quite right. It does a BFS of dependencies, picking the first one that it finds. Thus, even if something like selenium declares a dep on "23.0-server", unless it's "closer" to the build than the one that specifies "19.0", version 19 will be selected.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

RE: "Doing that will break android projects": Ah, you're saying that, if an Android project tells Maven "use the most recent available version of Guava," it will then get the server version? Good point.

RE: "maven dependency resolution": Thanks, I am aware of the BFS. Above you said: "Maven's dependency resolution looks at group and project IDs, and then attempts to find the most recent version." I was figuring that you were using something like Gradle's "newest" strategy or perhaps some plugin that automatically updates your project to depend on the newest versions (update: like this)? I'm trying to understand the places where it matters which version Maven considers to be "newest."

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

Re: Android. Yes, that's exactly what I'm saying. It's a thorny problem.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

Re: "guava-server". Not all java implementations are server-side. I think guava and guava-android are clearer. If you want to really spell out that one version is for the JRE, guava-jre seems to more accurately describe the version.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

Re: "maven dependency resolution" Glad you know about the full horror of how maven selects versions. It's good to have company :)

I'm just getting reports from users who are using maven's default versioning strategy (not necessarily using gradle). I guess they're using aether.

You know, GH could really do with threading of comments.

@lukeis
Copy link

lukeis commented Aug 16, 2017

In case it isn't clear, this particular issue Selenium was having was due to leaving the version as open ended:

<dependency>
  <groupId>com.google.guava</groupId>
  <artifactId>guava</artifactId>
  <version>[23.0,)</version>
</dependency>

This allows selenium to specify the minimum required version of guava but anticipating that we will still be compatible with future versions of guava that another project may specify. The problem we encounter is that if another project depends upon us and doesn't specify their version of guava, maven may choose the 'latest' it finds in central... which by timestamp happens to be 23.0-android.

Having users not accidentally include both android and jdk 8 versions is certainly a problem... but solving it in this way feels like bad maven practice. It seems like it's a bug for maven to try to figure out a way for everyone to identify two packages that are incompatible with each other.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

(I'm sure I don't know the full horror of Maven dependencies yet, hopefully ever :))

True, "server" is the wrong word. I think that "jre" may technically mean "Oracle," which isn't necessarily what we mean, either. Hmm. (Update: Admittedly "Android" is itself not the best term, since we advise Java 7 users to use it, too.)

Sounds like Aether may be "Maven Artifact Resolver" nowadays? The docs best docs seem to be this Wiki.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

jre just means Java Runtime Environment. IBM ships one too, and the OpenJDK has a "jre" package . I think it's an accepted industry term.

Yes, that's the aether version I was referring to.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

I think that #2914 (comment) properly describes the problems that guava users are experiencing, and is what we should be trying to resolve.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

Oops, I think you're right about "jre." Thanks.

@jbduncan
Copy link
Contributor

Oof, I can see that this is not easy to resolve. Just reading this comment thread is enough to do my head in. 😜

@lukeis Do you know if the Selenium team ever considered one of the following workarounds?

  1. Use a static version of Guava, and update it periodically.
  2. "Shade" Guava into Selenium with a tool like the Maven Shade plugin?

@lukeis
Copy link

lukeis commented Aug 16, 2017

@jbduncan we are currently working around the issue by setting the version explicitly to 23.0.

Selenium is highly customizable, in particular with our "Grid / Server" product. It would be a bit unkind to those users to shade a dependency that may need upgrading in a timely manner... we don't always timely upgrade our guava reference and it can be many months between our releases when we all get busy.

@cpovirk
Copy link
Member

cpovirk commented Aug 16, 2017

Thanks, @lukeis. That may have been clear to others but was not to me.

That does sound like a good reason for making sure that the jre artifact is newer than the android artifact. (As @shs96c noted, that doesn't help Android users, but I think we would have broken any Android users who used open-ended ranges when we released 21.0, which didn't come with an Android version.)

I'm guessing that there would still be problems -- say, if Selenium depended on something that requests 23.0-android, might Maven still pick that over the "newer" 23.0-jre, since it was requested specifically? And of course there's the problem I mentioned above in which someone wants 24.0-android and someone wants 23.0-jre: The only solution is 24.0-jre, which Maven is never going to figure out on its own.

I will keep thinking about all this.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

@jbduncan: I'm the selenium lead :)

  1. Internally we use a static version of guava, but our maven dependency used to pin to (23,] (version 23 or later). Prior to that, we didn't specify a range 23.0, but this caused problems when we replaced usages of guava's Function with java 8's.
  2. Shading will bloat the jar, and won't resolve this issue for other users.

@shs96c
Copy link
Author

shs96c commented Aug 16, 2017

@cpovirk, yeah, making sure that the jre version is released after the android version will mask this issue for many users.

@cpovirk
Copy link
Member

cpovirk commented Aug 31, 2017

I think the root problem here is the difference between Java 8 and Android as base SDKs. Java 9 is coming in September, and will widen the gap further.

(We will likely keep the main artifact on Java 8, but I don't know that we've made that decision for certain. It's similar to how we stayed on Java 6 and (IIRC) upgraded straight to Java 8 without releasing a Java 7 version.)

One solution is to shade the Android library into a different package. Libraries that expect to run on Android can make the move to the new package. Those that expect to run on the JRE need do nothing (or make a breaking change, and make everyone make a choice).

We're trying to serve the case of cross-platform libraries, too. It's possible that they could fork or preprocess their code, too, of course, but that has its own tradeoffs.

Ideally I should note this in the linked doc someday.

Also, why not just distribute the Android version of guava as an AAR, and not as a JAR?

We also promise that it's usable as on the JRE (for Java 7 users). (I don't know enough about AAR to say more than that.)

Given the fact that maven version/artifact resolution is not going to be
sufficient to, in and of itself, note guava-jre and guava-android as
incompatible, can we use some approach like maven-enforcer or whatever to
supply constraints, such that a project won't have both in its deps graph?

My understanding has been that users can opt into such enforcement themselves, but we have no way of doing it for them. If users know enough to take action, the solution is generally simple enough (at least once we switch to -jre). The hard part is getting it right automatically most of the time.

@shs96c
Copy link
Author

shs96c commented Sep 1, 2017

We also promise that it's usable as on the JRE (for Java 7 users). (I don't know enough about AAR to say more than that.)

AAR is an Android-specific way of packaging a library. If you've got something that is only meant for an Android app, that's the best way to distribute it.

https://gist.github.com/lopspower/6f62fe1492726d848d6d might be useful.

@JakeWharton
Copy link

JakeWharton commented Sep 1, 2017 via email

@shs96c
Copy link
Author

shs96c commented Sep 1, 2017

You'd be shipping two: just guava as a jar (targeting java 8, as now), and guava as an aar, which android projects could depend on.

@JakeWharton
Copy link

That would then contradict this statement by @cpovirk:

Update: Admittedly "Android" is itself not the best term, since we advise Java 7 users to use it, too.

@ben-manes
Copy link
Contributor

An aar would require the artifact only notation in Gradle. I think that if the explicit extension and transitively unqualified dependency of guava were in the tree, then both would be retrieved. If so, this would require Android users to manually exclude which would require a fix to the user's build. That doesn't seem clearer than using a resolution strategy / enforcer to restrict versions.

cpovirk added a commit that referenced this issue Sep 7, 2017
This makes Maven consider 24.0-jre to be "newer" than 24.0-android.
That's preferable, as discussed in #2914, #2839, and https://docs.google.com/document/d/1NYGbfz56C0Oh4IGymXjeQUVK4FcRiqDbpc4vGLnDMrY/edit#

RELNOTES: To upgrade to Guava 24.0, change your version numbers to "24.0-jre." (Or, if are writing an Android app or Android-compatible library, use "24.0-android.")

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167799453
@cpovirk cpovirk added P2 P1 and removed P2 labels Sep 14, 2017
@yborovikov
Copy link

yborovikov commented Sep 19, 2017

@cpovirk If we changed -android version suffix to -a, maven version comparison rules would treat it as lesser than vanilla release version, i.e. 24.0 (for java 8+) would be considered newer than 24.0-a (for android / java 7) by maven.
This way we can keep normal guava version normal (and "latest") while providing android / java 7 specific version, too.

@perceptron8
Copy link
Contributor

@yborovikov Isn't a intended for alpha releases? It could be misleading.

@yborovikov
Copy link

yborovikov commented Sep 19, 2017

@perceptron8 -a is indeed sorted the same way as -alpha (and -b - as -beta). however, the meaning of -a doesn't have to translate to "alpha", and, unless we publish alpha releases to maven central, should not create much confusion (at least no more than -android).

for example, java build 1.7.0_80-b15 doesn't mislead us into thinking it's a beta release, does it? :)

i believe a single sentence in the README.md would be enough to explain the meaning and purpose of releasing both guava:24.0a and guava:24.0.

@yborovikov
Copy link

Downside of using -a: it might be sorted before rc, etc. and considered a "pre-release" version by various tools (e.g. mvnrepository).
So we could use -ga ("google android") suffix instead - this is considered to be equal to no suffix, so 24.0 and 24.0-ga would be treated as equally fresh by maven.

Checked these (version xx-ga and xx) against maven-enforcer-plugin's requireUpperBoundDeps rule - neither is considered newer than the other, both can be used interchangeably.

@ben-manes
Copy link
Contributor

GA is sometimes used as General Availability in version strings. It looks like some JBoss artifacts used this and then switched to Final, so perhaps there was an issue they encountered. I wouldn't redefine known suffixes even if they are non-standard.

@yborovikov
Copy link

@ben-manes We're trying to achieve something Maven doesn't have a prescribed solution for here.

You're right, ga is one of well-known qualifiers (and stands for general availability, as you mentioned), and that's the reason this version qualifier gets special treatment (re version comparison) from Maven.

We would be piggy-backing on this documented Maven feature:

strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering. Well-known qualifiers (case insensitive) are:

  • alpha or a
  • beta or b
  • milestone or m
  • rc or cr
  • snapshot
  • (the empty string) or ga or final
  • sp

to provide two Guava versions that can be treated equally from version comparison perspective (at least by Maven itself).

This approach should resolve this very issue of Maven version comparison treating 23.0-android version as the latest over 23.0 one - as stated by the issue submitter.

As for why JBoss switched from ga version suffix to Final - personally I have no idea, as well as why do projects use -ga or .Final suffixes for their vanilla release artifact versions, to begin with. My guess would be some historical reasons (e.g. JBoss docs circa 2004 said: "We did not want to depend on a specific build tools naming and ordering rules, because those things tend to change over time. While Maven is the popular choice today ...").

yborovikov pushed a commit to yborovikov/guava that referenced this issue Sep 19, 2017
yborovikov pushed a commit to yborovikov/guava that referenced this issue Sep 19, 2017
…anilla versions from 24.0-jre back to 24.0
@yborovikov
Copy link

yborovikov commented Sep 19, 2017

Copied from the discussion doc shared by @cpovirk:

While it's considered bad practice to just tell Maven "use latest version" (or rely on version ranges), there's another practice of assuring (via maven-enforcer-plugin) that a library (transitively shared by multiple dependencies) resolves a version no older than what's required by any of the dependencies.

In our case we try to maintain Java 7 compatibility by using android-flavored guava version, and upgrade to 23.0-android broke the (java 8) builds that depend on the project yet try to enforce the latest versions among shared dependencies - 23.0-android is considered by Maven to be a newer one and guava-23.0 fails the check.

Arguably, java 7 / android builds will experience similar issues (e.g., trying to use 24.0-android while some dependencies use 24.0-jre).

Ideally, Guava would produce two versions that are considered equally fresh by Maven tooling - and there is such option: use 24.0 for vanilla version and 24.0-ga for android one.

This would allow a project to pick the flavor it needs while enforcing that the latest (among dependencies) version is being effectively used.

(Of course, android builds would still rely on / check that Java 8 features are not being actually used in the execution path of the libraries - but that's unavoidable anyways.)

@cpovirk
Copy link
Member

cpovirk commented Oct 4, 2017

I replied to the latest comment on the doc a while back. I still need to go back and reply to the remaining comments on the doc. Sorry that I'm taking so long. For the moment, in any case, we're trying -android and -jre. Please let us know if that causes any new trouble.

@omarzouk
Copy link

omarzouk commented Jul 19, 2019

@cpovirk yes it does, it just did for me and I had to chase around java dependency trees for 3 hours to figure it out! I'm using Google Cloud java SDK libraries, and they sometimes use the -jre and sometimes use the -android as dependencies and if one of the SDK libraries has a more recent guava android version (e.g. 27-android vs 26-jre), SBT will decide to prefer the more recent -android over the -jre. Which breaks everything.

This postfix concept, is a misuse that is incompatible with what a version is supposed to be in the maven echo-system.

excerpt from sbt evicted:

com.google.guava:guava:27.1-android is selected over {20.0, 26.0-jre, 25.1-android, 26.0-android, 19.0}

@Gr33nbl00d
Copy link

Version should describe a different version of the SAME artifact. If it is a different artifact use a different artifact name!

@dbyron0
Its a shame you simply close such issues..

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

No branches or pull requests