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

DSL: allow all Casks to use uninstall stanzas #4865

Merged
merged 7 commits into from
Jun 28, 2014

Conversation

rolandwalker
Copy link
Contributor

Previously, uninstall was silently ignored unless there was an install or
caskroom_only stanza present.

This PR works by promoting uninstall to a full, independent artifact. uninstall was
previously integrated into Cask::Artifact::Pkg.

This PR also contains considerable refactoring. Notably, install and uninstall
methods on all artifacts were changed to install_phase and uninstall_phase
to help distinguish these actions from the new uninstall artifact. (Both the install_phase
and uninstall_phase methods are invoked on every artifact type present in a
Cask, though install_phase on the uninstall artifact is a no-op).

install-based Casks (ie pkg-based Casks) must still use an explicit uninstall
stanza as before. Implicit cleanup of link stanza symlinks during uninstall is
unchanged. The only functional difference is that a link-type Cask can do
something like:

  uninstall :quit => 'com.bundle.id'`

@rolandwalker
Copy link
Contributor Author

References: #4688

rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Jun 13, 2014
HOLD until Homebrew#4865 is merged.

After Homebrew#4865, `caskroom_only` can be subclassed from Cask::Artifact::Base
like any other artifact.  It no longer has the special property of
making `uninstall` stanzas work.

This stanza has never been documented.  It is possible that it will
no longer be needed, after Homebrew#4865 and subsequent cleanup on Casks that
use this form.

References: Homebrew#4688
@rolandwalker rolandwalker changed the title Allow all Casks to use uninstall stanzas DSL: allow all Casks to use uninstall stanzas Jun 13, 2014
as dispatch_uninstall_directives
the set contains the uninstall directives from the DSL,
possibly more than one artifact
- separated from class `Cask::Artifact::Pkg` (the `:uninstall` DSL key)
- `:uninstall` can now be invoked for all Casks
- `uninstall_test.rb` also had to be separated from `pkg_test.rb`
and `uninstall_phase`, to improve clarity, now that we have an
independent `uninstall` artifact.
rolandwalker added a commit that referenced this pull request Jun 28, 2014
DSL: allow all Casks to use `uninstall` stanzas
@rolandwalker rolandwalker merged commit 5215201 into Homebrew:master Jun 28, 2014
@rolandwalker rolandwalker deleted the allow_all_uninstall branch June 28, 2014 14:09
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Jul 30, 2014
Missed in Homebrew#4865.  install method was renamed.
Bug exercised by Cask amazon_music.rb.
@rolandwalker rolandwalker mentioned this pull request Sep 11, 2014
9 tasks
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Nov 18, 2014
`caskroom_only` was never documented.  Its original purpose was
obsoleted in Homebrew#4865, and its use has been recently been reduced to
two Casks.

This PR
* continues the rationalization of naming by changing `caskroom_only`
  to `stage_only`. "stage" is the verb for "make a copy under the
  caskroom directory"
* documents `stage_only`
* adds tests for `stage_only`
* validates the argument to `stage_only`
* gives sensible output in `brew cask info` for `stage_only` Casks
* enforces that `stage_only` cannot coexist with any activatable
  artifacts

`caskroom_only` is still supported for backward compatibility,
but should be removed before 0.50.0.
@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.

1 participant