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: Clean up all previous finch version installation registries in postinstall and uninstall #688

Merged
merged 2 commits into from
Nov 8, 2023

Conversation

KevinLiAWS
Copy link
Contributor

Issue #, if available:
#687
Old finch installation registry remains if uninstall script not explicitly called, the mac will treat a new installation with upgrade info message.

Description of changes:
Added a full clean up step in the postinstall script to remove all old registries except the current installed version. Also make the uninstall script not only remove the current but also remove all historical registry record.

Testing done:
Tested with cloud build: https://github.com/runfinch/finch/actions/runs/6791189430
Also validated locally with pkg installation:

// Before install the fixed pkg, it print all finch registries
% pkgutil --pkgs | grep '^org\.Finch'
org.Finch.v0.8.0
org.Finch.pkg-tool
org.Finch.v0.7.0
org.Finch.v0.6.2
org.Finch.main
org.Finch.0.6.0
org.Finch.0.4.1
org.Finch.0.4.0

// After install the fixed pkg, it only has the current registry
% pkgutil --pkgs | grep '^org\.Finch'
org.Finch.fix-unregistry

// After execute uninstall, all the registries are removed
% pkgutil --pkgs | grep '^org\.Finch'
% // empty
  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@KevinLiAWS KevinLiAWS marked this pull request as ready for review November 7, 2023 23:15
@KevinLiAWS KevinLiAWS self-assigned this Nov 7, 2023
Copy link
Member

@pendo324 pendo324 left a comment

Choose a reason for hiding this comment

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

This will work as long as we only allow one version of Finch to be installed per machine (which we do now, so no concerns). Just something to think about if we ever allow changing the install location, or per-user installs.

installer-builder/darwin/scripts/postinstall Show resolved Hide resolved
@KevinLiAWS
Copy link
Contributor Author

KevinLiAWS commented Nov 8, 2023

This will work as long as we only allow one version of Finch to be installed per machine (which we do now, so no concerns). Just something to think about if we ever allow changing the install location, or per-user installs.

For this concern:

  1. pkg forget always need admin permission
  2. mac os standard pkg installation is system level, if we support user level installation, pkgutil will be not in the scope, it only maintains system level registry. In other words, user level installation doesn't need to touch pkg registry or forget. When we support user installation case, we can add addtional check based on some condition, such as the location software is installed.

@KevinLiAWS KevinLiAWS merged commit 9afc0b9 into main Nov 8, 2023
13 checks passed
@KevinLiAWS KevinLiAWS deleted the fix-unregistry branch November 8, 2023 00:23
This was referenced Dec 4, 2023
KevinLiAWS pushed a commit that referenced this pull request Dec 7, 2023
🤖 I have created a release *beep* *boop*
---


## [1.0.1](v1.0.0...v1.0.1)
(2023-12-07)


### Bug Fixes

* Clean up all previous finch version installation registries in
postinstall and uninstall
([#688](#688))
([9afc0b9](9afc0b9))
* Fix to be able to run finch build with --ssh option
([#696](#696))
([4d1e6cf](4d1e6cf))
* Fix to delete ~/.finch when uninstalling finch
([#703](#703))
([8d7389f](8d7389f))
* remove virtual machine image when running make clean
([98c8ee4](98c8ee4))
* resolve shellcheck warnings
([#684](#684))
([d9f695a](d9f695a))
* Use LimaUser method instead of host user name
([#712](#712))
([7c02e08](7c02e08))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
KevinLiAWS added a commit that referenced this pull request Dec 11, 2023
🤖 I have created a release *beep* *boop*
---


## [1.0.1](v1.0.0...v1.0.1)
(2023-12-11)


### Bug Fixes

* Change the default behavoir for deleting .finch folder to false when
uninstall ([#732](#732))
([e818743](e818743))
* Clean up all previous finch version installation registries in
postinstall and uninstall
([#688](#688))
([9afc0b9](9afc0b9))
* Fix to be able to run finch build with --ssh option
([#696](#696))
([4d1e6cf](4d1e6cf))
* Fix to delete ~/.finch when uninstalling finch
([#703](#703))
([8d7389f](8d7389f))
* remove virtual machine image when running make clean
([98c8ee4](98c8ee4))
* resolve shellcheck warnings
([#684](#684))
([d9f695a](d9f695a))
* Use LimaUser method instead of host user name
([#712](#712))
([7c02e08](7c02e08))

### Reverts

* "fix: resolve shellcheck warnings"
([#725](#725))
([8ea255a](8ea255a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: Kevin Li <[email protected]>
Co-authored-by: Kevin Li <[email protected]>
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.

2 participants