ninxsoft / mist-cli

A Mac command-line tool that automatically downloads macOS Firmwares / Installers.
MIT License
620 stars 29 forks source link

Old betas in the list show up as newer than their stable counterparts #97

Closed webknjaz closed 2 years ago

webknjaz commented 2 years ago

$sbj.

Actual behavior

Just look at the log below:

$ mist list installer 12.5 --include-betas --compatible -q 
Identifier │ Name                │ Version │ Build    │ Size     │ Date       │ Compatible
───────────┼─────────────────────┼─────────┼──────────┼──────────┼────────────┼───────────
012-37355  │ macOS Monterey beta │ 12.5    │ 21G5063a │ 12.26 GB │ 2022-07-07 │ True      
012-42714  │ macOS Monterey      │ 12.5    │ 21G72    │ 12.41 GB │ 2022-07-20 │ True      

$ mist list installer 12.5 --include-betas --compatible -q --latest
Identifier │ Name                │ Version │ Build    │ Size     │ Date       │ Compatible
───────────┼─────────────────────┼─────────┼──────────┼──────────┼────────────┼───────────
012-37355  │ macOS Monterey beta │ 12.5    │ 21G5063a │ 12.26 GB │ 2022-07-07 │ True

12.5 beta is preferred over 12.5 stable even though the stable version is newer (2022-07-20 > 2022-07-07, and 012-42714 > 012-37355, and 21G72 > 21G5063a).

Expected behavior

I expected all the table entries to be sorted by the release date but it seems that the betas take precedence for some reason (I haven't yet checked the source code to see why).

Proposed solution

I think that --include-betas shouldn't behave like this. If one wanted betas to be preferable, why not have a --prefer-betas or --only-betas for that use-case?

I'm convinced that this is a bug in sorting with --include-betas set, and it needs to be corrected by fixing the ordering. As for other use-cases — they are probably valid and could be implemented with extra CLI options, although that would be out of the scope of the current bug report (being rather a feature request).

webknjaz commented 2 years ago

I don't see any code specifically prioritizing betas and quick grepping suggests that the problem is in the sorting logic at https://github.com/ninxsoft/mist-cli/blob/41c60ed/Mist/Helpers/HTTP.swift#L161 with a similar mechanism for firmwares at https://github.com/ninxsoft/mist-cli/blob/41c60ed/Mist/Helpers/HTTP.swift#L78. What's clear to me is that the date field isn't used for sorting, and the solution would be to make use of it.

Unfortunately, I don't know Swift nor do I have a dev env for experimentation so I won't be sending a PR to fix this :(

I hope this analysis helps someone to come up with a patch, though. Have a nice day!

ninxsoft commented 2 years ago

@webknjaz thank you for the detailed analysis! 🙌

For some strange reason, the current logic is as follows:

  1. Compare versions
  2. If versions are not the same, sort from highest to lowest
  3. If versions are the same, compare builds
  4. If the builds are of the same length, sort from highest to lowest
  5. If the builds are not of the same length, sort from longest to shortest <-- 🤯

If the sorting ever reaches Step 5, betas will always be sorted higher due to the length of the build string!

At first glance it seems like the easy fix is to just sort build strings letter by letter, which is just as simple as sorting the entire string from highest to lowest.

Updating from:

https://github.com/ninxsoft/mist-cli/blob/41c60eda5b461c7afd67bcbfe5b87aa702c8f8f7/Mist/Helpers/HTTP.swift#L161

To:

products.sort { $0.version == $1.version ? $0.build > $1.build : $0.version > $1.version }

Yields the following results:

➜ ~ mist list installer "12.5" --include-betas --quiet
Identifier │ Name                │ Version │ Build    │ Size     │ Date       │ Compatible
───────────┼─────────────────────┼─────────┼──────────┼──────────┼────────────┼───────────
012-42714  │ macOS Monterey      │ 12.5    │ 21G72    │ 12.41 GB │ 2022-07-21 │ True      
012-37355  │ macOS Monterey beta │ 12.5    │ 21G5063a │ 12.26 GB │ 2022-07-08 │ True      

Which works great for macOS Monterey 12.5, but not so great for all installers, eg. macOS Catalina 10.15.7:

➜ ~ mist list installer "10.15.7" --include-betas --quiet 
Identifier │ Name                   │ Version │ Build    │ Size     │ Date       │ Compatible
───────────┼────────────────────────┼─────────┼──────────┼──────────┼────────────┼───────────
001-57224  │ macOS Catalina         │ 10.15.7 │ 19H4     │ 08.75 GB │ 2020-10-28 │ False     
001-51042  │ macOS Catalina         │ 10.15.7 │ 19H2     │ 08.75 GB │ 2020-09-25 │ True      
001-68446  │ macOS Catalina         │ 10.15.7 │ 19H15    │ 08.75 GB │ 2020-11-12 │ True      

It appears the numerical digits after the letter in the build string should determine the sort order, which is possibly why the current rudimentary logic assumes a longer string will have a higher number at the end of the build string.

The build number suffixes also coincide with the release date.

webknjaz commented 2 years ago

Slightly harder (more robust?) fix: sort by build strings up until the first letter, then compare and sort the remaining integer values (and also factor in an optional lowercase letter for betas)

Have you considered using the first (identifier) column it seems to be monotonous. I'd assume that splitting it by the dash and turning into tuple of integers would make the comparison easier, no?

ninxsoft commented 2 years ago

Simply sorting by identifiers will not work, and sorting by the 2nd half of the tuple as you suggested might work, but there doesn't seem to be any documentation I am aware of that explains what the identifiers actually mean.

I am definitely leaning towards the approach of sorting by version, then by date for each version.