Merge pull request #12014 from chrisroberts/f-broken-trigger-hook

Fix application of action named triggers/hooks

lib/vagrant/action/builder.rb

@@ -173,16 +173,23 @@ module Vagrant
         # be modified
         builder = self.dup
 
+        if env[:builder_applied] != env[:action_name]
+          apply_action_name = true
+          env[:builder_applied] = env[:action_name]
+        end
+
         # Apply all dynamic modifications of the stack. This
         # will generate dynamic hooks for all actions within
         # the stack, load any triggers for action classes, and
         # apply them to the builder's stack
         builder.apply_dynamic_updates(env)
 
-        # Now that the stack is fully expanded, apply any
-        # action hooks that may be defined so they are on
-        # the outermost locations of the stack
-        builder.apply_action_name(env)
+        if apply_action_name
+          # Now that the stack is fully expanded, apply any
+          # action hooks that may be defined so they are on
+          # the outermost locations of the stack
+          builder.apply_action_name(env)
+        end
 
         # Wrap the middleware stack with the Warden to provide a consistent
         # and predictable behavior upon exceptions.
@@ -259,6 +266,7 @@ module Vagrant
       # @return [Builder]
       def apply_action_name(env)
         return self if !env[:action_name]
+
         hook = Hook.new
         machine_name = env[:machine].name if env[:machine]
 
@@ -306,21 +314,8 @@ module Vagrant
         # we can just send ourself back
         return self if hook.empty?
 
-        # These are the options to pass into hook application.
-        options = {}
-
-        # If we already ran through once and did append/prepends,
-        # then don't do it again.
-        if env[:action_hooks_already_ran]
-          options[:no_prepend_or_append] = true
-        end
-
-        # Specify that we already ran, so in the future we don't repeat
-        # the prepend/append hooks.
-        env[:action_hooks_already_ran] = true
-
         # Apply all the hooks to the new builder instance
-        hook.apply(self, options)
+        hook.apply(self)
 
         self
       end

lib/vagrant/action/runner.rb

@@ -85,7 +85,7 @@ module Vagrant
         action_name = environment[:action_name]
 
         # We place a process lock around every action that is called
-        @logger.info("Running action: #{environment[:action_name]} #{callable_id}")
+        @logger.info("Running action: #{action_name} #{callable_id}")
         Util::Busy.busy(int_callback) { callable.call(environment) }
 
         # Return the environment in case there are things in there that

lib/vagrant/machine.rb

@@ -231,15 +231,18 @@ module Vagrant
     # @param [Proc] callable
     # @param [Hash] extra_env Extra env for the action env.
     # @return [Hash] The resulting env
-    def action_raw(name, callable, extra_env=nil)
+    def action_raw(name, callable, extra_env={})
+      if !extra_env.is_a?(Hash)
+        extra_env = {}
+      end
+
       # Run the action with the action runner on the environment
-      env = {
+      env = {ui: @ui}.merge(extra_env).merge(
         raw_action_name: name,
         action_name: "machine_action_#{name}".to_sym,
         machine: self,
-        machine_action: name,
-        ui: @ui,
-      }.merge(extra_env || {})
+        machine_action: name
+      )
       @env.action_runner.run(callable, env)
     end
 

test/unit/vagrant/action/builder_test.rb

@@ -255,10 +255,10 @@ describe Vagrant::Action::Builder do
       subject.call(data)
 
       expect(data[:data]).to eq([1, 2])
-      expect(data[:action_hooks_already_ran]).to eq(true)
+      expect(data[:builder_applied]).to eq(:test_action)
     end
 
-    it "applies without prepend/append if it has already" do
+    it "applies without adding action hooks/triggers if it has already" do
       hook_proc = proc{ |h| h.append(appender_proc(2)) }
       expect(manager).to receive(:action_hooks).with(:test_action).
         and_return([hook_proc])
@@ -266,7 +266,7 @@ describe Vagrant::Action::Builder do
       data[:action_name] = :test_action
 
       subject.use appender_proc(1)
-      subject.call(data.merge(action_hooks_already_ran: true))
+      subject.call(data.merge(builder_applied: :test_action))
 
       expect(data[:data]).to eq([1])
       subject.call(data)
@@ -637,11 +637,6 @@ describe Vagrant::Action::Builder do
     before { allow(triggers).to receive(:find).and_return([]) }
     after { @subject = nil }
 
-    it "should mark action hooks applied within env" do
-      subject.apply_action_name(env)
-      expect(env[:action_hooks_already_ran]).to be_truthy
-    end
-
     context "when a plugin has added an action hook" do
       let(:plugin) do
         @plugin ||= Class.new(Vagrant.plugin("2")) do
@@ -663,13 +658,6 @@ describe Vagrant::Action::Builder do
         subject.apply_action_name(env)
         expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Call)
       end
-
-      it "should only add new action to the call stack once" do
-        subject.apply_action_name(env)
-        subject.apply_action_name(env)
-        expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Call)
-        expect(subject.stack[1].first).not_to eq(Vagrant::Action::Builtin::Call)
-      end
     end
 
     context "when trigger has been defined for raw action" do