Merge pull request #383 from mpapis/feature/improved_command_logging_for_parallel

Improve parallel commands logging

lib/capistrano/command.rb

@@ -17,7 +17,7 @@ module Capistrano
       include Enumerable
 
       class Branch
-        attr_accessor :command, :callback
+        attr_accessor :command, :callback, :condition
         attr_reader :options
 
         def initialize(command, options, callback)
@@ -43,14 +43,17 @@ module Capistrano
           true
         end
 
-        def to_s
+        def to_s(parallel=false)
+          if parallel && @condition
+            "#{condition.inspect} :: #{command.inspect}"
+          else
             command.inspect
           end
         end
+      end
 
       class ConditionBranch < Branch
         attr_accessor :configuration
-        attr_accessor :condition
 
         class Evaluator
           attr_reader :configuration, :condition, :server
@@ -89,9 +92,12 @@ module Capistrano
         def match(server)
           Evaluator.new(configuration, condition, server).result
         end
+      end
 
-        def to_s
+      class ElseBranch < Branch
-          "#{condition.inspect} :: #{command.inspect}"
+        def initialize(command, options, callback)
+          @condition = "else"
+          super(command, options, callback)
         end
       end
 
@@ -106,7 +112,7 @@ module Capistrano
       end
 
       def else(command, &block)
-        @fallback = Branch.new(command, {}, block)
+        @fallback = ElseBranch.new(command, {}, block)
       end
 
       def branches_for(server)

lib/capistrano/configuration/actions/invocation.rb

@@ -159,12 +159,26 @@ module Capistrano
         # use, but should instead be called indirectly, via #run or #parallel,
         # or #invoke_command.
         def run_tree(tree, options={}) #:nodoc:
-          if tree.branches.empty? && tree.fallback
+          options = add_default_command_options(options)
-            logger.debug "executing #{tree.fallback}" unless options[:silent]
+
-          elsif tree.branches.any?
+          if tree.branches.any? || tree.fallback
+            _, servers = filter_servers(options)
+            branches = servers.map{|server| tree.branches_for(server)}.compact
+            case branches.size
+            when 0
+              branches = tree.branches.dup + [tree.fallback]
+              case branches.size
+              when 1
+                logger.debug "no servers for #{branches.first}"
+              else
+                logger.debug "no servers for commands"
+                branches.each{ |branch| logger.trace "-> #{branch.to_s(true)}" }
+              end
+            when 1
+              logger.debug "executing #{branches.first}" unless options[:silent]
+            else
               logger.debug "executing multiple commands in parallel"
-            tree.each do |branch|
+              branches.each{ |branch| logger.trace "-> #{branch.to_s(true)}" }
-              logger.trace "-> #{branch}"
             end
           else
             raise ArgumentError, "attempt to execute without specifying a command"
@@ -172,8 +186,6 @@ module Capistrano
 
           return if dry_run || (debug && continue_execution(tree) == false)
 
-          options = add_default_command_options(options)
-
           tree.each do |branch|
             if branch.command.include?(sudo)
               branch.callback = sudo_behavior_callback(branch.callback)

lib/capistrano/configuration/connections.rb

@@ -147,37 +147,43 @@ module Capistrano
         end
       end
 
-      # Determines the set of servers within the current task's scope and
+      # Determines the set of servers within the current task's scope
-      # establishes connections to them, and then yields that list of
+      def filter_servers(options={})
-      # servers.
-      def execute_on_servers(options={})
-        raise ArgumentError, "expected a block" unless block_given?
-
         if task = current_task
           servers = find_servers_for_task(task, options)
 
           if servers.empty?
             if ENV['HOSTFILTER'] || task.options.merge(options)[:on_no_matching_servers] == :continue
               logger.info "skipping `#{task.fully_qualified_name}' because no servers matched"
-              return
             else
+              unless dry_run
                 raise Capistrano::NoMatchingServersError, "`#{task.fully_qualified_name}' is only run for servers matching #{task.options.inspect}, but no servers matched"
               end
             end
+          end
 
           if task.continue_on_error?
             servers.delete_if { |s| has_failed?(s) }
-            return if servers.empty?
           end
         else
           servers = find_servers(options)
-          if servers.empty?
+          if servers.empty? && !dry_run
             raise Capistrano::NoMatchingServersError, "no servers found to match #{options.inspect}" if options[:on_no_matching_servers] != :continue
-            return
           end
         end
 
         servers = [servers.first] if options[:once]
+        [task, servers.compact]
+      end
+
+      # Determines the set of servers within the current task's scope and
+      # establishes connections to them, and then yields that list of
+      # servers.
+      def execute_on_servers(options={})
+        raise ArgumentError, "expected a block" unless block_given?
+
+        task, servers = filter_servers(options)
+        return if servers.empty?
         logger.trace "servers: #{servers.map { |s| s.host }.inspect}"
 
         max_hosts = (options[:max_hosts] || (task && task.max_hosts) || servers.size).to_i

test/configuration/actions/invocation_test.rb

@@ -27,6 +27,10 @@ class ConfigurationActionsInvocationTest < Test::Unit::TestCase
       @options.fetch(*args)
     end
 
+    def filter_servers(options = {})
+      [nil, @servers]
+    end
+
     def execute_on_servers(options = {})
       yield @servers
     end

test/configuration/connections_test.rb

@@ -5,6 +5,7 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
   class MockConfig
     attr_reader :original_initialize_called
     attr_reader :values
+    attr_reader :dry_run
     attr_accessor :current_task
 
     def initialize