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

Unresolvable dependency after 1.30.8 #1487

Closed
worpet opened this issue Feb 3, 2020 · 19 comments · Fixed by #1491
Closed

Unresolvable dependency after 1.30.8 #1487

worpet opened this issue Feb 3, 2020 · 19 comments · Fixed by #1491
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@worpet
Copy link

worpet commented Feb 3, 2020

Environment details

  1. OS type and version: Linux
  2. Java version: 8
  3. google-api-client version(s): 1.30.8

Steps to reproduce

  1. Attempt to build and resolve dependencies with Ivy.
  2. Ivy fails to resolve androidx.annotation#annotation;1.1.0.

Code example

<dependency org="com.google.api-client" name="google-api-client" rev="1.30.8" conf="compile->default"/>

Stack trace

:: problems summary ::
:::: WARNINGS
		module not found: androidx.annotation#annotation;1.1.0

	==== central: tried

	  https://repo1.maven.org/maven2/androidx/annotation/annotation/1.1.0/annotation-1.1.0.pom

	  -- artifact androidx.annotation#annotation;1.1.0!annotation.jar:

	  https://repo1.maven.org/maven2/androidx/annotation/annotation/1.1.0/annotation-1.1.0.jar

		::::::::::::::::::::::::::::::::::::::::::::::

		::          UNRESOLVED DEPENDENCIES         ::

		::::::::::::::::::::::::::::::::::::::::::::::

		:: androidx.annotation#annotation;1.1.0: not found

		::::::::::::::::::::::::::::::::::::::::::::::

Any additional information below

Previous versions up to 1.30.7 build without any problems.

This seems to mention a similar error, but I am not positive if it is related: #1467 (comment)

[ERROR] /tmpfs/src/github/google-api-java-client/google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java:[17,26] error: package androidx.annotation does not exist

@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Feb 4, 2020
@chingor13 chingor13 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 10, 2020
@chingor13 chingor13 self-assigned this Feb 10, 2020
@chingor13
Copy link
Collaborator

#1467 looks like it introduced a dependency that exists in Google's Android Maven repository. I'm not sure that we should have taken this dependency as it seems to force all downstream users to add a Maven repository.

As a workaround, you can add https://maven.google.com/ as an additional Maven repository in your project configuration.

@yoshi-automation yoshi-automation removed the triage me I really want to be triaged. label Feb 10, 2020
@elharo
Copy link
Contributor

elharo commented Feb 10, 2020

There was a very specific bug fixed by this change. We can explore other options but I don't think requiring that repository is as bad as the bug it fixes.

@chingor13 chingor13 added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Feb 10, 2020
@chingor13
Copy link
Collaborator

This seems to be only affecting ivy and not Maven or gradle users. Please try adding the additional Maven repository url.

@yoshi-automation yoshi-automation removed the 🚨 This issue needs some love. label Feb 10, 2020
@worpet
Copy link
Author

worpet commented Feb 12, 2020

Will there be a permanent fix for this or will manually adding the additional repository be permanently required? Although Ivy is not as common as Maven, it is still a mainstream dependency manager and this is the only library we have encountered (as of 1.30.8) that fails to work "out of the box".

Our application is not used with Android, so ideally we would not be required to pull in any extra Android dependencies.

@elharo
Copy link
Contributor

elharo commented Feb 12, 2020

Unless we find new information, this is not a bug. The Maven repository system was always meant to be multi-repo. There is no expectation that all dependencies be loaded from Maven Central. and it sounds like Ivy can indeed resolve this dependency with a simple modification to the build.xml or ivy-settings.xml file.

I don't know why Android chooses not to upload their artifacts to Maven Central. You could ask them to do so. However we do support android in this library, and that does require us to use this dependency.

@elharo elharo closed this as completed Feb 12, 2020
@kilink
Copy link

kilink commented Feb 12, 2020

@elharo Shouldn't androidx.annotation:annotation only be a build/compile time dependency though? I don't think these annotations are needed at runtime, so consumers of google-api-java-client shouldn't need to pull in the additional dependency.

@elharo
Copy link
Contributor

elharo commented Feb 12, 2020

Java's default retention policy is CLASS, and I think that's what androidx.annotation.keep has. However, Maven doesn't have a real compileOnly dependency scope like Gradle does. compile scope in Maven is available at runtime. Perhaps we could make this provided or <optional>true</optional> but I'm not sure that would actually work.

@eugine
Copy link

eugine commented Feb 13, 2020

