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

Add uninstall stanza for Jawbone Updater #3431

Merged
merged 1 commit into from
Mar 8, 2014
Merged

Add uninstall stanza for Jawbone Updater #3431

merged 1 commit into from
Mar 8, 2014

Conversation

alebcay
Copy link
Member

@alebcay alebcay commented Mar 6, 2014

Uninstalls four packages from the system. I attempted to use regular expressions, but they weren't working, so I hard-coded it.

@rolandwalker
Copy link
Contributor

Hold this one. I believe that using multiple :pkgutil works on the master branch but not in release.

@alebcay
Copy link
Member Author

alebcay commented Mar 7, 2014

I test all the Casks I submit from my production setup (brew install phinze/cask/brew-cask ; brew cask install <someLocalTweakedCaskFile>), and this worked without a hiccup.

caleb@caleb:/Volumes/MacData/homebrew/homebrew-cask/developer/bin$ cask rm jawbone-updater
==> Running uninstall process for jawbone-updater; your password may be necessar
==> Removing files from pkgutil Bill-of-Materials
==> Running uninstall process for jawbone-updater; your password may be necessar
==> Removing files from pkgutil Bill-of-Materials
==> Running uninstall process for jawbone-updater; your password may be necessar
==> Removing files from pkgutil Bill-of-Materials
==> Running uninstall process for jawbone-updater; your password may be necessar
==> Removing files from pkgutil Bill-of-Materials

@rolandwalker rolandwalker self-assigned this Mar 7, 2014
@rolandwalker
Copy link
Contributor

That's good to know, and reveals a bug. What was broken until #3217 was the form

  uninstall :pkgutil => [
                          'com.Aliph.jawbone.pkg',
                          'com.Aliph.JawboneUpdater.pkg',
                          'com.Aliph.jawboneUpdater.preflight.pkg',
                          'com.Aliph.jawboneUpdater.postflight.pkg',
                         ]

The bug is: the DSL treats uninstall as an artifact type, just like link or install. Multiple artifacts are permitted by default, so multiple uninstall directives are permitted.

However, the uninstall has an implicit order-of-operations. The docs claim that the order
in which uninstall keys appear in the Cask file is ignored
. The reasons for the order are explained in comments here.

When there are multiple uninstall directives, the order-of-operations is not respected.

Since you only use the same operation :pkgutil in each uninstall, order of operations doesn't matter, and this Cask will work OK. But I need to do a quick scan for Casks that use this syntax, and fix the underlying bug.

@rolandwalker
Copy link
Contributor

To find the packages matching a regexp, you can do pkgutil --pkgs='REGEXP'. I should make a devscript wrapper for that.

I suspect that what tripped you up with this regexp was the "J" being in both upper and lower case. This form worked for me:

  uninstall :pkgutil => 'com.Aliph.[Jj]awbone(|Updater.*).pkg'

I suppose we should start anchoring those regexps with ^ and escaping the literal dots.

rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Mar 7, 2014
In Homebrew#3431, we recently discovered that this form is valid
to the DSL, but can exercise a bug at uninstall time.
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Mar 7, 2014
rolandwalker added a commit that referenced this pull request Mar 8, 2014
Add uninstall stanza for Jawbone Updater
@rolandwalker rolandwalker merged commit d3f7d50 into Homebrew:master Mar 8, 2014
@alebcay alebcay deleted the jawbone-updater branch March 8, 2014 16:20
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants