resolve_latest_keg should return the latest HEAD keg when no stable kegs exist

[cnnrmnn]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Closes #11741.

Currently, resolve_latest_keg has the following behavior:

• If only one keg exists (HEAD or stable), return that keg.
• Otherwise, return the latest stable keg.

This behavior doesn't handle the case where there are multiple HEAD kegs, but no stable kegs. This PR adds the following behavior to resolve_latest_keg:

• If there are HEAD kegs but no stable kegs, return the latest HEAD keg.

[BrewTestBot]

Review period will end on 2021-08-05 at 22:58:17 UTC.

[cnnrmnn]

The test is failing on CI (but passing for me locally) because at the moment, comparing two HEAD versions returns 0. I suspect the best way to deal with this would be in resolve_latest_keg. We could use HeadVersion#commit to determine which version is newer. Alternatively, a less sophisticated approach would be picking the keg with the most recently modified directory. What do you think? @Rylan12

[Rylan12]

I can take a closer look late tomorrow night (might not be until Friday—traveling today).

Formula#latest_head_version uses the modification time of the tab to determine which HEAD version is the latest:

brew/Library/Homebrew/formula.rb

Lines 549 to 558 in 0fc7762

	def latest_head_version
	head_versions = installed_prefixes.map do |pn|
	pn_pkgversion = PkgVersion.parse(pn.basename.to_s)
	pn_pkgversion if pn_pkgversion.head?
	end.compact
	head_versions.max_by do |pn_pkgversion|
	[Tab.for_keg(prefix(pn_pkgversion)).source_modified_time, pn_pkgversion.revision]
	end
	end

I think it would be appropriate to use the same logic here (ideally this could be done without duplicating code but it's possible some refactoring would need to be done to support that)

[cnnrmnn]

I think it would be appropriate to use the same logic here (ideally this could be done without duplicating code but it's possible some refactoring would need to be done to support that)

Thanks for pointing me towards Formula#latest_head_version. Will use same logic for this issue.

[BrewTestBot]

Review period ended.

[cnnrmnn]

I included the source_modified_time code here directly since it is so brief. If you think this should be encapsulated elsewhere for use in resolve_latest_keg and latest_head_version, I'm happy to make changes. @Rylan12

[Rylan12]

@cnnrmnn I think this is fine. I'm not sure it really makes much sense to extract what's essentially #source_modified_time to its own method.

Just making sure, the revision secondary sort from Formula#latest_head_version isn't necessary here, right?

[cnnrmnn]

I'm not sure that the revision secondary sort is needed anywhere we're dealing exclusively with HEAD versions. If you check out the parsing function for PkgVersion, you'll see that it parses the revision using a regex. HEAD paths (e.g., HEAD-6974ce8) matched to that regex will always set revision to nil. Any HEAD version should have a revision equal to 0 since to_i is called on the parsed nil revision.

Am I missing anything?

[Rylan12]

Gotcha makes sense. So it could probably be removed from latest_head_version too, right? Probably not worth doing but just curious if there's a difference.

[cnnrmnn]

So it could probably be removed from latest_head_version too, right?

Yep, should be fine to remove. I can do that in a quick PR as to not confuse anyone dealing with that code in the future.

[Rylan12]

You definitely don't need to but it probably wouldn't hurt. Plus it will expose more maintainers to it in case anyone knows something we're missing and it turns out it is necessary

[cnnrmnn]

@Rylan12 Did some quick digging, and found a spec that tests HEAD versions with revisions. Do you know if there's any scenario where a HEAD version would actually have a revision?

I would think so given that the above test exists, but I'm still curious. If you don't know, I can do a little more digging and restore the secondary sort here if appropriate.

[Rylan12]

Ah, good call. If you do brew install --HEAD on a formula that's specified a revision it will show up as HEAD-6974ce8_3 (which means there will be a revision), so I think it does need to be added here.

[cnnrmnn]

Gotcha, will push a fix shortly.

[carlocab]

Makes sense to me. Thanks @cnnrmnn!