The issue was reproduced with a gradle build after migration from 1.30.3 to 1.30.8:
`
Execution failed for task ':compileJava'.

Could not resolve all files for configuration ':compileClasspath'.
> Could not find androidx.annotation:annotation:1.1.0.
Required by:
project : > com.google.apis:google-api-services-analyticsreporting:v4-rev20200210-1.30.8 > com.google.api-client:google-api-client:1.30.8`

The build can be fixed by adding 'google()` repository.

However, it's incorrect fix.

Nobody will expect to add a new repository for such things: either move the androidx.annotation to maven central repository (mavenCentral()) or remove this dependency in the library.

@elharo
Copy link
Contributor

elharo commented Feb 13, 2020

We're not going to remove the dependency. It's necessary to make this library work on Android, which is a requirement.

You can ask the android team to publish this artifact on Maven Central. I don't know why they don't.

@worpet
Copy link
Author

worpet commented Feb 13, 2020

Given that it's also necessary to make this library work on not-Android, could you at least update your installation instructions in your README.md to mention this additional repository dependency? For example, @eugine has confirmed Gradle is affected by this, but your Gradle instructions say only the central repository is needed:

To use Gradle, add the following lines to your build.gradle file:

repositories {
    mavenCentral()
}

The frustration here is that, for many build systems, the only way to discover this dependency is to attempt to build and find it is not able to build.

@rleibman
Copy link

I would prefer not to add the dependency, but given the stuborness that the dependency must be there... may I suggest that semantic versioning is thus broken and you should definitely increase at least the minor number? Something that breaks the build should most certainly NOT be a patch only increase.

@elharo
Copy link
Contributor

elharo commented Feb 13, 2020

I am not sure this is something semver considered. There are no API changes here, at all. This change was a bug fix, nothing more. Per semver that's a patch release.

@rleibman
Copy link

Maybe not by the letter of the semver law, but the change breaks every single downstream build, and that fact is barely being acknowledged, at least a semver minor change says: "be careful, expect build to break"

@elharo
Copy link
Contributor

elharo commented Feb 13, 2020

This appears to be a deliberate design decision in Gradle to not consider repositories declared in the pom

"Maven POM metadata can reference additional repositories. These will be ignored by Gradle, which will only use the repositories declared in the build itself.
This is a reproducibility safe-guard but also a security protection. Without it, an updated version of a dependency could pull artifacts from anywhere into your build."

@abhishekmohite
Copy link

Everything worked normally till ivy + 1.30.7 and now we see the mentioned issue since ivy + 1.30.8.

@anuraaga
Copy link
Contributor

tl;dr Sent #1491 which I think should make everyone happy. But I see more issues here than just a POM file so let me provide some opinion.

Since this is a library for developers, shouldn't developer experience be a higher priority than philosophy regarding how maven or gradle should work? I think you'd be hard pressed to find even one other library on Maven Central that requires resolving artifacts from a separate repository. In fact, it's strongly discouraged by Maven Central

http://maven.apache.org/repository/guide-central-repository-upload.html

It's true that Google's isn't a "sketchy" repository, but keeping builds simple and easy to reason about is likely just as much a motivation for this recommendation. Requiring Gradle users to add an additional repository primarily for Android to non-Android Java builds will make them slower (more repositories to resolve from) and confusing (the README change only adds a single line, google(), but in practice, everyone needs additional lines of comments to explain this is necessary solely to use a simple API client).

But anyways, the problem here can be solved without affecting user builds by replacing the annotation with an embedded proguard file in resources. I can't find much documentation besides this tweet and another library that uses the technique

https://twitter.com/jakewharton/status/1004401938467876865?lang=en
https://github.com/square/retrofit/blob/master/retrofit/src/main/resources/META-INF/proguard/retrofit2.pro

You can ask the android team to publish this artifact on Maven Central. I don't know why they don't.

Sorry I do have to comment about my disappointment at this statement. It would be much more expedient to work out this sort of issue internally (and probably gotten a quick tip about embedded proguard to boot!) than force your users to follow up.

@CydeWeys
Copy link
Contributor

This is causing us problems too. It's weird to have to pull in an additional repository considering that out of hundreds of dependencies, nothing else requires this.

@salqadri
Copy link

Please fix this issue. I have many many dependencies, and have never had to add another repository, and we will not be adding anything more to our scala project other than the default one. As such, we are not able to upgrade to the latest google java apis. Why was this issue closed?

@chingor13
Copy link
Collaborator

This was fixed in 1.30.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet