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

Permit Guava to be used with FindBugs 2.0.0 by altering the JSR-305 dependency. #1018

Closed
gissuebot opened this issue Oct 31, 2014 · 19 comments
Closed

Comments

@gissuebot
Copy link

Original issue created by diwakergupta on 2012-05-29 at 08:14 PM


As of 12.0, Guava is still using FindBugs 1.3.9 and specifically the JSR 305 annotations artifact. FindBugs 2.0.0 has been out for many months now and as their website says:

"Anyone currently using FindBugs 1.3.9 should find FindBugs 2.0 to largely be a drop-in replacement that offers better accuracy and performance."

Furthermore, projects using FindBugs 2.0.0 with Guava 12 will run into warnings about duplicate class files when generating packages using Maven. This is because in FindBugs 2.0.0, the JSR305 annotations are part of the core annotations.jar and a separate artifact is not required.

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-05-30 at 07:49 PM


(No comment entered for this change.)


Labels: Type-Dev

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-06-19 at 05:44 PM


We'll investigate upgrading the external build to use the new findbugs jar. Also, should we have declared this as a compile-time-only dependency? That might prevent this from being a problem that affects anyone.


Status: Acknowledged

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-06-22 at 06:16 PM


(No comment entered for this change.)


Status: Research

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-06-22 at 06:57 PM


(No comment entered for this change.)


Labels: Package-General

@gissuebot
Copy link
Author

gissuebot commented Nov 1, 2014

Original comment posted by mfu...@us.ibm.com on 2012-08-03 at 06:39 PM


Hi Kevin,

I noticed that Chris set the "provided" scope on the jsr305 artifact. Judging from the way the annotations are used, the scope should actually be "compile" and you should add the <optional>true</optional> tag. (That way, if someone wants to compile and perform static analysis on guava, he or she would have the option of adding the dependency. However, people who using the guava jar don't need the artifact.)

Regards,
-Matt

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-08-03 at 07:49 PM


Christian, what do you think of this?


Owner: cgruber@google.com

@gissuebot
Copy link
Author

Original comment posted by cgruber@google.com on 2012-08-07 at 08:04 PM


Guava's use of JSR-305 is a bit of an edge-case in maven, with respect to scoping and optionality of dependencies.

Optional is intended to be used by code that may have several sets of bytecode needed to compile, but only one of which might be used at run-time, so optional lets you cut the transitivity off, so that the down-stream projects can (and must) declare the one of their choosing). Key use-cases of this would include compiling a framework against several back-end database drivers, but only needing whichever one is actually used at runtime, or several caching backends, etc.

Provided is intended to be used by projects that need dependencies at compile-time, but at run-time these will be provided by some other means, most notably by the JDK or by a container, etc. Key use-cases of this would be javax.servlet, etc.

In practice, they both behave identically.

Guava's use is about halfway in between, and is still different. We use findbugs to provide an annotation whose bytecode does NOT have to be present at runtime, and if needed, will be provided by some other means. But we're not choosing from multiple options, and we're not necessarily running in the container. In our case, either mechanism behaves in exactly the same way.

I chose "provides" scope because I knew precisely what behaviour would result (down-stream dependents would have to declare it if they need it), and it was sort of a 50/50 decision on which way to go. And in our case, we don't even need any bytecode for @Nullable to exist at runtime because it's an annotation. So... meh?

I don't see a problem in changing this to compile/optional, but I also don't see a payoff. The dependency resolution mechanism will perform exactly the same way - it'll be present at compile/test time, and not present in the transitive closure of down-stream dependents.

I'm closing this on the basis of inertia, unless anyone cares enough to make an impassioned plea for optional that shows substantial superiority.


Status: WorkingAsIntended

@garretwilson
Copy link

@gissuebot , thanks for that explanation of why Guava chose "optional" instead of "provided". FWIW, I've had a long, hard thought about which way to go with our own libraries regarding JSR 305. As you note, they will both have the same effect. But it seems to me that semantically "optional" fits better for us. Think about it this way: why would consumers want JSR 305 annotations at runtime? They may may have a tool that does post-compile analysis. Or maybe the consumer wants to produce documentation based upon the JSR 305 annotations. The tool would need access to these annotations.

Ah, but you might say that the tool will provide them. Yes, but I think that Maven's "provided" gives the expectation that there must be some container providing them (e.g. servlets, as you mentioned). In this case, even the "provided" part is "optional". I therefore think that "optional" provides the closest semantics; they won't be included unless users want them --- they aren't "required but automatically provided by a container".

In your explanation you indicate that "optional" is used to choose among "several sets of bytecode". But that's not always the case; perhaps you are confusing "optional" with "having an option among choices, and you must choose one". There are many use cases of "optional" for which things are entirely optional: if the dependency is present, something happens; otherwise nothing happens.

In any case, I'm not saying you made the wrong choice; I'm just saying that I thought about it a lot and I think we'll go with "optional", and I'm putting my thoughts here in case they are useful to someone else. Cheers!

@jbduncan
Copy link
Contributor

@garretwilson I wonder if I'm missing something with regards to your comment above, because Guava has had both a compile-time and runtime dependency on JSR305 annotations since 31st June (0e29934). Furthermore, I believe that the Guava team is considering removing or replacing them with alternative annotations to accommodate Java 9 (#2960).

Would you mind kindly clarifying the purpose of your comment above for me?

@garretwilson
Copy link

garretwilson commented Oct 28, 2017

