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

Roadmap: Rationalizing and Extending the Cask DSL (DSL 1.0) #4688

Closed
rolandwalker opened this issue Jun 5, 2014 · 12 comments
Closed

Roadmap: Rationalizing and Extending the Cask DSL (DSL 1.0) #4688

rolandwalker opened this issue Jun 5, 2014 · 12 comments
Assignees

Comments

@rolandwalker
Copy link
Contributor

Background

Some of you may have noticed that #3066 works, but has been sitting unmerged
for a long time. The reason I pressed "pause" on that PR was the realization
that storing Cask files as metadata (while appealingly simple) would mean that
we need to keep backwards-compatibility with the DSL essentially forever.

We could still add stanzas or keys, but it would become difficult to remove
anything, because we would need to be able to load arbitrarily old metadata-Casks.

Recently, we did make substantial changes to the DSL (including the removal
of stanzas) all of which was accomplished pretty much glitch-free. (#2931, #4264,
#4257, #4689). The process goes something like this (duplicated in Tracker below):

  • push forward-compatible code, but do not document
  • cut a release
  • wait 6 weeks
  • document
    • explain new DSL forms
    • deprecate old DSL forms
  • amend Casks
  • wait a few more weeks
  • delete backward-compatible code
  • delete backward-looking documentation

Argument

Like most successful things, the Cask DSL has grown incrementally over time
and was not always planned. This seems like a good moment to review and
rationalize the DSL (before merging #3066 or something like it).

As with #4678, this is only a draft, and will be edited in-place.

Goals

  • keep things friendly toward first-time Cask authors
  • make things even more friendly toward first-time Cask authors
  • enabling tech for brew cask upgrade
  • remove everything we aren't committed to keeping
  • batch multiple DSL changes together, so we aren't always in a wait-6-weeks/amend-Casks loop

Proposed DSL Changes

  • Add app stanza, and substitute it for the most common case where link is currently used. (DSL: forward-compatible synonyms #4845)
    • affects 1304 Casks in main repo
    • consistency: most artifacts are named after the artifact type (a noun, not a verb)
      • note also that the internal classes have always used App for this
    • additional semantics can be attached to app: there is an Info.plist to consult for version numbers, the app can be shut down during upgrades, etc.
    • keep link in the DSL for unusual cases? No, this is a bad idea because it is a verb, and will be confusing when combined with an option to install by copying
    • This is the most inconvenient change, because some of us will continue to type link out of habit. However, the inconvenience is balanced by the fact that link will continue to work (it just won't gain new features as app will). No, that compatibility is not worth the potential confusion.
  • Add suite stanza for Casks that link a directory into ~/Applications (DSL: forward-compatible synonyms #4845)
    • affects 8 Casks in main repo
    • we may later append new keys to suite detailing the location of apps inside the folder
  • Add artifact stanza for "generic artifact", to cover corner cases (DSL: add artifact stanza #6225)
  • Replace install with pkg (DSL: forward-compatible synonyms #4845)
    • affects 179 Casks in main repo
    • consistency: most artifacts are named after the artifact type (a noun, not a verb)
      • note also that the internal classes have always used Pkg for this
    • clarity: currently the semantics of install/uninstall are enforced
      such that uninstall is unavailable to non-pkg Casks. We need to
      break that linkage and make uninstall available for all Casks.
      Removing install as a keyword will help Cask authors intuit that
      install and uninstall are not connected (DSL: allow all Casks to use uninstall stanzas #4865).
  • Replace before_install/after_install with preflight/postflight (DSL: forward-compatible synonyms #4845)
    • affects 34 Casks in main repo
    • intention: to expose preflight and postflight as discrete verbs in the CLI
  • Replace before_uninstall/after_uninstall with uninstall_preflight/uninstall_postflight (DSL: forward-compatible synonyms #4845)
    • affects 5 Casks in main repo
    • consistency with preflight/postflight
  • Decide whether to remove caskroom_only stanza (DSL: remove special properties of caskroom_only stanza #4866)
    • affects 13 Casks in main repo
    • caskroom_only currently does somewhat more than the name suggests, because the class is based on Cask::Artifact::Pkg rather than Cask::Artifact::Base. Specifically, :uninstall is fully respected.
    • If caskroom_only is preserved, then Cask::Artifact::CaskroomOnly should be based on Cask::Artifact::Base
    • if uninstall can be made available to all Cask types, then we might not need caskroom_only
      • caskroom_only is also used in a funny idiom with after_install, a new stanza might be needed for that (proposed installer :script below)
    • decision: keep, per discussion in changed the new install method for baiduinput #6243, but rename to stage_only
  • Add dsl stanza to store DSL specification version number? (DSL: add dsl stanza, defining DSL specification version for a Cask #4870)
  • Add license stanza (DSL: add license stanza #4873, DSL add NCSA OSS license type #6071, update valid license values #6357)
    • affects all Casks
  • Replace depends_on_formula with extensible depends_on :formula (DSL: add depends_on stanza #4896)
    • affects 1 Cask in main repo, 11 Casks in fonts repo
    • original implementation was just a mistake
    • depends_on :formula matches Homebrew
  • Add conflicts_with stanza to complement depends_on (DSL: add conflicts_with stanza #5569)
  • Consider removing if statements in favor of a more meaningful syntax for variants
    • decided against. It's too many changes at once, can wait for DSL 2.x.
  • add copy stanza (name negotiable) to support special cases
    • I don't yet fully understand what it would do, but we can add the keyword now. As with appcast, it doesn't have to undertake a function until later.
    • decided against, will be covered by suite, see comments below.
  • add gpg stanza to support GPG signatures (DSL: add gpg stanza #4848, Revise gpg stanza order and parameters #5975)
  • replace 'latest' string with :latest symbol in version stanza (DSL: allow version :latest (symbol not string) #4849)
    • once upon a time, 'latest' was simply a convention, no different from other version strings at the technical level. Recently, we have been assigning special semantics to 'latest'. Changing it to a symbol underscores the special meaning at the language level. On the backend, we will enforce that a symbol argument for version must be one of a specific list (currently only :latest), which will catch errant spellings of the special value.
    • :latest will stringify to latest, so layout on disk will be unchanged.
    • the ability to use the string 'latest' would not be removed, though we might remove the special semantics from it over time
    • we will be attempting to reconcile string version values against version numbers from appcasts and bundle Info.plists. The symbol value :latest will also indicate "this is not a version-string: do not expect it to match appcast or Info.plist.
  • add zap, an extension of uninstall that removes more stuff (DSL: add zap stanza #4869, functionality and docs for zap stanza and verb #6155, sanity check against deleting user files #6610, protect against inner ".." in uninstall paths #6193)
    • the first implementation is extremely simple: an exact duplicate of the uninstall syntax, describing uninstall-like actions which do not run by default — only with the new zap command verb
    • I have never figured out how to best to dynamically read zap targets from the mackup project. We could simply mine mackup for a list of files as an initial seed.
  • extend appcast to include keys such as a checksum. (DSL: extend appcast stanza #4847)
    • The current single-value URL string is not sufficient to be useful without information about what content is expected at that URL
  • add installer :script (name negotiable) as a more direct way to express caskroom_only/after_install (DSL: add install_script stanza #4951, make install_script stanza more robust #6306, DSL: add installer stanza #6660)
  • add :glob option to file targets which currently support :target. (WIP DSL: add :glob key to expand fileglobs #5043)
    • did not finish in time, bumped to DSL 1.1
    • possible alternate name :expand ?
    • possible alternate interface: wrap files with expand method: app expand('*/VLC.app')
    • currently, :target is interpreted inside Cask::Artifact::Symlinked. The relevant code should be moved to dsl.rb and/or constructors called from dsl.rb, after which it would be easier to add support for :expand
  • consider changing uninstall directive :files to :remove or :delete. (DSL: add uninstall :delete and :trash #4928)
    • language reason: to add :trash, an uninstall directive which works by moving files to the OS X Trash
    • functionality reason: moving Apps to Trash can allow the App to keep running during an upgrade, and then smoothly start again with the new version after relaunch.
  • add uninstall :rmdir (add uninstall :rmdir #6192)
  • add tags stanza (DSL: add tags stanza #4953)
  • Remove limitations on Cask class names (forward-compatibility to remove naming limitations on Casks #5365)
    • eg currently Casks are unable to have leading digits
    • @phinze had some work/thoughts along these lines, the key is "decouple cask names themselves from Ruby class names". On IRC one proposed format was
      phinze> likecask OnePassword; name '1password'; ... ; end
    • @rolandwalker's current proposal reads like cask :v1 => 'google-chrome' do; end
  • change container_type to container (DSL: change container_type to extensible container #6068, bug: fix container :type => <type> #6118)
    • new syntax: container :type => :naked
    • rationale: extensible without adding a new stanza
  • merge nested_container into container (new DSL form container :nested => <inner> #6120)
    • new syntax: container :nested => <inner-container>
    • rationale
      • one less stanza
      • more mnemonic for first-time Cask author
  • promote manual_installer to a first-class artifact (DSL: add installer stanza #6660)
    • current position inside caveats is opaque
    • promotion needed to assess conflicts
    • proposed name: installer :manual
    • can merge with installer :script
  • add internet_plugin artifact per user request (add support for internet_plugin artifact #5923)
  • add postflight mini-DSL (Add DSL for after_install and similar blocks #5723)
  • add accessibility_access (DSL: add accessibility_access stanza #7854)

Tracker

This tracker ignores new functionality such as implementing gpg on the backend or adding zap stanzas to Casks. The purpose is tracking the stabilization of Cask DSL 1.0 (purely as a language) and when we can merge #3066.

References

This is a tracking issue referred to in #4678 , the roadmap for brew cask upgrade

There is a followup tracker at #7683.

@rolandwalker rolandwalker changed the title Roadmap: Rationalizing the Cask DSL Draft Roadmap: Rationalizing the Cask DSL Jun 5, 2014
@nanoxd nanoxd self-assigned this Jun 5, 2014
@vitorgalvao
Copy link
Member

We also need a stanza for after copying is implemented, to deal with special cases.

@fanquake
Copy link
Contributor

fanquake commented Jun 5, 2014

Will need to read through this tomorrow when I get a chance

@rolandwalker
Copy link
Contributor Author

@fanquake take your time, there is no hurry.

@rolandwalker
Copy link
Contributor Author

I guess I forgot to prioritize these. The important removal in my view is install. The important addition is app. The rest are under the category of "easier to make changes all at once than to parcel them out".

@vitorgalvao would that work as an option copy do ... end block? I.e. we add an extensible option keyword?

Also paging @phinze.

@vitorgalvao
Copy link
Member

I don’t think we’d need a block. I was envisioning it simply as an optional stanza, since it won’t need extra options (this is really the only part with different needs in linking and copying), and should be needed sparingly. Something like (using gridwars as the example)

class Gridwars < Cask
  url 'http://gridwars.marune.de/bin/gridwars_osx_x86.zip'
  homepage 'http://gridwars.marune.de/'
  version '9.3.2006'
  sha256 '39694d6d7f7af1a25818d10dbd811d0309562622f91f43f71d8b217bccdb03fb'
  app 'gridwars_osx_x86/gridwars.app'
  copy 'gridwars_osx_x86'
end

copy is a verb, so we might want to change that.

@nanoxd nanoxd added documentation and removed cask labels Jun 5, 2014
@rolandwalker rolandwalker changed the title Draft Roadmap: Rationalizing the Cask DSL Draft Roadmap: Rationalizing and Extending the Cask DSL Jun 7, 2014
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this issue Jun 12, 2014
This makes the following stanzas available in the DSL as
synonyms to existing stanza, with no change to existing
functionality:
- pkg for `install`
- app for `link`
- suite for `link`
- preflight for `before_install`
- postflight for `before_uninstall`
- uninstall_preflight for `before_uninstall`
- uninstall_postflight for `after_uninstall`

References Homebrew#4688

This works, but is marked WIP because we are not in a hurry,
and because I intend to add tests.
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this issue 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
This was referenced Jun 13, 2014
@vitorgalvao
Copy link
Member

@rolandwalker Just to clarify, how are you envisioning suite working? Looking through the documentation, it might be going in a slightly different direction than what I was thinking. Let’s take gridwars.rb as the example. Its zip contains a gridwars_osx_x86 directory, with gridwars.app and other assets (needed for the game to actually run) inside it. Since copying is to not be considered a stanza anymore (which I think is a good decision) here’s how I am envisioning suite:

  • app and suite are not mutually exclusive. An app can have either, neither (cases of pkg, or installer, for example), or both.
  • If only app or only suite are present, they’ll do the same: link or copy (when that’s implemented) their arguments to the applicable location.
  • If both app and suite are present, then they’ll work differently when linking or copying. When linking, only app will be considered1; when copying, only suite will be considered1.

So gridwars.rb, under this example, would become

cask :v1 => 'gridwars' do
  version '9.3.2006'
  sha256 '39694d6d7f7af1a25818d10dbd811d0309562622f91f43f71d8b217bccdb03fb'

  url 'http://gridwars.marune.de/bin/gridwars_osx_x86.zip'
  homepage 'http://gridwars.marune.de/'
  license :unknown

  app 'gridwars_osx_x86/gridwars.app'
  suite 'gridwars_osx_x86'
end

while apps like sketchup.rb (uses suite) and acorn.rb (uses app) would remain as they are, since copying or linking them should yield the same result.


1 It works since the link is still accessing the app in the directory with every other resource that might be needed.
2 It needs to be this way, since copying only the app would break the application.

@rolandwalker rolandwalker assigned rolandwalker and unassigned nanoxd Nov 18, 2014
@rolandwalker
Copy link
Contributor Author

@vitorgalvao, hmm, I think the implicit change in behavior described above would be unpredictable to both end-users and Cask authors.

However, there is a need describe the apps that are within a suite for other reasons, such as determining the runtime version number.

And I'm definitely in favor of giving control to end-users who prefer a flat Applications folder, or a hierarchical one. We just need to define a good interface for that. What about a command-line option and corresponding config-file option? I'm not saying this is a good choice of name, but

$ brew cask install --appfolder gridwars

@vitorgalvao
Copy link
Member

@rolandwalker I’m not so much thinking about control, but breakage. In the gridwars example, linking only the app bundle (the current behaviour) makes a lot of sense because you don’t care about whatever else is in that directory, and having to navigate to it doesn’t make sense (you just want to execute the game, anyway). However, having the whole directory is a necessity when copying, because without it the game won’t run.

When I asked “how are you envisioning suite working”, it was the wrong question. What I should ask is “how are you envisioning copying working?” It’s fine if you’re just thinking “grab whatever is in app or suite and copy instead of linking”, but then why have suite at all? Then it’s just there for semantics, but that could’ve been solved by keeping link around and using it in app bundles or directories as needed. As it stands, I see no compelling reason for both app and suite to exist: they’re just doing the same.

Having a command-line option won’t work, because then we’re putting the burden on the user of knowing how the cask is structured. It’s safe to assume that once we have copying implemented, users will be divided between the ones that do one or the other exclusively. It doesn’t make sense for the copying user (that likely will have that option set in one of the shell’s startup files) to have to think “oh, in this articular cask I need to give this specific flag, or else it’ll break”. Even us wouldn’t be able to remember.

@alebcay
Copy link
Member

alebcay commented Dec 11, 2014

At last, glad to see this is through.

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

No branches or pull requests

6 participants