brew icon indicating copy to clipboard operation
brew copied to clipboard

linux/diagnostic: add check for versioned GCC linkage

Open carlocab opened this issue 3 years ago • 9 comments

  • [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?
  • [ ] 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?
  • [ ] Have you successfully run brew tests with your changes locally?

This complements my other two GCC-on-Linux PRs (#13631, #13633), however they are both reliant on bottles eventually being (re-)poured.

Let's try to speed that up by returning an error message from brew doctor whenever a user has formulae installed that would benefit from a brew reinstall.

carlocab avatar Aug 03 '22 13:08 carlocab

Review period will end on 2022-08-04 at 13:00:15 UTC.

BrewTestBot avatar Aug 03 '22 13:08 BrewTestBot

As before, I still need to test this. But this is the proof-of-concept.

carlocab avatar Aug 03 '22 13:08 carlocab

Is this for the scenario where the user doesn't use brew update?

Bo98 avatar Aug 03 '22 13:08 Bo98

I'll admit to not having thought too carefully about failure modes yet: my motivation is that it would be good to provide as many paths to brew reinstall as possible to minimise breakage.

Something like this (omitting the #core_tap? check) might help for your concern over source-built formulae from https://github.com/Homebrew/brew/pull/13633#issuecomment-1203951369, though.

carlocab avatar Aug 04 '22 00:08 carlocab

Review period ended.

BrewTestBot avatar Aug 04 '22 15:08 BrewTestBot

Benchmarked:

Before

Command Mean [s] Min [s] Max [s] Relative
brew doctor 5.248 ± 0.053 5.166 5.328 1.00

After

Command Mean [s] Min [s] Max [s] Relative
brew doctor 2.915 ± 0.033 2.862 2.957 1.00

This is on a system with about ~430 formulae installed, which is pretty extreme.

This means brew doctor is nearly twice as slow as before, which is pretty bad. OTOH, it didn't take long at all to run before, so it's not too bad in the end.

carlocab avatar Aug 05 '22 17:08 carlocab

This means brew doctor is nearly twice as slow as before, which is pretty bad. OTOH, it didn't take long at all to run before, so it's not too bad in the end.

Yeh, I think that's too slow to run for everyone every time 😭.

MikeMcQuaid avatar Aug 05 '22 18:08 MikeMcQuaid

Running some slightly more comprehensive benchmarks here: https://github.com/carlocab/workflow-test/actions/runs/2805207136/attempts/1#summary-7696068383

One of the runs is still going, but the ones that finished are a bit all over the place.

I'm ok to leave this be for now. We can reconsider it depending on the number of users who report issues that might be helped by this.

carlocab avatar Aug 05 '22 18:08 carlocab

I'm ok to leave this be for now. We can reconsider it depending on the number of users who report issues that might be helped by this.

Shouldn't this be handled automatically by brew update? If so, can we check if the setting is set, make this a no-op if so and tell them to run brew update if anything fails? Similarly, we could make it fail the first time we find a single failure rather than getting all the failures from all formulae.

MikeMcQuaid avatar Aug 05 '22 18:08 MikeMcQuaid

Let's ship this and revert if it's causing issues/too slow.

MikeMcQuaid avatar Aug 23 '22 10:08 MikeMcQuaid