Merge pull request #1954 from opscode/adamedx/windows_paths_leading

Fix Windows path bugs, run all config tests against Windows

8 files changed, 429 insertions, 338 deletions

diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 69e191bdf8..2de3ca3e64 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -25,7 +25,6 @@ require 'chef/log'
 require 'chef/rest'
 require 'chef/api_client'
 require 'chef/api_client/registration'
-require 'chef/platform/query_helpers'
 require 'chef/node'
 require 'chef/role'
 require 'chef/file_cache'
diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index 0cf13b79da..e8a9839d71 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -23,6 +23,7 @@ require 'chef/log'
 require 'chef/exceptions'
 require 'mixlib/config'
 require 'chef/util/selinux'
+require 'chef/util/path_helper'
 require 'pathname'
 
 class Chef
@@ -30,6 +31,8 @@ class Chef
 
     extend Mixlib::Config
 
+    PathHelper = Chef::Util::PathHelper
+
     # Evaluates the given string as config.
     #
     # +filename+ is used for context in stacktraces, but doesn't need to be the name of an actual file.
@@ -58,37 +61,13 @@ class Chef
       configuration.inspect
     end
 
-    def self.on_windows?
-      RUBY_PLATFORM =~ /mswin|mingw|windows/
-    end
-
-    BACKSLASH = '\\'.freeze
-
-    def self.platform_path_separator
-      if on_windows?
-        File::ALT_SEPARATOR || BACKSLASH
-      else
-        File::SEPARATOR
-      end
-    end
-
-    def self.path_join(*args)
-      args = args.flatten
-      args.inject do |joined_path, component|
-        unless joined_path[-1,1] == platform_path_separator
-          joined_path += platform_path_separator
-        end
-        joined_path += component
-      end
-    end
-
     def self.platform_specific_path(path)
-      if on_windows?
-        # turns /etc/chef/client.rb into C:/chef/client.rb
-        system_drive = env['SYSTEMDRIVE'] ? env['SYSTEMDRIVE'] : ""
-        path = File.join(system_drive, path.split('/')[2..-1])
-        # ensure all forward slashes are backslashes
-        path.gsub!(File::SEPARATOR, (File::ALT_SEPARATOR || '\\'))
+      path = PathHelper.cleanpath(path)
+      if Chef::Platform.windows?
+        # turns \etc\chef\client.rb and \var\chef\client.rb into C:/chef/client.rb
+        if env['SYSTEMDRIVE'] && path[0] == '\\' && path.split('\\')[2] == 'chef'
+          path = PathHelper.join(env['SYSTEMDRIVE'], path.split('\\', 3)[2])
+        end
       end
       path
     end
@@ -102,9 +81,9 @@ class Chef
 
     default(:config_dir) do
       if config_file
-        ::File.dirname(config_file)
+        PathHelper.dirname(config_file)
       else
-        path_join(user_home, ".chef#{platform_path_separator}")
+        PathHelper.join(user_home, ".chef", "")
       end
     end
 
@@ -150,7 +129,7 @@ class Chef
       # In local mode, we auto-discover the repo root by looking for a path with "cookbooks" under it.
       # This allows us to run config-free.
       path = cwd
-      until File.directory?(path_join(path, "cookbooks"))
+      until File.directory?(PathHelper.join(path, "cookbooks"))
         new_path = File.expand_path('..', path)
         if new_path == path
           Chef::Log.warn("No cookbooks directory found at or above current directory.  Assuming #{Dir.pwd}.")
@@ -164,9 +143,9 @@ class Chef
 
     def self.derive_path_from_chef_repo_path(child_path)
       if chef_repo_path.kind_of?(String)
-        path_join(chef_repo_path, child_path)
+        PathHelper.join(chef_repo_path, child_path)
       else
-        chef_repo_path.map { |path| path_join(path, child_path)}
+        chef_repo_path.map { |path| PathHelper.join(path, child_path)}
       end
     end
 
@@ -238,7 +217,7 @@ class Chef
     # this is under the user's home directory.
     default(:cache_path) do
       if local_mode
-        path_join(config_dir, 'local-mode-cache')
+        PathHelper.join(config_dir, 'local-mode-cache')
       else
         primary_cache_root = platform_specific_path("/var")
         primary_cache_path = platform_specific_path("/var/chef")
@@ -247,8 +226,7 @@ class Chef
         # Otherwise, we'll create .chef under the user's home directory and use that as
         # the cache path.
         unless path_accessible?(primary_cache_path) || path_accessible?(primary_cache_root)
-          secondary_cache_path = File.join(user_home, '.chef')
-          secondary_cache_path.gsub!(File::SEPARATOR, platform_path_separator) # Safety, mainly for Windows...
+          secondary_cache_path = PathHelper.join(user_home, '.chef')
           Chef::Log.info("Unable to access cache at #{primary_cache_path}. Switching cache to #{secondary_cache_path}")
           secondary_cache_path
         else
@@ -263,20 +241,20 @@ class Chef
     end
 
     # Where cookbook files are stored on the server (by content checksum)
-    default(:checksum_path) { path_join(cache_path, "checksums") }
+    default(:checksum_path) { PathHelper.join(cache_path, "checksums") }
 
     # Where chef's cache files should be stored
-    default(:file_cache_path) { path_join(cache_path, "cache") }
+    default(:file_cache_path) { PathHelper.join(cache_path, "cache") }
 
     # Where backups of chef-managed files should go
-    default(:file_backup_path) { path_join(cache_path, "backup") }
+    default(:file_backup_path) { PathHelper.join(cache_path, "backup") }
 
     # The chef-client (or solo) lockfile.
     #
     # If your `file_cache_path` resides on a NFS (or non-flock()-supporting
     # fs), it's recommended to set this to something like
     # '/tmp/chef-client-running.pid'