… Guava has had both a compile-time and runtime dependency on JSR305 annotations since 31st June …

Eep! Really? What Guava version is that you're referring to?

I've been reading https://stackoverflow.com/a/24264330/421049 and doing some intense research in https://stackoverflow.com/q/45596949/421049 and thinking long and hard on how best to include JSR 305 in our own library, based on Guava optionally including it (historically using "provided"). Now you're telling me that Guava has changed its mind altogether and is now including JSR 305 transitively, so that all consumers bring in the JSR 305 dependency automatically? Just so I make sure I'm understanding correctly…

@jbduncan
Copy link
Contributor

jbduncan commented Oct 28, 2017

Now you're telling me that Guava has changed its mind altogether and is now including JSR 305 transitively, so that all consumers bring in the JSR 305 dependency automatically?

Yes, I believe so.

What Guava version is that you're referring to?

If this comment is to believed, it looks like Guava versions 22.0+ are the ones I'm talking about. (And indeed, the release page for 22.0 confirms this. 😃)

(On a side note, it also occurs to me that Guava changed their dep on JSR305 from "provided" to "optional" some time between when this issue was closed and when 22.0 was released, as according to 0e29934, the dep was last set to <optional>true</optional> as opposed to <scope>provided</scope>. I'm unclear on when they made that change, though. 🤔)

Am I right to understand that (1) you're on a Guava version older than 22.0 and (2) you're trying to make sure you have an explicit compile-time and runtime dependency on JSR305?

If so, then upgrading to Guava 23.3 (the latest version at the time of me writing this), or at least some 22.0+ version, should resolve your problem automatically. 😄

@garretwilson
Copy link

Wow, thanks for notifying me of the latest developments. So it turns out that Guava had indeed eventually agreed with what I wrote above, I guess, and switched to using <optional> rather than a scope of provided! That had made more sense to me. But I never knew about that, and nobody mentioned it on the numerous Stack Overflow discussions I mentioned above. Curious. So definitely thanks for alerting me to this.

Am I right to understand that (1) you're on a Guava version older than 22.0 and (2) you're trying to make sure you have an explicit compile-time and runtime dependency on JSR305?

You're right about the first part, but the reason why I'm investigating this has nothing to do with not getting JSR 305 in a project. Rather we have our own libraries which use JSR 305, and I am debating whether to make their use of JSR 305 optional or not; I was using Guava's experience to help guide my own decision.

After much reading, researching, and contemplating, I was ready to make a final decision and settle on using <optional>, even though Guava (I thought) had settled on provided. Now Guava's latest move throws everything into doubt again. It seemed pretty evident to me that <optional> was the right way to go, because users may want to use our general library without pulling in another dependency (as with Guava). And finding out that Guava had switched to <optional>, too, was somewhat of a validation of my decision. But now why is Guava throwing out "optional" altogether? Does it have something to do with moving towards Java 9 modules, which may not support optional-ness? This new turn of events has confused me again.

@garretwilson
Copy link

Oh, here is why they are non-optional now: #2721

But interestingly that issue has to do with a separate set of annotations, not the JSR 305 annotations.

@jbduncan
Copy link
Contributor

jbduncan commented Oct 28, 2017

But interestingly that issue has to do with a separate set of annotations, not the JSR 305 annotations.

Yes, that's right. But IIUC the whole conversation on that issue, I believe it was (or still is) applicable not only to those specific annotations (error-prone's annotations, I believe), but to all "optional" and "provided" annotations in general.

I'm pretty sure that it was because the issue applied to all "optional"/"provided" annotations that Guava depended on, rather than just error-prone's annotations, that the Guava team decided to make all of Guava's annotation deps "compile" scope.

@jbduncan
Copy link
Contributor

jbduncan commented Oct 28, 2017

And unfortunately, Java 9 throws in a whole other can of worms, with regards to Guava's dependency on the JSR305 annotations specifically. Because of how JPMS modules work, it may prompt the Guava team to drop JSR305 altogether or adopt a different annotations library.

See issue #2960 for the gory details. :(

@garretwilson
Copy link

Yeah, I saw that, but didn't quite understand the details. I'm not yet up to speed on Java 9 modules. Sheesh, why couldn't they have simply finished JSR 305 and made it part of Java? (Trying not to go into a rant here.)

Well thanks for all the info --- be sure to add anything else you think of. I'm now in a holding pattern deciding which way to go with JSR 305 in our own library.

@jbduncan
Copy link
Contributor

@garretwilson You're very welcome! And I'm very sorry to hear that I inadvertently made things more confusing for you.

I wish you best of luck, and I hope you can come to a reasonable conclusion in the near future!

I'm not sure if there's anything else to add, but I'd recommend that you follow #2960 so that, even if the details go over your head, you have some consensus to refer to if it reaches a solid conclusion. :)

@cpovirk
Copy link
Member

cpovirk commented Oct 28, 2017

Just to summarize from #2721: We want "present at compile time but not at runtime." But optional (and probably provided?) means "present at Guava compile time but not at users' compile time or runtime."

@garretwilson
Copy link

"present at compile time but not at runtime."

…meaning "present at users' compile time but not at users' runtime". That's an interesting state of affairs --- it's like a scope Maven doesn't provide. But as I mention in #2721, it's unclear whether such a scope would really be needed. Anyway, thanks for helping make this clearer! It will help me come to a decision of what to do in my own library. Cheers!

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

No branches or pull requests

5 participants