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

Fix it so upgrade from previous versions actually works #19354

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aysiu
Copy link

@aysiu aysiu commented Feb 22, 2025

Unless you delete these subdirectories before installing, Brew will still report the old version as being installed.

Fixes this issue: Mac installer packages do not upgrade Homebrew from older versions

Unless you delete these subdirectories before installing, Brew will still report the old version as being installed.
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Need to think of a different approach here.

Comment on lines +38 to +39
echo "Deleting ${brew_dir}/${subdir} for clean upgrade..."
/bin/rm -rf "${brew_dir:?}"/"${subdir}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't do this unconditionally because it'll mean that anyone with e.g. contributions to Homebrew they haven't pushed yet will be destroyed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't do this unconditionally because it'll mean that anyone with e.g. contributions to Homebrew they haven't pushed yet will be destroyed.

What's the minimum we'd have to destroy to get brew to actually report back the new version if you install a newer package?

@gromgit
Copy link
Contributor

gromgit commented Feb 24, 2025

I don't see how the code in this PR would run at all, as preinstall exits before it reaches it.

Also, if the intent is to update Homebrew when the installer .pkg is run on an existing installation, then I'd think simply running brew update would make more sense than nuking the existing files.

@MikeMcQuaid
Copy link
Member

Also, if the intent is to update Homebrew when the installer .pkg is run on an existing installation, then I'd think simply running brew update would make more sense than nuking the existing files.

👍🏻

@aysiu
Copy link
Author

aysiu commented Feb 24, 2025

I don't see how the code in this PR would run at all, as preinstall exits before it reaches it.

Good point, but if I move the exit 0 down, it also doesn't sound as if this PR will be accepted.

Also, if the intent is to update Homebrew when the installer .pkg is run on an existing installation, then I'd think simply running brew update would make more sense than nuking the existing files.

That also kind of defeats the whole point of a package, which is to deliver a payload. If the whole thing can be done by script, maybe the "package" itself should be a payload-free package and just script the installation or update of brew. But if that's the PR you'll accept, I'd rather have it actually update brew than be broken (which it currently is). I'm not a fan of unimplemented perfect being the enemy of implemented good.

Let me know what you'll accept. If the only way to get this PR accepted is to do a brew update, I'll push that commit instead.

@EricFromCanada
Copy link
Member

With a postinstall modified with set -x and some extra commands in this block:

# reset Git repository
cd "${homebrew_directory}"
git config --global --add safe.directory "${homebrew_directory}"
git show -1
git describe --tags
git reset --hard
git checkout --force master
git reset --hard $(git describe --tags --abbrev=0)
git branch | grep -v '\*' | xargs -n 1 git branch --delete --force || true
git show -1
git describe --tags
rm "${homebrew_directory}/.gitconfig"

This is the output on Sequoia arm64 when running the 4.4.22 installer over a 4.4.21 installation:

+ cd /opt/homebrew
+ git config --global --add safe.directory /opt/homebrew
+ git show -1
commit 89d1d6b8f4adb0b8f6678912f648dca2475b7c7c
Merge: b9eff75108 7880490f85
Author: Mike McQuaid <[email protected]>
Date:   Mon Feb 24 09:02:02 2025 +0000
    Merge pull request #19359 from Homebrew/no-attr_rw
    
    refactor: inline use of attr_rw
+ git describe --tags
4.4.22
+ git reset --hard
HEAD is now at 89d1d6b8f4 Merge pull request #19359 from Homebrew/no-attr_rw
+ git checkout --force master
Previous HEAD position was 89d1d6b8f4 Merge pull request #19359 from Homebrew/no-attr_rw
Switched to branch 'master'
++ git describe --tags --abbrev=0
+ git reset --hard 4.4.21
HEAD is now at 2e57f8f794 Merge pull request #19284 from zyoshoka/cask/url/remove-arch-placeholder
+ git branch
+ grep -v '\*'
+ xargs -n 1 git branch --delete --force
+ true
+ git show -1
commit 2e57f8f794af60804dcc185d1807c6d6ca10418c
Merge: 7436173473 20e33166e1
Author: Bo Anderson <[email protected]>
Date:   Mon Feb 17 05:23:07 2025 +0000
    Merge pull request #19284 from zyoshoka/cask/url/remove-arch-placeholder
    
    cask/url: remove arch placeholder when checking if unversioned
+ git describe --tags
4.4.21
+ rm /opt/homebrew/.gitconfig

When unmodified, this part of the script:

  1. enters the installation path and adds it to Git's safe.directory list
  2. hard-resets its contents, nuking any uncommitted changes
  3. does a force-checkout of the master branch (somehow this removes any memory of the latest tag, such that git reset --hard $(git describe --tags --abbrev=0) resets to the previous tag)
  4. deletes any branches other than the current

So this series of commands may need a rework to ensure the latest tag is checked out by the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants