Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Petition to remove brew upgrade from install script #12794

Closed
brorbw opened this issue May 3, 2021 · 9 comments · Fixed by #12921
Closed

Petition to remove brew upgrade from install script #12794

brorbw opened this issue May 3, 2021 · 9 comments · Fixed by #12921

Comments

@brorbw
Copy link

brorbw commented May 3, 2021

The line to upgrade brew in the install script for macOS can have unintended consequences for users. In my case it broke some of my installs. I can see that it ignores pinned versions but to be honest, nobody really uses this feature.

If this is not doable - there should be a promt to inform the user that the script will run brew upgrade and the ability to opt out!

The line in question:
https://github.com/qmk/qmk_firmware/blob/master/util/install/macos.sh#L12

@brorbw brorbw changed the title Pertition to remove brew upgrade from install script Petition to remove brew upgrade from install script May 3, 2021
@askreet
Copy link
Contributor

askreet commented May 4, 2021

I just ran into this as well, big +1 from me. My local dev environment for work has some packages pinned and this nearly caught me by surprise. Fortunately when I saw it was updating 45 packages from homebrew I aborted it and started digging.

@askreet
Copy link
Contributor

askreet commented May 4, 2021

In case anyone else lands here and is curious, this seems to be enough to get buildable on master:

brew update
brew upgrade qmk/qmk/qmk # or presumably install if fresh
brew link --force avr-gcc@8
brew link --force arm-gcc-bin@8
python3 -m pip install -r $QMK_FIRMWARE_DIR/requirements.txt

@bb2020
Copy link

bb2020 commented May 16, 2021

+1

It can be replaced with brew update && brew outdated just to remind about outdated packages, but without upgrading them.

@fauxpark
Copy link
Member

If running brew upgrade breaks your install, then you might have bigger problems.

The problem is that some of our dependencies must be relatively up to date before attempting install, so I don't see this line going anywhere anytime soon.

@brorbw
Copy link
Author

brorbw commented May 16, 2021

@fauxpark not really - brew is not that smart. I would suggest updating the specific dependencies and not everything if it's really necessary. Or checking the version of the dependencies and advising the user to update.

@skullydazed
Copy link
Member

We're facing several problems here and there's no good way to make everyone happy. Let's talk about the issues we're facing.

To start with howebrew is really not designed to "stay back". Yes, you can pin versions but there are situations where those versions will still be upgraded (like when they are a dependency for another program that is being upgraded.) Another problem is that you can very easily find yourself in a state that can't be rebuilt if you have a problem that requires moving to a new machine. These are not QMK specific but they are issues you should consider before pinning homebrew packages.

For an issue more relevant to QMK we have the support problem. We've found that every time we make a step optional we get more people popping up on discord asking why something is broken after they said no. As an entirely volunteer-driven project we are very sensitive to anything that takes volunteer time away from things like reviewing PRs.

Then there's my personal experience- over my career I've spent too many hours dealing with conflicting packages and broken servers after some software package updated everything for me without notice. Believe me when I say I've experienced your pain and am re-experiencing it now second hand. It would be my preference that we never require the user to update before installing the software necessary for QMK.

This brings us to the current situation- why are we running brew upgrade even though many of the volunteers within QMK would prefer not to? Because it causes the fewest problems all around.

For everyone in your position there are easily 5 or 10 people who have avoided problems because that brew upgrade is there. In this specific case we are following one of homebrew's philosophies:

it forces a lot of opinionated decisions on users if the maintainers think it benefits the majority

While we are not likely to change the core decision (if you feel you have a particularly strong argument we are always willing to hear it out) there is always room for improvement. The messaging in the tutorial and/or script can be improved as we work to find the right balance between presenting all relevant information and avoiding a wall of text that is completely ignored. There are possibly other changes that can improve the experience for people who are impacted by decisions that affect a minority of users.

It's clear this is an itch for some of you, I encourage you to start to scratch it with PR's that improve the situation without disrupting how it works for most users.

@brorbw
Copy link
Author

brorbw commented May 16, 2021

I've started a PR. I intended to open a draft PR but it appears that I can't do that. Anyhow I want to welcome anyone to comment on the PR.

@skullydazed thank you for your input. In regards to the "brew philosophies" there is a good saying were I'm from that you should not wash your hands in someone else's filth.

@stale
Copy link

stale bot commented Aug 21, 2021

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

@stale stale bot added the stale Issues or pull requests that have become inactive without resolution. label Aug 21, 2021
@brorbw
Copy link
Author

brorbw commented Aug 22, 2021

#12921

@stale stale bot removed the stale Issues or pull requests that have become inactive without resolution. label Aug 22, 2021
@brorbw brorbw mentioned this issue Sep 27, 2021
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants