From 7331623a39fd9084aee4ced8c09791f58ef47046 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 2 Nov 2020 10:30:56 -0800 Subject: [PATCH 1/3] Only allow the ui to be overridden when running action --- lib/vagrant/machine.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index c92ceb52f..921774fd1 100644 --- a/lib/vagrant/machine.rb +++ b/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 From cba5bca7de58070e98800fd0a3e91a9140bc37cc Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 2 Nov 2020 10:31:28 -0800 Subject: [PATCH 2/3] Use variable when sending info to logger --- lib/vagrant/action/runner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vagrant/action/runner.rb b/lib/vagrant/action/runner.rb index dd36588e6..cdc3f30b4 100644 --- a/lib/vagrant/action/runner.rb +++ b/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 From db24f26daa80a46516f86eec0642f1b608d3c13a Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 2 Nov 2020 10:31:51 -0800 Subject: [PATCH 3/3] Track application of action name hooks / triggers When expanding stack track the origin action name and only apply it once the stack has completed its expansion. The local env data is marked with origin action to prevent it from being applied in nested builders as they are expanded. The value of the stored action name is checked and invalidated if another action is applied to the same env in the future so hooks / triggers for that action are applied as expected. --- lib/vagrant/action/builder.rb | 31 ++++++++++-------------- test/unit/vagrant/action/builder_test.rb | 18 +++----------- 2 files changed, 16 insertions(+), 33 deletions(-) diff --git a/lib/vagrant/action/builder.rb b/lib/vagrant/action/builder.rb index 7e25d185b..76c21eeed 100644 --- a/lib/vagrant/action/builder.rb +++ b/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 diff --git a/test/unit/vagrant/action/builder_test.rb b/test/unit/vagrant/action/builder_test.rb index efb419457..e3f5d75d3 100644 --- a/test/unit/vagrant/action/builder_test.rb +++ b/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