From 271d427c57d9abd373c8c9f444b18f492f55e6d1 Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Tue, 27 Aug 2019 11:15:44 -0700
Subject: [PATCH] Fix bug in :each provisioner sorting

Ensure each provisioners are properly inserted into the final
provisioner array
---
 .../action/builtin/mixin_provisioners.rb      | 28 +++++++------------
 .../action/builtin/mixin_provisioners_test.rb |  3 --
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb
index 98d510e61..4f1c318cb 100644
--- a/lib/vagrant/action/builtin/mixin_provisioners.rb
+++ b/lib/vagrant/action/builtin/mixin_provisioners.rb
@@ -45,8 +45,6 @@ module Vagrant
 
         # Sorts provisioners based on order specified with before/after options
         #
-        # TODO: make sure all defined provisioners work here (i.e. even thoughs defined in separate but loaded Vagrantfile)
-        #
         # @return [Array<Provisioner, Hash>]
         def sort_provisioner_instances(pvs)
           final_provs = []
@@ -70,8 +68,6 @@ module Vagrant
           # extract all provisioners
           all_provs = pvs.map { |p,o| [p,o] if o[:before] == :all || o[:after] == :all }.reject(&:nil?)
 
-          # TODO: Log here, that provisioner order is being changed
-
           # insert provisioners in order
           final_provs = root_provs
           dep_provs.each do |p,options|
@@ -90,26 +86,22 @@ module Vagrant
           # Add :each and :all provisioners in reverse to preserve order in Vagrantfile
 
           # add each to final array
-          # TODO: This doesn't work if before each provisioners are defined before after provisioners
-          #
-          # Maybe do before and after individually instead??
-          tmp_final_provs = final_provs.dup
-          index_offset = 0
+          tmp_final_provs = []
           final_provs.each_with_index.map do |(prv,o), i|
+            tmp_before = []
+            tmp_after = []
+
             each_provs.reverse_each do |p, options|
-              idx = 0
               if options[:before]
-                idx = (i+index_offset+1)-1 unless i == 0
-                puts "b: #{idx}"
-                index_offset += 1
-                tmp_final_provs.insert(idx, [p,options])
+                tmp_before << [p,options]
               elsif options[:after]
-                idx = (i+index_offset)+1
-                index_offset += 1
-                puts "a: #{idx}"
-                tmp_final_provs.insert(idx, [p,options])
+                tmp_after << [p,options]
               end
             end
+
+            tmp_final_provs += tmp_before unless tmp_before.empty?
+            tmp_final_provs += [[prv,o]]
+            tmp_final_provs += tmp_after unless tmp_after.empty?
           end
           final_provs = tmp_final_provs
 
diff --git a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb
index 9b4934060..708247787 100644
--- a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb
+++ b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb
@@ -188,9 +188,6 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
 
       it "puts the each shortcut provisioners in place" do
         result = subject.provisioner_instances(env)
-        result.each do |p,o|
-          puts o[:name]
-        end
 
         expect(result[0].last[:name]).to eq("before-test")
         expect(result[1].last[:name]).to eq("root-test")