Store and use revision in tab runtime dependencies

[Bo98]

Should fix brew upgrade <pkg> producing broken installs. We were treating two packages with the same version but different revisions as compatible (because we didn't store revision info at all). This was often incorrect and lead to several reports of broken installs.

[MikeMcQuaid]

Thanks for the PR! This will fix the problem but I think we don't really need a new field and would be good to tighten this up so we don't ignore all existing tabs, only a subset (e.g. those where the dependency has a higher revision than noted or something)

[SMillerDev]

A bunch of people are reporting this every day. @MikeMcQuaid is there a chance to merge this, release and improve it further later?

[MikeMcQuaid]

A bunch of people are reporting this every day. @MikeMcQuaid is there a chance to merge this, release and improve it further later?

@SMillerDev Not as-is, no. I agree it's important though and, if no-one else has picked it up by Friday, I'll try to get it merged then.

[MikeMcQuaid]

@Bo98 @apainintheneck I've adjusted this to:

• also set pkg_version field in tabs, it's more useful to non-Homebrew consumers this way
• implement the conservative logic discussed above based on the table that @apainintheneck came up with
• tried to make the formula_installer logic more minimal and upgrade and dependency logic more similar and better documented
• fixed up tests and style where needed

[MikeMcQuaid]

@Bo98 @apainintheneck once you're both happy: can one of you merge this? Thanks ❤️

[Bo98]

I think this looks good, with this one logic tweak that should align with what was intended:

[Bo98]

(LGTM otherwise assuming 0.present? will indeed return true - I forget with that one).

[Bo98]

Also pushed so we Version.new wrap the version field since PkgVersion.new actually needs that rather than a string. Did so on the formula installer side as a type signature of T.nilable(Version) makes sense.

[MikeMcQuaid]

Thanks for help before and after, @Bo98!

[JL-EU]

Thanks for fixing these issues. When and how can we check if these updates will work? In my situation Homebrew/homebrew-core#153593

[Bo98]

It will be released probably on Monday. The fix won't do anything to people with already broken installs, however such installs are fixable by updating your out-of-date formulae (even before this fix is released).

It looks like your particular issue might be bug within platformio formula itself however - I would need to look into it.

[cho-m]

I think this may be causing current CI dependent failure when only revision bumping like in https://github.com/Homebrew/homebrew-core/actions/runs/6856551968/job/18644302924?pr=154195#step:3:244

Dependent test tries to install bottle for formula that was built in PR. Not sure if just checking formula.latest_version_installed? works or if there is better option.

[MikeMcQuaid]

Can you narrow this down to the specific line and, ideally, open a PR (even if it's not fully working yet)?

The formula.revision.zero? part at least is essential here: that's basically the point of all my changes to this PR.

[Bo98]

Fixed hopefully in #16220

[cho-m]

It should be that line with reproduction by local source-building a formula with extra revision bump then trying to install deps, e.g.

$ brew bump-revision --write-only python-click
$ brew install -s python-click
$ brew install --only-dependencies airshare
...
==> Downloading https://ghcr.io/v2/homebrew/core/python-click/manifests/8.1.7_1-1
curl: (22) The requested URL returned error: 404

==> Pouring python-click--8.1.7.sonoma.bottle.1.tar.gz
Error: /usr/local/Cellar/python-click/8.1.7_1 is not a directory

Verifying latest version/revision is installed does make error go away, e.g.

Suggested change

	elsif installed_version.version == minimum_version
	formula.revision.zero?
	elsif installed_version.version == minimum_version
	formula.revision.zero? || formula.latest_version_installed?