-    default(:lockfile) { path_join(file_cache_path, "chef-client-running.pid") }
+    default(:lockfile) { PathHelper.join(file_cache_path, "chef-client-running.pid") }
 
     ## Daemonization Settings ##
     # What user should Chef run as?
@@ -372,7 +350,7 @@ class Chef
     # Path to the default CA bundle files.
     default :ssl_ca_path, nil
     default(:ssl_ca_file) do
-      if on_windows? and embedded_path = embedded_dir
+      if Chef::Platform.windows? and embedded_path = embedded_dir
         cacert_path = File.join(embedded_path, "ssl/certs/cacert.pem")
         cacert_path if File.exist?(cacert_path)
       else
@@ -384,7 +362,7 @@ class Chef
     # certificates in this directory will be added to whatever CA bundle ruby
     # is using. Use this to add self-signed certs for your Chef Server or local
     # HTTP file servers.
-    default(:trusted_certs_dir) { path_join(config_dir, "trusted_certs") }
+    default(:trusted_certs_dir) { PathHelper.join(config_dir, "trusted_certs") }
 
     # Where should chef-solo download recipes from?
     default :recipe_url, nil
@@ -488,7 +466,7 @@ class Chef
     default(:syntax_check_cache_path) { cache_options[:path] }
 
     # Deprecated:
-    default(:cache_options) { { :path => path_join(file_cache_path, "checksums") } }
+    default(:cache_options) { { :path => PathHelper.join(file_cache_path, "checksums") } }
 
     # Set to false to silence Chef 11 deprecation warnings:
     default :chef11_deprecation_warnings, true
@@ -536,7 +514,7 @@ class Chef
 
     # Those lists of regular expressions define what chef considers a
     # valid user and group name
-    if on_windows?
+    if Chef::Platform.windows?
       set_defaults_for_windows
     else
       set_defaults_for_nix
diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb
index 3f80290f90..881c94b862 100644
--- a/lib/chef/mixin/shell_out.rb
+++ b/lib/chef/mixin/shell_out.rb
@@ -20,7 +20,6 @@
 require 'chef/shell_out'
 
 require 'mixlib/shellout'
-require 'chef/config'
 
 class Chef
   module Mixin
@@ -97,3 +96,6 @@ class Chef
     end
   end
 end
+
+# Break circular dep
+require 'chef/config'
diff --git a/lib/chef/platform.rb b/lib/chef/platform.rb
index 8f494731ab..841aa1b46c 100644
--- a/lib/chef/platform.rb
+++ b/lib/chef/platform.rb
@@ -16,8 +16,9 @@
 # limitations under the License.
 #
 
-require 'chef/platform/provider_mapping'
+# Order of these headers is important: query helpers is needed by many things
 require 'chef/platform/query_helpers'
+require 'chef/platform/provider_mapping'
 
 class Chef
   class Platform
diff --git a/lib/chef/platform/provider_mapping.rb b/lib/chef/platform/provider_mapping.rb
index d61298e182..7f79c38a6a 100644
--- a/lib/chef/platform/provider_mapping.rb
+++ b/lib/chef/platform/provider_mapping.rb
@@ -16,8 +16,8 @@
 # limitations under the License.
 #
 
-require 'chef/config'
 require 'chef/log'
+require 'chef/exceptions'
 require 'chef/mixin/params_validate'
 require 'chef/version_constraint/platform'
 
diff --git a/lib/chef/util/path_helper.rb b/lib/chef/util/path_helper.rb
index 534a9087ae..e9fb4e7773 100644
--- a/lib/chef/util/path_helper.rb
+++ b/lib/chef/util/path_helper.rb
@@ -16,15 +16,50 @@
 # limitations under the License.
 #
 
-require 'chef/platform'
-require 'chef/exceptions'
-
 class Chef
   class Util
     class PathHelper
       # Maximum characters in a standard Windows path (260 including drive letter and NUL)
       WIN_MAX_PATH = 259
 
