brew icon indicating copy to clipboard operation
brew copied to clipboard

Sort printed package list in `brew upgrade`

Open jesboat opened this issue 4 months ago • 5 comments

At the beginning of a brew upgrade run, brew prints the list of packages to be upgraded e.g. as

ncdu 2.2.2 -> 2.3
nmap 7.94 -> 7.94_1
jq 1.6 -> 1.7.1
sdl2 2.26.5 -> 2.30.0
libusb 1.0.26 -> 1.0.27
[email protected] 3.11.3 -> 3.11.7_1
file-formula 5.44 -> 5.45

The order is apparently arbitrary (I think maybe FS readdir order?) which makes it difficult to easily eyeball the list to see if some specific package of interest is included. Instead, sort it.

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

jesboat avatar Feb 20 '24 21:02 jesboat

(Suggestion: sure.)

While I've got you here... The cask upgrade list has the same problem. Adding a sort there is easy enough. The weird thing is that brew outdated prints the outdated formulae and then the outdated casks, and they're no separation between the lists. You can end up with something like:

dune (3.7.1) < 3.14.0
fmt (9.1.0, 10.0.0, 10.2.1) < 10.2.1_1
go (1.20.3, 1.20.5, 1.21.7) < 1.22.0
...
[email protected] (3.12.1_1) < 3.12.2
unbound (1.17.1, 1.19.0) < 1.19.1
vim (9.1.0050) < 9.1.0100
dotnet-sdk (8.0.101,4d6f...) != 8.0.201,ca83...

I can't think of a solution to this which doesn't suck in at least one way.

Interleaving the lists would require some horrific interplay between Homebrew::upgrade calls upgrade_outdated_formulae which prints the formula list and Homebrew::upgrade calls upgrade_outdated_casks calls Cask::Upgrade.upgrade_cask

Sorting the lists independently, and then printing it consecutively leads to an output like above, where the overall output of brew outdated appears sorted... except for one misplaced item.

The least objectionable I can think of code-wise would be for upgrade_outdated_formulae to return "yes there was something outdated" to Homebrew::upgrade; then Homebrew::upgrade passes a boolean to upgrade_outdated_casks which passes it to Cask::Upgrade.upgrade_cask which adds a newline before the outdated list. Code-wise it's gross and makes me sad, but I can't think of a behavior (other than one list after the other, with no separation) which is less gross, and at least with this, the grossness is limited to the single boolean. But then there's also the problem that brew oudated's output now differs from before because there can be blank lines, and I can see that breaking somebody who's trying to use brew outdated in a script.

jesboat avatar Feb 21 '24 15:02 jesboat

I think we can just add some headings to the brew outdated output, as long as we don't break brew outdated | xargs ... and brew outdated --json output, possibly by reusing the output_dump_formula_or_cask_report method from brew update-report.

reitermarkus avatar Feb 21 '24 16:02 reitermarkus

The order is apparently arbitrary (I think maybe FS readdir order?)

It's only arbitrary when you run brew upgrade with no arguments. As-is this PR will mean if I do brew upgrade wget ack it'll always try to upgrade ack first which is not correct.

Instead, I'd suggest you do something like change Formula.installed.select to have a trailing .sort (like @reitermarkus suggests but in a different location) to only sort the zero-argument case.

I think we can just add some headings to the brew outdated output

👎🏻 from me. I don't think it's necessary to communicate that there are formulae or casks separately in the output. Homebrew is increasingly trying to make the use of either mostly transparent to the casual user.

Regardless: let's not blow up the scope of this PR and keep it focused just on formulae for now and casks can be handled/combined later.

MikeMcQuaid avatar Feb 22 '24 09:02 MikeMcQuaid

I do brew upgrade wget ack it'll always try to upgrade ack first which is not correct.

Why would this matter? I would not expect this to necessarily follow the argument order.

reitermarkus avatar Feb 23 '24 11:02 reitermarkus

Why would this matter? I would not expect this to necessarily follow the argument order.

  1. because it's the way things have always been done and I'd rather not change it for no good reason
  2. because of how errors are handled

MikeMcQuaid avatar Feb 23 '24 14:02 MikeMcQuaid

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Mar 16 '24 00:03 github-actions[bot]

Passing on this, sorry.

MikeMcQuaid avatar Mar 18 '24 12:03 MikeMcQuaid