Keep order of env provided

2 files changed, 24 insertions, 11 deletions

diff --git a/lib/chef/provider/env.rb b/lib/chef/provider/env.rb
index 600cac1e6b..970131c407 100644
--- a/lib/chef/provider/env.rb
+++ b/lib/chef/provider/env.rb
@@ -61,9 +61,12 @@ class Chef
       def requires_modify_or_create?
         if @new_resource.delim
           #e.g. check for existing value within PATH
-          not new_values.all? do |val|
-            current_values.include? val
+          new_values.inject(0) do |index, val|
+            next_index = current_values.find_index val
+            return true if next_index.nil? || next_index < index
+            next_index
           end
+          false
         else
           @new_resource.value != @current_resource.value
         end
@@ -145,10 +148,7 @@ class Chef
 
       def modify_env
         if @new_resource.delim
-          values = new_values.reject do |v|
-            current_values.include?(v)
-          end
-          @new_resource.value((values + [@current_resource.value]).join(@new_resource.delim))
+          @new_resource.value((new_values + current_values).uniq.join(@new_resource.delim))
         end
         create_env
       end
diff --git a/spec/unit/provider/env_spec.rb b/spec/unit/provider/env_spec.rb
index 19233dfba9..230603dcb3 100644
--- a/spec/unit/provider/env_spec.rb
+++ b/spec/unit/provider/env_spec.rb
@@ -252,7 +252,7 @@ describe Chef::Provider::Env do
     end
 
     context "when new_resource's value contains the delimiter"  do
-      it "should return false if all the current values are contained" do
+      it "should return false if all the current values are contained in specified order" do
         @new_resource.value("C:/biz;C:/baz")
         @new_resource.delim(";")
         @current_resource.value("C:/biz;C:/foo/bin;C:/baz")
@@ -265,6 +265,13 @@ describe Chef::Provider::Env do
         @current_resource.value("C:/biz;C:/foo/bin;C:/baz")
         expect(@provider.requires_modify_or_create?).to be_truthy
       end
+
+      it "should return true if values are contained in different order" do
+        @new_resource.value("C:/biz;C:/baz")
+        @new_resource.delim(";")
+        @current_resource.value("C:/baz;C:/foo/bin;C:/biz")
+        expect(@provider.requires_modify_or_create?).to be_truthy
+      end
     end
   end
 
@@ -286,12 +293,18 @@ describe Chef::Provider::Env do
       expect(passed_value).to eq(new_value)
     end
 
-    it "should only add values not already contained when a delimiter is provided" do
+    it "should only add values not already contained" do
       @new_resource.value("C:/foo;C:/bar;C:/baz")
-      @new_resource.delim(";")
-      @current_resource.value("C:/foo/bar;C:/bar;C:/baz")
+      @current_resource.value("C:/bar;C:/baz;C:/foo/bar")
+      @provider.modify_env
+      expect(@new_resource.value).to eq("C:/foo;C:/bar;C:/baz;C:/foo/bar")
+    end
+
+    it "should reorder values to keep order which asked" do
+      @new_resource.value("C:/foo;C:/bar;C:/baz")
+      @current_resource.value("C:/foo/bar;C:/baz;C:/bar")
       @provider.modify_env
-      expect(@new_resource.value).to eq("C:/foo;C:/foo/bar;C:/bar;C:/baz")
+      expect(@new_resource.value).to eq("C:/foo;C:/bar;C:/baz;C:/foo/bar")
     end
   end
 end