JDiff links point to non-matching JDIFF output

[reschke]

Guava Version

33.4.6-jre

Description

In https://github.com/google/guava/releases, the links to JDiff output is broken.

Example

The link "33.4.6-jre vs. 33.4.5-jre" links to the JDIFF "Between Guava 33.3.1-jre and Guava 33.4.6-jre".

Expected Behavior

Either fix link descriptions or link targets.

Actual Behavior

The link "33.4.6-jre vs. 33.4.5-jre" links to the JDIFF "Between Guava 33.3.1-jre and Guava 33.4.6-jre".

Packages

No response

Platforms

No response

Checklist

• I agree to follow the code of conduct.

• I can reproduce the bug with the latest version of Guava available.

[cpovirk]

Thanks!

I had noticed the second half ("Between Guava 33.3.1-jre and Guava 33.4.6-jre") and convinced myself that that made some sense: A patch release wouldn't contain any API additions or removals, so we might as well diff against the most recent version that we would actually have changes relative to. I still found it little surprising.

And maybe part of the reason that I found it surprising is the inconsistency that you've pointed out, which had gone entirely over my head. Clearly we should say either the one thing or the other :)

After actually looking into this for a moment, I'm guessing that the issue is that I never updated

guava/_util/util.sh

Lines 265 to 266 in 87933ba

	ceiling_major="$(cut -d. -f1 <<< "$ceiling_expanded")"
	ceiling_minor="$(cut -d. -f2 <<< "$ceiling_expanded")"

when I started publishing patch releases.

@cgdecker, does that sound right to you?

[cpovirk]

(And this is particularly vexing right now, given that JDiff thinks that lots of APIs changed since 33.3.1, thanks to its poor understanding of type-use annotations.)

[cgdecker]

I don't know if this is the right thing to do, but it looks like the comparison to the most recent version that has a lower major or minor version is intentional:

# Prints the highest non-rc release from the sorted list of releases produced
# by sort_releases. If a release argument is provided, print the highest non-rc
# release that has a major or minor version that is lower than the given
# release. For example, given "16.0.1", return "15.0". Given "16.1", return
# "16.0.1".

[cpovirk]

Ah, thanks. (Who would ever have thought to read the docs? :))

I'm not sure what's best, either.

[cgdecker]

I think the intention was definitely to show the diff from the last version that should actually have API differences, which generally makes sense to me (in theory there should be no meaningful JDiff at all between patch versions, though I suppose things that wouldn't directly be potentially-breaking changes like some annotation changes could be in patches). Maybe we can just change the code that generates the release notes to say what it's actually doing?

[reschke]

Next Q:

https://guava.dev/releases/33.4.7-jre/api/diffs/changes/com.google.common.collect.TreeBasedTable.html

claims that two methods were removed, but that does not seem to be the case. What's going on?

[cpovirk]

What's going on is that JDiff is unmaintained and somewhat bad at its job :) We should really replace it with japicmp. (There used to be another tool that someone used to publish reports for Guava, but I think it broke with our -jre/-android split.)

What's going on here specifically seems to be that JDiff doesn't recognize that TreeBasedTable still inherits methods like rowKeySet from StandardRowSortedTable after #7660.

[cpovirk]

Maybe we can just change the code that generates the release notes to say what it's actually doing?

That sounds reasonable.

I could even see just omitting the link to the JDiff from the release notes for patch releases. (We could even avoid generating the diffs in the first place, but omitting the link is easier.)

[hadiseShaaban]

Hi! I'd love to contribute to this feature. Comparing lists regardless of order sounds useful. I can help implement and write tests if it's still open. Let me know if you'd like me to proceed! 😊

[mrunalinidudhagawali8]

Hey! I will be more than happy to work on this issue as part of my first contribution to Guava. Since it's still open and unassigned, I’m happy to take it up without being formally assigned. Please let me know if there’s anything I should be aware of before I begin. Thanks!

[mrunalinidudhagawali8]

Just following up on this PR: #7798, and I have tested the changes locally. Let me know if there are any additional steps you'd like me to take. Happy to move this forward! Thanks!