DSL: add depends_on :x11, permit multiple depends_on stanzas

doc/CASK_LANGUAGE_REFERENCE.md

@@ -232,7 +232,6 @@ The following methods may be called to generate standard warning messages:
 | `reboot`                          | users should reboot to complete installation
 | `assistive_devices`               | users should grant the application access to assistive devices
 | `files_in_usr_local`              | the Cask installs files to `/usr/local`, which may confuse Homebrew
-| `x11_required`                    | the Cask requires X11 to run
 
 Example:
 
@@ -595,6 +594,8 @@ depends_on :macos => '>= :mavericks'
 depends_on :macos => '>= 10.9'
 ```
 
+A comparison expression cannot be combined with any other form of `depends_on :macos`.
+
 ### Depends_on :arch
 
 The value for `depends_on :arch` may be a symbol or an array of symbols,
@@ -622,25 +623,22 @@ depends_on :arch => :intel
 depends_on :arch => [:i386, :x86_64]   # same meaning as above
 ```
 
-Since PowerPC hardware is no longer common, the expression most
-frequently needed will be:
+Since PowerPC hardware is no longer common, the expression most frequently
+needed will be:
 
 ```ruby
 depends_on :arch => :x86_64
 ```
 
 ### All Depends_on Keys
 
-Several other keys are accepted by `depends_on`, in anticipation of future
-functionality:
-
 | key        | description |
 | ---------- | ----------- |
 | `:formula` | a Homebrew Formula
 | `:cask`    | *stub - not yet functional*
 | `:macos`   | a symbol, string, array, or comparison expression defining OS X release requirements.
 | `:arch`    | a symbol or array defining hardware requirements.
-| `:x11`     | *stub - not yet functional*
+| `:x11`     | a Boolean indicating a dependency on X11.
 | `:java`    | *stub - not yet functional*
 
 

doc/cask_language_deltas.md

