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

JDiff links point to non-matching JDIFF output #7765

Open
2 tasks done
reschke opened this issue Apr 7, 2025 · 5 comments
Open
2 tasks done

JDiff links point to non-matching JDIFF output #7765

reschke opened this issue Apr 7, 2025 · 5 comments
Labels
P3 no SLO status=triaged type=defect Bug, not working as expected

Comments

@reschke
Copy link

reschke commented Apr 7, 2025

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.

@reschke reschke added the type=defect Bug, not working as expected label Apr 7, 2025
@cpovirk
Copy link
Member

cpovirk commented Apr 7, 2025

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
Copy link
Member

cpovirk commented Apr 8, 2025

(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
Copy link
Member

cgdecker commented Apr 9, 2025

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
Copy link
Member

cpovirk commented Apr 9, 2025

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

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

@cgdecker
Copy link
Member

cgdecker commented Apr 9, 2025

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 no SLO status=triaged type=defect Bug, not working as expected
Projects
None yet
Development

No branches or pull requests

3 participants