+      def self.dirname(path)
+        if Chef::Platform.windows?
+          # Find the first slash, not counting trailing slashes
+          end_slash = path.size
+          while true
+            slash = path.rindex(/[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator)}]/, end_slash - 1)
+            if !slash
+              return end_slash == path.size ? '.' : path_separator
+            elsif slash == end_slash - 1
+              end_slash = slash
+            else
+              return path[0..slash-1]
+            end
+          end
+        else
+          ::File.dirname(path)
+        end
+      end
+
+      BACKSLASH = '\\'.freeze
+
+      def self.path_separator
+        if Chef::Platform.windows?
+          File::ALT_SEPARATOR || BACKSLASH
+        else
+          File::SEPARATOR
+        end
+      end
+
+      def self.join(*args)
+        args.flatten.inject do |joined_path, component|
+          # Joined path ends with /
+          joined_path = joined_path.sub(/[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator)}]+$/, '')
+          component = component.sub(/^[#{Regexp.escape(File::SEPARATOR)}#{Regexp.escape(path_separator)}]+/, '')
+          joined_path += "#{path_separator}#{component}"
+        end
+      end
+
       def self.validate_path(path)
         if Chef::Platform.windows?
           unless printable?(path)
@@ -32,7 +67,7 @@ class Chef
             Chef::Log.error(msg)
             raise Chef::Exceptions::ValidationFailed, msg
           end
-            
+
           if windows_max_length_exceeded?(path)
             Chef::Log.debug("Path '#{path}' is longer than #{WIN_MAX_PATH}, prefixing with'\\\\?\\'")
             path.insert(0, "\\\\?\\")
@@ -50,7 +85,7 @@ class Chef
             return true
           end
         end
-        
+
         false
       end
 
@@ -75,7 +110,7 @@ class Chef
         if Chef::Platform.windows?
           # Add the \\?\ API prefix on Windows unless add_prefix is false
           # Downcase on Windows where paths are still case-insensitive
-          abs_path.gsub!(::File::SEPARATOR, ::File::ALT_SEPARATOR)
+          abs_path.gsub!(::File::SEPARATOR, path_separator)
           if add_prefix && abs_path !~ /^\\\\?\\/
             abs_path.insert(0, "\\\\?\\")
           end
@@ -86,9 +121,22 @@ class Chef
         abs_path
       end
 
+      def self.cleanpath(path)
+        path = Pathname.new(path).cleanpath.to_s
+        # ensure all forward slashes are backslashes
+        if Chef::Platform.windows?
+          path = path.gsub(File::SEPARATOR, path_separator)
+        end
+        path
+      end
+
       def self.paths_eql?(path1, path2)
         canonical_path(path1) == canonical_path(path2)
       end
     end
   end
 end
+
+# Break a require loop when require chef/util/path_helper
+require 'chef/platform'
+require 'chef/exceptions'
diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb
index 2aab3297a4..af71c43b77 100644
--- a/spec/unit/config_spec.rb
+++ b/spec/unit/config_spec.rb
@@ -19,6 +19,7 @@
 
 require 'spec_helper'
 require 'chef/exceptions'
+require 'chef/util/path_helper'
 
 describe Chef::Config do
   describe "config attribute writer: chef_server_url" do
@@ -117,325 +118,304 @@ describe Chef::Config do
 
   end
 
-  describe "class method: plaform_specific_path" do
-    it "should return given path on non-windows systems" do
-      platform_mock :unix do
-        path = "/etc/chef/cookbooks"
-        Chef::Config.platform_specific_path(path).should == "/etc/chef/cookbooks"
-      end
-    end
-
-    it "should return a windows path on windows systems" do
-      platform_mock :windows do
-        path = "/etc/chef/cookbooks"
-        Chef::Config.stub(:env).and_return({ 'SYSTEMDRIVE' => 'C:' })
-        # match on a regex that looks for the base path with an optional
-        # system drive at the beginning (c:)
-        # system drive is not hardcoded b/c it can change and b/c it is not present on linux systems
-        Chef::Config.platform_specific_path(path).should == "C:\\chef\\cookbooks"
-      end
-    end
-  end
-
-  describe "default values" do
-    def primary_cache_path
-      if windows?
-        "#{Chef::Config.env['SYSTEMDRIVE']}\\chef"
-      else
-        "/var/chef"
-      end
-    end
+  [ false, true ].each do |is_windows|
 
-    def secondary_cache_path
-      if windows?
-        "#{Chef::Config[:user_home]}\\.chef"
-      else
-        "#{Chef::Config[:user_home]}/.chef"
+    context "On #{is_windows ? 'Windows' : 'Unix'}" do
+      def to_platform(*args)
+        Chef::Config.platform_specific_path(*args)
       end
-    end
 
-    before do
-      if windows?
-        Chef::Config.stub(:env).and_return({ 'SYSTEMDRIVE' => 'C:' })
-        Chef::Config[:user_home] = 'C:\Users\charlie'
-      else
-        Chef::Config[:user_home] = '/Users/charlie'
+      before :each do
+        Chef::Platform.stub(:windows?).and_return(is_windows)
       end
 
-      Chef::Config.stub(:path_accessible?).and_return(false)
-    end
-
-    describe "Chef::Config[:cache_path]" do
-      context "when /var/chef exists and is accessible" do
-        it "defaults to /var/chef" do
-          Chef::Config.stub(:path_accessible?).with(Chef::Config.platform_specific_path("/var/chef")).and_return(true)
-          Chef::Config[:cache_path].should == primary_cache_path
-        end
-      end
-
-      context "when /var/chef does not exist and /var is accessible" do
-        it "defaults to /var/chef" do
-          File.stub(:exists?).with(Chef::Config.platform_specific_path("/var/chef")).and_return(false)
-          Chef::Config.stub(:path_accessible?).with(Chef::Config.platform_specific_path("/var")).and_return(true)
-          Chef::Config[:cache_path].should == primary_cache_path
+      describe "class method: platform_specific_path" do
+        if is_windows
+          it "should return a windows path on windows systems" do
+            path = "/etc/chef/cookbooks"
+            Chef::Config.stub(:env).and_return({ 'SYSTEMDRIVE' => 'C:' })
+            # match on a regex that looks for the base path with an optional
+            # system drive at the beginning (c:)
+            # system drive is not hardcoded b/c it can change and b/c it is not present on linux systems
+            Chef::Config.platform_specific_path(path).should == "C:\\chef\\cookbooks"
+          end
+        else
+          it "should return given path on non-windows systems" do
+            path = "/etc/chef/cookbooks"
+            Chef::Config.platform_specific_path(path).should == "/etc/chef/cookbooks"
+          end
         end
       end
 
-      context "when /var/chef does not exist and /var is not accessible" do
-        it "defaults to $HOME/.chef" do
-          File.stub(:exists?).with(Chef::Config.platform_specific_path("/var/chef")).and_return(false)
-          Chef::Config.stub(:path_accessible?).with(Chef::Config.platform_specific_path("/var")).and_return(false)
-          Chef::Config[:cache_path].should == secondary_cache_path
+      describe "default values" do
+        let :primary_cache_path do
+          if is_windows
+            "#{Chef::Config.env['SYSTEMDRIVE']}\\chef"
+          else
+            "/var/chef"
+          end
         end
-      end
-
-      context "when /var/chef exists and is not accessible" do
-        it "defaults to $HOME/.chef" do
-          File.stub(:exists?).with(Chef::Config.platform_specific_path("/var/chef")).and_return(true)
-          File.stub(:readable?).with(Chef::Config.platform_specific_path("/var/chef")).and_return(true)
-          File.stub(:writable?).with(Chef::Config.platform_specific_path("/var/chef")).and_return(false)
 
-          Chef::Config[:cache_path].should == secondary_cache_path
+        let :secondary_cache_path do
+          if is_windows
+            "#{Chef::Config[:user_home]}\\.chef"
+          else
+            "#{Chef::Config[:user_home]}/.chef"
+          end
         end
-      end
 
-      context "when chef is running in local mode" do
         before do
-          Chef::Config.local_mode = true
-        end
-
-        context "and config_dir is /a/b/c" do
-          before do
-            Chef::Config.config_dir '/a/b/c'
+          if is_windows
+            Chef::Config.stub(:env).and_return({ 'SYSTEMDRIVE' => 'C:' })
+            Chef::Config[:user_home] = 'C:\Users\charlie'
+          else
+            Chef::Config[:user_home] = '/Users/charlie'
           end
 
-          it "cache_path is /a/b/c/local-mode-cache" do
-            Chef::Config.cache_path.should == '/a/b/c/local-mode-cache'
-          end
+          Chef::Config.stub(:path_accessible?).and_return(false)
         end
 
-        context "and config_dir is /a/b/c/" do
-          before do
-            Chef::Config.config_dir '/a/b/c/'
+        describe "Chef::Config[:cache_path]" do
+          context "when /var/chef exists and is accessible" do
+            it "defaults to /var/chef" do
+              Chef::Config.stub(:path_accessible?).with(to_platform("/var/chef")).and_return(true)
+              Chef::Config[:cache_path].should == primary_cache_path
+            end
           end
 
-          it "cache_path is /a/b/c/local-mode-cache" do
-            Chef::Config.cache_path.should == '/a/b/c/local-mode-cache'
+          context "when /var/chef does not exist and /var is accessible" do
+            it "defaults to /var/chef" do
+              File.stub(:exists?).with(to_platform("/var/chef")).and_return(false)
+              Chef::Config.stub(:path_accessible?).with(to_platform("/var")).and_return(true)
+              Chef::Config[:cache_path].should == primary_cache_path
+            end
           end
-        end
-      end
-    end
-
-    it "Chef::Config[:file_backup_path] defaults to /var/chef/backup" do
-      Chef::Config.stub(:cache_path).and_return(primary_cache_path)
-      backup_path = windows? ? "#{primary_cache_path}\\backup" : "#{primary_cache_path}/backup"
-      Chef::Config[:file_backup_path].should == backup_path
-    end
-
-    it "Chef::Config[:ssl_verify_mode] defaults to :verify_none" do
-      Chef::Config[:ssl_verify_mode].should == :verify_none
-    end
-
-    it "Chef::Config[:ssl_ca_path] defaults to nil" do
-      Chef::Config[:ssl_ca_path].should be_nil
-    end
-
-    describe "when on UNIX" do
-      before do
-        Chef::Config.stub(:on_windows?).and_return(false)
-      end
-
-      it "Chef::Config[:ssl_ca_file] defaults to nil" do
-        Chef::Config[:ssl_ca_file].should be_nil
-      end
-    end
 
-    it "Chef::Config[:data_bag_path] defaults to /var/chef/data_bags" do
-      Chef::Config.stub(:cache_path).and_return(primary_cache_path)
-      data_bag_path = windows? ? "#{primary_cache_path}\\data_bags" : "#{primary_cache_path}/data_bags"
-      Chef::Config[:data_bag_path].should == data_bag_path
-    end
+          context "when /var/chef does not exist and /var is not accessible" do
+            it "defaults to $HOME/.chef" do
+              File.stub(:exists?).with(to_platform("/var/chef")).and_return(false)
+              Chef::Config.stub(:path_accessible?).with(to_platform("/var")).and_return(false)
+              Chef::Config[:cache_path].should == secondary_cache_path
+            end
+          end
 
-    it "Chef::Config[:environment_path] defaults to /var/chef/environments" do
-      Chef::Config.stub(:cache_path).and_return(primary_cache_path)
-      environment_path = windows? ? "#{primary_cache_path}\\environments" : "#{primary_cache_path}/environments"
-      Chef::Config[:environment_path].should == environment_path
-    end
+          context "when /var/chef exists and is not accessible" do
+            it "defaults to $HOME/.chef" do
+              File.stub(:exists?).with(to_platform("/var/chef")).and_return(true)
+              File.stub(:readable?).with(to_platform("/var/chef")).and_return(true)
+              File.stub(:writable?).with(to_platform("/var/chef")).and_return(false)
 
-    describe "joining platform specific paths" do
+              Chef::Config[:cache_path].should == secondary_cache_path
+            end
+          end
 
-      context "on UNIX" do
-        before do
-          Chef::Config.stub(:on_windows?).and_return(false)
+          context "when chef is running in local mode" do
+            before do
+              Chef::Config.local_mode = true
+            end
+
+            context "and config_dir is /a/b/c" do
+              before do
+                Chef::Config.config_dir to_platform('/a/b/c')
+              end
+
+              it "cache_path is /a/b/c/local-mode-cache" do
+                Chef::Config.cache_path.should == to_platform('/a/b/c/local-mode-cache')
+              end
+            end
+
+            context "and config_dir is /a/b/c/" do
+              before do
+                Chef::Config.config_dir to_platform('/a/b/c/')
+              end
+
+              it "cache_path is /a/b/c/local-mode-cache" do
+                Chef::Config.cache_path.should == to_platform('/a/b/c/local-mode-cache')
+              end
+            end
+          end
         end
 
-        it "joins components when some end with separators" do
-          Chef::Config.path_join("/foo/", "bar", "baz").should == "/foo/bar/baz"
+        it "Chef::Config[:file_backup_path] defaults to /var/chef/backup" do
+          Chef::Config.stub(:cache_path).and_return(primary_cache_path)
+          backup_path = is_windows ? "#{primary_cache_path}\\backup" : "#{primary_cache_path}/backup"
+          Chef::Config[:file_backup_path].should == backup_path
         end
 
-        it "joins components that don't end in separators" do
-          Chef::Config.path_join("/foo", "bar", "baz").should == "/foo/bar/baz"
+        it "Chef::Config[:ssl_verify_mode] defaults to :verify_none" do
+          Chef::Config[:ssl_verify_mode].should == :verify_none
         end
 
-      end
-
-      context "on Windows" do
-        before do
-          Chef::Config.stub(:on_windows?).and_return(true)
+        it "Chef::Config[:ssl_ca_path] defaults to nil" do
+          Chef::Config[:ssl_ca_path].should be_nil
         end
 
-        it "joins components with the windows separator" do
-          Chef::Config.path_join('c:\\foo\\', 'bar', "baz").should == 'c:\\foo\\bar\\baz'
+        # TODO can this be removed?
+        if !is_windows
+          it "Chef::Config[:ssl_ca_file] defaults to nil" do
+            Chef::Config[:ssl_ca_file].should be_nil
+          end
         end
-      end
-    end
-
-    describe "setting the config dir" do
 
-      context "when the config file is /etc/chef/client.rb" do
-
-        before do
-          Chef::Config.stub(:on_windows?).and_return(false)
-          Chef::Config.config_file = "/etc/chef/client.rb"
+        it "Chef::Config[:data_bag_path] defaults to /var/chef/data_bags" do
+          Chef::Config.stub(:cache_path).and_return(primary_cache_path)
+          data_bag_path = is_windows ? "#{primary_cache_path}\\data_bags" : "#{primary_cache_path}/data_bags"
+          Chef::Config[:data_bag_path].should == data_bag_path
         end
 
-        it "config_dir is /etc/chef" do
-          Chef::Config.config_dir.should == "/etc/chef"
+        it "Chef::Config[:environment_path] defaults to /var/chef/environments" do
+          Chef::Config.stub(:cache_path).and_return(primary_cache_path)
+          environment_path = is_windows ? "#{primary_cache_path}\\environments" : "#{primary_cache_path}/environments"
+          Chef::Config[:environment_path].should == environment_path
         end
 
-        context "and chef is running in local mode" do
-          before do
-            Chef::Config.local_mode = true
-          end
+        describe "setting the config dir" do
 
-          it "config_dir is /etc/chef" do
-            Chef::Config.config_dir.should == "/etc/chef"
-          end
-        end
+          context "when the config file is /etc/chef/client.rb" do
 
-        context "when config_dir is set to /other/config/dir/" do
-          before do
-            Chef::Config.config_dir = "/other/config/dir/"
-          end
+            before do
+              Chef::Config.config_file = to_platform("/etc/chef/client.rb")
+            end
 
-          it "yields the explicit value" do
-            Chef::Config.config_dir.should == "/other/config/dir/"
-          end
-        end
+            it "config_dir is /etc/chef" do
+              Chef::Config.config_dir.should == to_platform("/etc/chef")
+            end
 
-      end
+            context "and chef is running in local mode" do
+              before do
+                Chef::Config.local_mode = true
+              end
 
-      context "when the user's home dir is /home/charlie" do
-        before do
-          Chef::Config.user_home = "/home/charlie"
-        end
+              it "config_dir is /etc/chef" do
+                Chef::Config.config_dir.should == to_platform("/etc/chef")
+              end
+            end
 
-        it "config_dir is /home/charlie/.chef" do
-          Chef::Config.config_dir.should == "/home/charlie/.chef/"
-        end
+            context "when config_dir is set to /other/config/dir/" do
+              before do
+                Chef::Config.config_dir = to_platform("/other/config/dir/")
+              end
 
-        context "and chef is running in local mode" do
-          before do
-            Chef::Config.local_mode = true
-          end
+              it "yields the explicit value" do
+                Chef::Config.config_dir.should == to_platform("/other/config/dir/")
+              end
+            end
 
-          it "config_dir is /home/charlie/.chef" do
-            Chef::Config.config_dir.should == "/home/charlie/.chef/"
           end
-        end
-      end
-
-    end
 
-    describe "finding the windows embedded dir" do
-      let(:default_config_location) { "c:/opscode/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.6.0/lib/chef/config.rb" }
-      let(:alternate_install_location) { "c:/my/alternate/install/place/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.6.0/lib/chef/config.rb" }
-      let(:non_omnibus_location) { "c:/my/dev/stuff/lib/ruby/gems/1.9.1/gems/chef-11.6.0/lib/chef/config.rb" }
+          context "when the user's home dir is /home/charlie/" do
+            before do
+              Chef::Config.user_home = to_platform("/home/charlie")
+            end
 
-      let(:default_ca_file) { "c:/opscode/chef/embedded/ssl/certs/cacert.pem" }
+            it "config_dir is /home/charlie/.chef/" do
+              Chef::Config.config_dir.should == Chef::Util::PathHelper.join(to_platform("/home/charlie/.chef"), '')
+            end
 
-      it "finds the embedded dir in the default location" do
-        Chef::Config.stub(:_this_file).and_return(default_config_location)
-        Chef::Config.embedded_dir.should == "c:/opscode/chef/embedded"
-      end
+            context "and chef is running in local mode" do
+              before do
+                Chef::Config.local_mode = true
+              end
 
-      it "finds the embedded dir in a custom install location" do
-        Chef::Config.stub(:_this_file).and_return(alternate_install_location)
-        Chef::Config.embedded_dir.should == "c:/my/alternate/install/place/chef/embedded"
-      end
+              it "config_dir is /home/charlie/.chef/" do
+                Chef::Config.config_dir.should == Chef::Util::PathHelper.join(to_platform("/home/charlie/.chef"), '')
+              end
+            end
+          end
 
-      it "doesn't error when not in an omnibus install" do
-        Chef::Config.stub(:_this_file).and_return(non_omnibus_location)
-        Chef::Config.embedded_dir.should be_nil
-      end
+        end
 
-      it "sets the ssl_ca_cert path if the cert file is available" do
-        Chef::Config.stub(:_this_file).and_return(default_config_location)
-        Chef::Config.stub(:on_windows?).and_return(true)
-        File.stub(:exist?).with(default_ca_file).and_return(true)
-        Chef::Config.ssl_ca_file.should == default_ca_file
+        if is_windows
+          describe "finding the windows embedded dir" do
+            let(:default_config_location) { "c:/opscode/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.6.0/lib/chef/config.rb" }
+            let(:alternate_install_location) { "c:/my/alternate/install/place/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.6.0/lib/chef/config.rb" }
+            let(:non_omnibus_location) { "c:/my/dev/stuff/lib/ruby/gems/1.9.1/gems/chef-11.6.0/lib/chef/config.rb" }
+
+            let(:default_ca_file) { "c:/opscode/chef/embedded/ssl/certs/cacert.pem" }
+
+            it "finds the embedded dir in the default location" do
+              Chef::Config.stub(:_this_file).and_return(default_config_location)
+              Chef::Config.embedded_dir.should == "c:/opscode/chef/embedded"
+            end
+
+            it "finds the embedded dir in a custom install location" do
+              Chef::Config.stub(:_this_file).and_return(alternate_install_location)
+              Chef::Config.embedded_dir.should == "c:/my/alternate/install/place/chef/embedded"
+            end
+
+            it "doesn't error when not in an omnibus install" do
+              Chef::Config.stub(:_this_file).and_return(non_omnibus_location)
+              Chef::Config.embedded_dir.should be_nil
+            end
+
+            it "sets the ssl_ca_cert path if the cert file is available" do
+              Chef::Config.stub(:_this_file).and_return(default_config_location)
+              File.stub(:exist?).with(default_ca_file).and_return(true)
+              Chef::Config.ssl_ca_file.should == default_ca_file
+            end
+          end
+        end
       end
-    end
-  end
 
-  describe "Chef::Config[:user_home]" do
-    it "should set when HOME is provided" do
-      Chef::Config.stub(:env).and_return({ 'HOME' => "/home/kitten" })
-      Chef::Config[:user_home].should == "/home/kitten"
-    end
+      describe "Chef::Config[:user_home]" do
+        it "should set when HOME is provided" do
+          expected = to_platform("/home/kitten")
+          Chef::Config.stub(:env).and_return({ 'HOME' => expected })
+          Chef::Config[:user_home].should == expected
+        end
 
-    it "should be set when only USERPROFILE is provided" do
-      Chef::Config.stub(:env).and_return({ 'USERPROFILE' => "/users/kitten" })
-      Chef::Config[:user_home].should == "/users/kitten"
-    end
+        it "should be set when only USERPROFILE is provided" do
+          expected = to_platform("/users/kitten")
+          Chef::Config.stub(:env).and_return({ 'USERPROFILE' => expected })
+          Chef::Config[:user_home].should == expected
+        end
 
-    it "falls back to the current working directory when HOME and USERPROFILE is not set" do
-      Chef::Config.stub(:env).and_return({})
-      Chef::Config[:user_home].should == Dir.pwd
-    end
-  end
+        it "falls back to the current working directory when HOME and USERPROFILE is not set" do
+          Chef::Config.stub(:env).and_return({})
+          Chef::Config[:user_home].should == Dir.pwd
+        end
+      end
 
-  describe "Chef::Config[:encrypted_data_bag_secret]" do
-    db_secret_default_path =
-      Chef::Config.platform_specific_path("/etc/chef/encrypted_data_bag_secret")
+      describe "Chef::Config[:encrypted_data_bag_secret]" do
+        let(:db_secret_default_path){ to_platform("/etc/chef/encrypted_data_bag_secret") }
 
-    let(:db_secret_default_path){ db_secret_default_path }
+        before do
+          File.stub(:exist?).with(db_secret_default_path).and_return(secret_exists)
+        end
 
-    before do
-      File.stub(:exist?).with(db_secret_default_path).and_return(secret_exists)
-    end
+        context "/etc/chef/encrypted_data_bag_secret exists" do
+          let(:secret_exists) { true }
+          it "sets the value to /etc/chef/encrypted_data_bag_secret" do
+            Chef::Config[:encrypted_data_bag_secret].should eq db_secret_default_path
+          end
+        end
 
-    context "#{db_secret_default_path} exists" do
-      let(:secret_exists) { true }
-      it "sets the value to #{db_secret_default_path}" do
-        Chef::Config[:encrypted_data_bag_secret].should eq db_secret_default_path
+        context "/etc/chef/encrypted_data_bag_secret does not exist" do
+          let(:secret_exists) { false }
+          it "sets the value to nil" do
+            Chef::Config[:encrypted_data_bag_secret].should be_nil
+          end
+        end
       end
-    end
 
-    context "#{db_secret_default_path} does not exist" do
-      let(:secret_exists) { false }
-      it "sets the value to nil" do
-        Chef::Config[:encrypted_data_bag_secret].should be_nil
+      describe "Chef::Config[:event_handlers]" do
+        it "sets a event_handlers to an empty array by default" do
+          Chef::Config[:event_handlers].should eq([])
+        end
+        it "should be able to add custom handlers" do
+          o = Object.new
+          Chef::Config[:event_handlers] << o
+          Chef::Config[:event_handlers].should be_include(o)
+        end
       end
-    end
-  end
 
-  describe "Chef::Config[:event_handlers]" do
-    it "sets a event_handlers to an empty array by default" do
-      Chef::Config[:event_handlers].should eq([])
-    end
-    it "should be able to add custom handlers" do
-      o = Object.new
-      Chef::Config[:event_handlers] << o
-      Chef::Config[:event_handlers].should be_include(o)
-    end
-  end
-
-  describe "Chef::Config[:user_valid_regex]" do
-    context "on a platform that is not Windows" do
-      it "allows one letter usernames" do
-        any_match = Chef::Config[:user_valid_regex].any? { |regex| regex.match('a') }
-        expect(any_match).to be_true
+      describe "Chef::Config[:user_valid_regex]" do
+        context "on a platform that is not Windows" do
+          it "allows one letter usernames" do
+            any_match = Chef::Config[:user_valid_regex].any? { |regex| regex.match('a') }
+            expect(any_match).to be_true
+          end
+        end
       end
     end
   end
diff --git a/spec/unit/util/path_helper_spec.rb b/spec/unit/util/path_helper_spec.rb
index 36ba19fce6..66ad323c52 100644
--- a/spec/unit/util/path_helper_spec.rb
+++ b/spec/unit/util/path_helper_spec.rb
@@ -20,101 +20,184 @@ require 'chef/util/path_helper'
 require 'spec_helper'
 
 describe Chef::Util::PathHelper do
-  let(:path_helper) { Chef::Util::PathHelper }
+  PathHelper = Chef::Util::PathHelper
+
+  [ false, true ].each do |is_windows|
+    context "on #{is_windows ? "windows" : "unix"}" do
+      before(:each) do
+        Chef::Platform.stub(:windows?).and_return(is_windows)
+      end
+
+      describe "join" do
+        it "joins components when some end with separators" do
+          expected = PathHelper.cleanpath("/foo/bar/baz")
+          expected = "C:#{expected}" if is_windows
+          PathHelper.join(is_windows ? 'C:\\foo\\' : "/foo/", "bar", "baz").should == expected
+        end
+
+        it "joins components when some end and start with separators" do
+          expected = PathHelper.cleanpath("/foo/bar/baz")
+          expected = "C:#{expected}" if is_windows
+          PathHelper.join(is_windows ? 'C:\\foo\\' : "/foo/", "bar/", "/baz").should == expected
+        end
+
+        it "joins components that don't end in separators" do
+          expected = PathHelper.cleanpath("/foo/bar/baz")
+          expected = "C:#{expected}" if is_windows
+          PathHelper.join(is_windows ? 'C:\\foo' : "/foo", "bar", "baz").should == expected
+        end
+
+        it "joins starting with '' resolve to absolute paths" do
+          PathHelper.join('', 'a', 'b').should == "#{PathHelper.path_separator}a#{PathHelper.path_separator}b"
+        end
+
+        it "joins ending with '' add a / to the end" do
+          PathHelper.join('a', 'b', '').should == "a#{PathHelper.path_separator}b#{PathHelper.path_separator}"
+        end
+
+        if is_windows
+          it "joins components on Windows when some end with unix separators" do
+            PathHelper.join('C:\\foo/', "bar", "baz").should == 'C:\\foo\\bar\\baz'
+          end
+        end
+      end
+
+      if is_windows
+        it "path_separator is \\" do
+          PathHelper.path_separator.should == '\\'
+        end
+      else
+        it "path_separator is /" do
+          PathHelper.path_separator.should == '/'
+        end
+      end
+
+      if is_windows
+        it "cleanpath changes slashes into backslashes and leaves backslashes alone" do
+          PathHelper.cleanpath('/a/b\\c/d/').should == '\\a\\b\\c\\d'
+        end
+        it "cleanpath does not remove leading double backslash" do
+          PathHelper.cleanpath('\\\\a/b\\c/d/').should == '\\\\a\\b\\c\\d'
+        end
+      else
+        it "cleanpath removes extra slashes alone" do
+          PathHelper.cleanpath('/a///b/c/d/').should == '/a/b/c/d'
+        end
+      end
+
+      describe "dirname" do
+        it "dirname('abc') is '.'" do
+          PathHelper.dirname('abc').should == '.'
+        end
+        it "dirname('/') is '/'" do
+          PathHelper.dirname(PathHelper.path_separator).should == PathHelper.path_separator
+        end
+        it "dirname('a/b/c') is 'a/b'" do
+          PathHelper.dirname(PathHelper.join('a', 'b', 'c')).should == PathHelper.join('a', 'b')
+        end
+        it "dirname('a/b/c/') is 'a/b'" do
+          PathHelper.dirname(PathHelper.join('a', 'b', 'c', '')).should == PathHelper.join('a', 'b')
+        end
+        it "dirname('/a/b/c') is '/a/b'" do
+          PathHelper.dirname(PathHelper.join('', 'a', 'b', 'c')).should == PathHelper.join('', 'a', 'b')
+        end
+      end
+    end
+  end
 
   describe "validate_path" do
     context "on windows" do
       before(:each) do
         # pass by default
         Chef::Platform.stub(:windows?).and_return(true)
-        path_helper.stub(:printable?).and_return(true)
-        path_helper.stub(:windows_max_length_exceeded?).and_return(false)
+        PathHelper.stub(:printable?).and_return(true)
+        PathHelper.stub(:windows_max_length_exceeded?).and_return(false)
       end
 
       it "returns the path if the path passes the tests" do
-        expect(path_helper.validate_path("C:\\ThisIsRigged")).to eql("C:\\ThisIsRigged")
+        expect(PathHelper.validate_path("C:\\ThisIsRigged")).to eql("C:\\ThisIsRigged")
       end
 
       it "does not raise an error if everything looks great" do
-        expect { path_helper.validate_path("C:\\cool path\\dude.exe") }.not_to raise_error
+        expect { PathHelper.validate_path("C:\\cool path\\dude.exe") }.not_to raise_error
       end
 
       it "raises an error if the path has invalid characters" do
-        path_helper.stub(:printable?).and_return(false)
-        expect { path_helper.validate_path("Newline!\n") }.to raise_error(Chef::Exceptions::ValidationFailed)
+        PathHelper.stub(:printable?).and_return(false)
+        expect { PathHelper.validate_path("Newline!\n") }.to raise_error(Chef::Exceptions::ValidationFailed)
       end
 
       it "Adds the \\\\?\\ prefix if the path exceeds MAX_LENGTH and does not have it" do
         long_path = "C:\\" + "a" * 250 + "\\" + "b" * 250
         prefixed_long_path = "\\\\?\\" + long_path
-        path_helper.stub(:windows_max_length_exceeded?).and_return(true)
-        expect(path_helper.validate_path(long_path)).to eql(prefixed_long_path)
+        PathHelper.stub(:windows_max_length_exceeded?).and_return(true)
+        expect(PathHelper.validate_path(long_path)).to eql(prefixed_long_path)
       end
     end
   end
 
   describe "windows_max_length_exceeded?" do
     it "returns true if the path is too long (259 + NUL) for the API" do
-      expect(path_helper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 6)).to be_true
+      expect(PathHelper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 6)).to be_true
     end
 
     it "returns false if the path is not too long (259 + NUL) for the standard API" do
