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

Can't compile with Guava 20 and 21 when using annotation processor #2837

Closed
jhaber opened this issue Jun 1, 2017 · 9 comments
Closed

Can't compile with Guava 20 and 21 when using annotation processor #2837

jhaber opened this issue Jun 1, 2017 · 9 comments
Assignees
Labels
type=other Miscellaneous activities not covered by other type= labels

Comments

@jhaber
Copy link

jhaber commented Jun 1, 2017

We just upgraded our codebase to Guava 20 and started getting this compiler error:

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] cannot access com.google.errorprone.annotations.CanIgnoreReturnValue
  class file for com.google.errorprone.annotations.CanIgnoreReturnValue not found
[INFO] 1 error
[INFO] -------------------------------------------------------------

It seems similar to #2721 except that it affects both Guava 20 and 21 and also seems much more serious since it's an actual compile error and not a warning. After investigating further, the issue seems to be when using (any) annotation processing as well as ForwardingExecutorService (which was annotated with @CanIgnoreReturnValue as part of Guava 20). My uninformed guess is that javac wants to walk the type hierarchy so it can execute the right annotation processors, so when you have a class that extends ForwardingExecutorService it will try to read the annotations and fail when it gets to @CanIgnoreReturnValue.

I pushed a very simple example here, you can clone it and run mvn compile to reproduce the issue. The project just has a class that extends ForwardingExecutorService:

public class MyExecutorService extends ForwardingExecutorService {

  protected ExecutorService delegate() {
    return null;
  }
}

and a META-INF/services/javax.annotation.processing.Processor file to activate javac's annotation processing. Note that the service file points to a non-existent annotation processor and the build still fails with the CanIgnoreReturnValue error. This seems to suggest that the failure is happening while javac is setting up for annotation processing, rather than while the annotation processor is executing (eg, the failure is not related to the implementation of the annotation processor nor is there a way for an annotation processor to catch and ignore these errors).

I see that transitive dependencies have been added as part of Guava 22, which seems unfortunate but probably better than the alternative (especially in light of this issue). The other "fix" that comes to mind is to remove the class-level @CanIgnoreReturnValue annotation from ForwardingExecutorService and instead annotate the individual methods (in my testing this didn't cause the same compile error, but might cause the warnings that #2721 was referring to). Would it be possible to get a new release of Guava 20 and 21 with one of these (or some other) fix?

@cpovirk
Copy link
Member

cpovirk commented Jun 1, 2017

As a workaround, you can explicitly depend on the annotations from your own project: #2721 (comment) At least, I've been led to believe that that works.

I am conflicted about releasing a 20.0.1 and 21.0.1 because apparently our change to provide the dependencies in 22.0 caused people some trouble, too: #2824 + #2831

@jhaber
Copy link
Author

jhaber commented Jun 1, 2017

We have ~2,000 Maven modules that depend on Guava which complicates things a bit. Maven allows you to remove dependencies with exclusions but doesn't have a mechanism to do the opposite (add a new dependency to an artifact). If that existed, we could add the errorprone annotations dependency to Guava in our parent pom and have it apply to everything. But that's wishful thinking; in the meantime we just used the shade plugin to add an errorprone annotations dependency to Guava and depend on this version of Guava internally. For anyone who needs to do the same (I'm quite surprised no one has reported this issue yet) I put the POM in a gist.

To your second point, I don't like the idea of adding transitive dependencies to Guava (what if these dependencies in turn start adding their own transitive dependencies, and so on). But adding the dependencies seems better than the current state because, as I noted above, it is much easier (at least with Maven) to exclude an unwanted transitive dependency than it is to add a transitive dependency that doesn't exist. Anyone who doesn't want the new dependencies can simply add the appropriate exclusions (and if using dependencyManagement this only needs to be done in one place).

Other options that come to mind are to just remove the errorprone annotations which arguably don't belong in Guava, but it seems like that was already suggested and found unacceptable. A more extreme possibility is to release two versions of Guava. The first version would be like Guava 22; the classes have these extra annotations and there are transitive dependencies. The second version would have no transitive dependencies and in addition the class files would have these annotations stripped (to prevent weird issues like this one). I'm not sure if there's an existing Maven plugin that can do this specifically but it shouldn't be too hard to get working (it seems like a subset of the maven-shade-plugin functionality).

@Stephan202
Copy link
Contributor

@jhaber: If I understand your situation correctly you could also add error_prone_annotations to your parent-POM with the provided scope. That way it will be present compile-time but not be packaged. We do this in our project (~130 modules) for this dependency as well as others which contain only annotations (com.google.code.findbugs:jsr305, org.immutables:value). This also doesn't interfere with the dependency analysis performed by the maven-dependency-plugin, so it's okay if most/all modules don't actually use those dependencies.

ghost pushed a commit to jclouds/jclouds that referenced this issue Aug 25, 2017
This allows jclouds to compile with Guava 20.  Earlier versions
implicitly provided this.  Reference:

google/guava#2837
@ronshapiro ronshapiro added the type=other Miscellaneous activities not covered by other type= labels label Oct 4, 2017
@cpovirk
Copy link
Member

cpovirk commented Oct 4, 2017

Since we've seen less trouble with this than I expected, I feel OK with asking people to work around it. That's true especially because, if we release 20.0.1 and 21.0.1, people will probably upgrade and break in different ways (as noted above). But if other people are experiencing this, please speak up. Ditto if the workarounds don't work for you. It's possible that we could release something like a "20.0-rc99" that people could opt into but that most people wouldn't use because it's not considered an "upgrade" from 20.0.

@cpovirk cpovirk closed this as completed Oct 4, 2017
@jhaber
Copy link
Author

jhaber commented Oct 4, 2017

Just to follow up, we've since upgraded to 22.0. Having the dependencies listed and excluding the ones we don't want has been much easier on our end. And since it seems like this is going to be maintained going forward I don't think there's a pressing need to make any patch releases.

@cpovirk
Copy link
Member

cpovirk commented Oct 4, 2017

Thanks. I'm glad to hear that exclusion is working out better.

desht added a commit to desht/ModularRouters that referenced this issue Feb 19, 2018
Add provided dependency for error_prone_annotations to work around
  Guava 20 problem - google/guava#2837
@elharo
Copy link
Contributor

elharo commented Jul 13, 2018

I just hit this in googleapis/api-common-java

@cpovirk
Copy link
Member

cpovirk commented Jul 13, 2018

Thanks, and sorry. Does #2721 (comment) help?

@cpovirk
Copy link
Member

cpovirk commented Jul 13, 2018

I went and added this to the release notes for Guava 20 and 21, as I should have done a while ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type=other Miscellaneous activities not covered by other type= labels
Projects
None yet
Development

No branches or pull requests

5 participants