@@ -54,8 +54,6 @@ features which are available for the current Cask.
  * [`artifact`](CASK_LANGUAGE_REFERENCE.md#at-least-one-artifact-stanza-is-also-required)
  * [`depends_on :cask`](CASK_LANGUAGE_REFERENCE.md#depends_on-stanza-details)
    * *stub* - not yet functional
- * [`depends_on :x11`](CASK_LANGUAGE_REFERENCE.md#depends_on-stanza-details)
-   * *stub* - not yet functional
  * [`depends_on :java`](CASK_LANGUAGE_REFERENCE.md#depends_on-stanza-details)
    * *stub* - not yet functional
  * [`conflicts_with`](CASK_LANGUAGE_REFERENCE.md#conflicts_with-stanza-details)
@@ -79,6 +77,7 @@ features which are available for the current Cask.
 | ------------------------------------- |----------------
 | `after_install`                       | [`postflight`](CASK_LANGUAGE_REFERENCE.md#optional-stanzas)
 | `after_uninstall`                     | [`uninstall_postflight`](CASK_LANGUAGE_REFERENCE.md#optional-stanzas)
+| `arch_only` (within `caveats`)        | [`depends_on :arch`](CASK_LANGUAGE_REFERENCE.md#depends_on-arch)
 | `before_install`                      | [`preflight`](CASK_LANGUAGE_REFERENCE.md#optional-stanzas)
 | `before_uninstall`                    | [`uninstall_preflight`](CASK_LANGUAGE_REFERENCE.md#optional-stanzas)
 | `container_type`                      | [`container :type`](CASK_LANGUAGE_REFERENCE.md#optional-stanzas)
@@ -87,12 +86,12 @@ features which are available for the current Cask.
 | `install`                             | [`pkg`](CASK_LANGUAGE_REFERENCE.md#pkg-stanza-details)
 | `link`                                | [`app`](CASK_LANGUAGE_REFERENCE.md#app-stanza-details) (or sometimes `suite` or `artifact`)
 | `manual_installer` (within `caveats`) | [`installer :manual`](CASK_LANGUAGE_REFERENCE.md#installer-manual)
-| `os_version_only` (within `caveats`)  | [`depends_on :macos`](CASK_LANGUAGE_REFERENCE.md#depends_on-macos)
 | `nested_container`                    | [`container :nested =>`](CASK_LANGUAGE_REFERENCE.md#optional-stanzas)
+| `os_version_only` (within `caveats`)  | [`depends_on :macos`](CASK_LANGUAGE_REFERENCE.md#depends_on-macos)
 | `title` (in interpolations)           | [`token`](CASK_LANGUAGE_REFERENCE.md#caveats-as-a-string)
 | `uninstall :files`                    | [`uninstall :delete`](CASK_LANGUAGE_REFERENCE.md#uninstall-key-delete)
 | `version 'latest'`                    | [`version :latest`](CASK_LANGUAGE_REFERENCE.md#required-stanzas)
-| `arch_only` (within `caveats`)        | [`depends_on :arch`](CASK_LANGUAGE_REFERENCE.md#depends_on-stanza-details)
+| `x11_required` (within `caveats`)     | [`depends_on :x11`](CASK_LANGUAGE_REFERENCE.md#all-depends_on-keys)
 
 
 ## All Supported Stanzas (1.0)
@@ -149,7 +148,6 @@ For use in *eg* interpolation:
  * [`logout`](CASK_LANGUAGE_REFERENCE.md#caveats-mini-dsl)
  * [`path_environment_variable(path)`](CASK_LANGUAGE_REFERENCE.md#caveats-mini-dsl)
  * [`reboot`](CASK_LANGUAGE_REFERENCE.md#caveats-mini-dsl)
- * [`x11_required`](CASK_LANGUAGE_REFERENCE.md#caveats-mini-dsl)
  * [`zsh_path_helper(path)`](CASK_LANGUAGE_REFERENCE.md#caveats-mini-dsl)
 
 

lib/cask/dsl.rb

@@ -168,13 +168,10 @@ def license(arg=nil)
     end
 
     def depends_on(*args)
-      if @depends_on and !args.empty?
-        # todo: remove this constraint, and instead merge multiple depends_on stanzas
-        raise CaskInvalidError.new(self.token, "'depends_on' stanza may only appear once")
-      end
-      @depends_on ||= begin
-        Cask::DSL::DependsOn.new(*args) unless args.empty?
-      rescue StandardError => e
+      @depends_on ||= Cask::DSL::DependsOn.new()
+      begin
+        @depends_on.load(*args) unless args.empty?
+      rescue RuntimeError => e
         raise CaskInvalidError.new(self.token, e)
       end
       @depends_on

lib/cask/dsl/depends_on.rb

@@ -11,16 +11,16 @@ class Cask::DSL::DependsOn
                         :java,
                        ]
 
-  VALID_ARCHES = Set.new [
-                          # category
-                          :intel,
-                          :ppc,
-                          # specific
-                          :i386,
-                          :x86_64,
-                          :ppc_7400,
-                          :ppc_64,
-                         ]
+  VALID_ARCHES = [
+                  # category
+                  :intel,
+                  :ppc,
+                  # specific
+                  :i386,
+                  :x86_64,
+                  :ppc_7400,
+                  :ppc_64,
+                 ]
 
   # Intentionally undocumented: catch variant spellings.
   ARCH_SYNONYMS = {
@@ -39,16 +39,18 @@ class Cask::DSL::DependsOn
                    :ppc64      => :ppc_64,
                   }
 
-  attr_accessor :formula, :cask, :x11, :java
+  attr_accessor :java
   attr_accessor :pairs
 
-  def initialize(pairs={})
-    @pairs = pairs
+  def initialize()
+    @pairs ||= {}
+  end
+
+  def load(pairs={})
     pairs.each do |key, value|
       raise "invalid depends_on key: '#{key.inspect}'" unless VALID_KEYS.include?(key)
       writer_method = "#{key}=".to_sym
-      value = Array(value) if [:formula, :cask].include?(key)
-      send(writer_method, value)
+      @pairs[key] = send(writer_method, value)
     end
   end
 
@@ -78,39 +80,67 @@ def self.coerce_os_release(arg)
     end
   end
 
+  def formula
+    @formula
+  end
+
+  def formula=(*arg)
+    @formula ||= []
+    @formula.concat(Array(*arg))
+  end
+
+  def cask
+    @cask
+  end
+
+  def cask=(*arg)
+    @cask ||= []
+    @cask.concat(Array(*arg))
+  end
+
   def macos
     @macos
   end
 
-  def macos=(arg)
-    @macos = if not arg.kind_of?(Array) and
-      arg =~ %r{^\s*(<|>|[=<>]=)\s*(\S+)\s*$}
+  def macos=(*arg)
+    @macos ||= []
+    macos = if arg.count == 1 and
+               arg.first =~ %r{^\s*(<|>|[=<>]=)\s*(\S+)\s*$}
+      raise "'depends_on :macos' comparison expressions cannot be combined" unless @macos.empty?
       operator = $1.to_sym
       release = self.class.coerce_os_release($2)
-      [ operator, release ]
+      [[ operator, release ]]
     else
-      Array(arg).map do |elt|
+      raise "'depends_on :macos' comparison expressions cannot be combined" if @macos.first.is_a?(Symbol)
+      Array(*arg).map do |elt|
         self.class.coerce_os_release(elt)
       end.sort
     end
-    @pairs[:macos] = @macos
+    @macos.concat(macos)
   end
 
   def arch
     @arch
   end
 
-  def arch=(arg)
-    @arch = Array(arg).map do |elt|
+  def arch=(*arg)
+    @arch ||= []
+    arches = Array(*arg).map do |elt|
       elt = elt.to_s.downcase.sub(%r{^:},'').gsub('-','_').to_sym
       ARCH_SYNONYMS.key?(elt) ? ARCH_SYNONYMS[elt] : elt
     end
-    @arch.each do |elt|
-      unless VALID_ARCHES.include?(elt)
-        raise "invalid 'depends_on :arch' value: #{arg.inspect}"
-      end
-    end
-    @pairs[:arch] = @arch
+    invalid_arches = arches - VALID_ARCHES
+    raise "invalid 'depends_on :arch' values: #{invalid_arches.inspect}" unless invalid_arches.empty?
+    @arch.concat(arches)
+  end
+
+  def x11
+    @x11
+  end
+
+  def x11=(arg)
+    raise "invalid depends_on :x11 value: #{arg.inspect}" unless [true, false].include?(arg)
+    @x11 = arg
   end
 
   def to_yaml

lib/cask/exceptions.rb

@@ -67,6 +67,25 @@ def to_s;
   end
 end
 
+class CaskX11DependencyError < CaskError
+  attr_reader :token
+  def initialize(token)
+    @token = token
+  end
+
+  def to_s
+    <<-EOS.undent
+      #{token} requires XQuartz/X11, which can be installed via homebrew-cask by
+
+        brew cask install xquartz
+
+      or manually, by downloading the package from
+
+        http://xquartz.macosforge.org
+    EOS
+  end
+end
+
 class CaskUnspecifiedError < CaskError
   def to_s
     "This command requires a Cask token"

lib/cask/installer.rb

@@ -105,64 +105,65 @@ def install_artifacts
   #      dependencies should also apply for "brew cask stage"
   #      override dependencies with --force or perhaps --force-deps
   def satisfy_dependencies
-    macos_dependencies
-    arch_dependencies
-    formula_dependencies
+    if @cask.depends_on
+      ohai 'Satisfying dependencies'
+      macos_dependencies
+      arch_dependencies
+      x11_dependencies
+      formula_dependencies
+      puts 'complete'
+    end
   end
 
   def macos_dependencies
-    if @cask.depends_on and @cask.depends_on.macos
-      if @cask.depends_on.macos.first.is_a?(Symbol)
-        operator, release = @cask.depends_on.macos
-        unless MacOS.version.send(operator, release)
-          raise CaskError.new "Cask #{@cask} depends on OS X release #{operator} #{release}, but you are running release #{MacOS.version}."
-        end
-      elsif @cask.depends_on.macos.length > 1
-        unless @cask.depends_on.macos.include?(Gem::Version.new(MacOS.version.to_s))
-          raise CaskError.new "Cask #{@cask} depends on OS X release being one of: #{@cask.depends_on.macos(&:to_s).inspect}, but you are running release #{MacOS.version}."
-        end
-      else
-        unless MacOS.version == @cask.depends_on.macos.first
-          raise CaskError.new "Cask #{@cask} depends on OS X release #{@cask.depends_on.macos.first}, but you are running release #{MacOS.version}."
-        end
+    return unless @cask.depends_on.macos
+    if @cask.depends_on.macos.first.is_a?(Array)
+      operator, release = @cask.depends_on.macos.first
+      unless MacOS.version.send(operator, release)
+        raise CaskError.new "Cask #{@cask} depends on OS X release #{operator} #{release}, but you are running release #{MacOS.version}."
+      end
+    elsif @cask.depends_on.macos.length > 1
+      unless @cask.depends_on.macos.include?(Gem::Version.new(MacOS.version.to_s))
+        raise CaskError.new "Cask #{@cask} depends on OS X release being one of: #{@cask.depends_on.macos(&:to_s).inspect}, but you are running release #{MacOS.version}."
+      end
+    else
+      unless MacOS.version == @cask.depends_on.macos.first
+        raise CaskError.new "Cask #{@cask} depends on OS X release #{@cask.depends_on.macos.first}, but you are running release #{MacOS.version}."
       end
     end
   end
 
   def arch_dependencies
-    if @cask.depends_on and @cask.depends_on.arch
-      @current_arch ||= [
-                         Hardware::CPU.type,
-                         Hardware::CPU.is_32_bit? ?
-                           (Hardware::CPU.intel? ? :i386   : :ppc_7400) :
-                           (Hardware::CPU.intel? ? :x86_64 : :ppc_64)
-                        ]
-      if Array(@cask.depends_on.arch & @current_arch).count == 0
-        raise CaskError.new "Cask #{@cask} depends on hardware architecture being one of #{@cask.depends_on.arch.inspect}, but you are running #{@current_arch.inspect}"
-      end
-    end
+    return unless @cask.depends_on.arch
+    @current_arch ||= [
+                       Hardware::CPU.type,
+                       Hardware::CPU.is_32_bit? ?
+                         (Hardware::CPU.intel? ? :i386   : :ppc_7400) :
+                         (Hardware::CPU.intel? ? :x86_64 : :ppc_64)
+                      ]
+    return unless Array(@cask.depends_on.arch & @current_arch).empty?
+    raise CaskError.new "Cask #{@cask} depends on hardware architecture being one of #{@cask.depends_on.arch.inspect}, but you are running #{@current_arch.inspect}"
+  end
+
+  def x11_dependencies
+    return unless @cask.depends_on.x11
+    raise CaskX11DependencyError.new(@cask.token) unless Cask.x11_executable.exist?
   end
 
   def formula_dependencies
-    # todo The Cask::DependsOn object needs to be more friendly.
-    #      Currently @cask.depends_on.formula raises an exception
-    #      if :formula was not set.
-    if @cask.depends_on and
-       @cask.depends_on.formula and
-       not @cask.depends_on.formula.empty?
-      ohai 'Installing Formula dependencies from Homebrew'
-      @cask.depends_on.formula.each do |dep_name|
-        print "#{dep_name} ... "
-        installed = @command.run(HOMEBREW_BREW_FILE,
-                                 :args => ['list', '--versions', dep_name],
-                                 :print_stderr => false).stdout.include?(dep_name)
-        if installed
-          puts "already installed"
-        else
-          @command.run!(HOMEBREW_BREW_FILE,
-                        :args => ['install', dep_name])
-          puts "done"
-        end
+    return unless @cask.depends_on.formula and not @cask.depends_on.formula.empty?
+    ohai 'Installing Formula dependencies from Homebrew'
+    @cask.depends_on.formula.each do |dep_name|
+      print "#{dep_name} ... "
+      installed = @command.run(HOMEBREW_BREW_FILE,
+                               :args => ['list', '--versions', dep_name],
+                               :print_stderr => false).stdout.include?(dep_name)
+      if installed
+        puts "already installed"
+      else
+        @command.run!(HOMEBREW_BREW_FILE,
+                      :args => ['install', dep_name])
+        puts "done"
       end
     end
   end

lib/cask/locations.rb

@@ -143,5 +143,9 @@ def tcc_db
     def pre_mavericks_accessibility_dotfile
       @pre_mavericks_accessibility_dotfile ||= Pathname.new('/private/var/db/.AccessibilityAPIEnabled')
     end
+
+    def x11_executable
+      @x11_executable ||= Pathname.new('/usr/X11/bin/X')
+    end
   end
 end