-      expect(path_helper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 5)).to be_false
+      expect(PathHelper.windows_max_length_exceeded?("C:\\" + "a" * 250 + "\\" + "b" * 5)).to be_false
     end
 
     it "returns false if the path is over 259 characters but uses the \\\\?\\ prefix" do
-      expect(path_helper.windows_max_length_exceeded?("\\\\?\\C:\\" + "a" * 250 + "\\" + "b" * 250)).to be_false
+      expect(PathHelper.windows_max_length_exceeded?("\\\\?\\C:\\" + "a" * 250 + "\\" + "b" * 250)).to be_false
     end
   end
 
   describe "printable?" do
     it "returns true if the string contains no non-printable characters" do
-      expect(path_helper.printable?("C:\\Program Files (x86)\\Microsoft Office\\Files.lst")).to be_true
+      expect(PathHelper.printable?("C:\\Program Files (x86)\\Microsoft Office\\Files.lst")).to be_true
     end
 
     it "returns true when given 'abc' in unicode" do
-      expect(path_helper.printable?("\u0061\u0062\u0063")).to be_true
+      expect(PathHelper.printable?("\u0061\u0062\u0063")).to be_true
     end
 
     it "returns true when given japanese unicode" do
-      expect(path_helper.printable?("\uff86\uff87\uff88")).to be_true
+      expect(PathHelper.printable?("\uff86\uff87\uff88")).to be_true
     end
 
     it "returns false if the string contains a non-printable character" do
-      expect(path_helper.printable?("\my files\work\notes.txt")).to be_false
+      expect(PathHelper.printable?("\my files\work\notes.txt")).to be_false
     end
 
     # This isn't necessarily a requirement, but here to be explicit about functionality.
     it "returns false if the string contains a newline or tab" do
-      expect(path_helper.printable?("\tThere's no way,\n\t *no* way,\n\t that you came from my loins.\n")).to be_false
+      expect(PathHelper.printable?("\tThere's no way,\n\t *no* way,\n\t that you came from my loins.\n")).to be_false
     end
   end
 
   describe "canonical_path" do
     context "on windows", :windows_only do
       it "returns an absolute path with backslashes instead of slashes" do
-        expect(path_helper.canonical_path("\\\\?\\C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
+        expect(PathHelper.canonical_path("\\\\?\\C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
       end
 
       it "adds the \\\\?\\ prefix if it is missing" do
-        expect(path_helper.canonical_path("C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
+        expect(PathHelper.canonical_path("C:/windows/win.ini")).to eq("\\\\?\\c:\\windows\\win.ini")
       end
 
       it "returns a lowercase path" do
-        expect(path_helper.canonical_path("\\\\?\\C:\\CASE\\INSENSITIVE")).to eq("\\\\?\\c:\\case\\insensitive")
+        expect(PathHelper.canonical_path("\\\\?\\C:\\CASE\\INSENSITIVE")).to eq("\\\\?\\c:\\case\\insensitive")
       end
     end
 
     context "not on windows", :unix_only  do
       context "ruby is at least 1.9", :ruby_gte_19_only do
         it "returns a canonical path" do
-          expect(path_helper.canonical_path("/etc//apache.d/sites-enabled/../sites-available/default")).to eq("/etc/apache.d/sites-available/default")
+          expect(PathHelper.canonical_path("/etc//apache.d/sites-enabled/../sites-available/default")).to eq("/etc/apache.d/sites-available/default")
         end
       end
 
       context "ruby is less than 1.9", :ruby_18_only do
         it "returns a canonical path" do
-          expect { path_helper.canonical_path("/etc//apache.d/sites-enabled/../sites-available/default") }.to raise_error(NotImplementedError)
+          expect { PathHelper.canonical_path("/etc//apache.d/sites-enabled/../sites-available/default") }.to raise_error(NotImplementedError)
         end
       end
     end
@@ -122,15 +205,15 @@ describe Chef::Util::PathHelper do
 
   describe "paths_eql?" do
     it "returns true if the paths are the same" do
-      path_helper.stub(:canonical_path).with("bandit").and_return("c:/bandit/bandit")
-      path_helper.stub(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
-      expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_true
+      PathHelper.stub(:canonical_path).with("bandit").and_return("c:/bandit/bandit")
+      PathHelper.stub(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      expect(PathHelper.paths_eql?("bandit", "../bandit/bandit")).to be_true
     end
 
     it "returns false if the paths are different" do
-      path_helper.stub(:canonical_path).with("bandit").and_return("c:/Bo/Bandit")
-      path_helper.stub(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
-      expect(path_helper.paths_eql?("bandit", "../bandit/bandit")).to be_false
+      PathHelper.stub(:canonical_path).with("bandit").and_return("c:/Bo/Bandit")
+      PathHelper.stub(:canonical_path).with("../bandit/bandit").and_return("c:/bandit/bandit")
+      expect(PathHelper.paths_eql?("bandit", "../bandit/bandit")).to be_false
      end
   end
 end