Merge pull request #3088 from chef/ksubrama/home_dir

Change all accesses to ENV['HOME'] or ~ to PathHelper.home instead.
chef · Mar 20, 2015 · 669047a · 669047a
2 parents 98ff0ea + 8366c82
commit 669047a
Show file tree

Hide file tree

Showing 15 changed files with 169 additions and 71 deletions.
diff --git a/lib/chef/config.rb b/lib/chef/config.rb
@@ -571,11 +571,12 @@ def self.env
     end
 
     def self.windows_home_path
-      env['SYSTEMDRIVE'] + env['HOMEPATH'] if env['SYSTEMDRIVE'] && env['HOMEPATH']
+      Chef::Log.deprecation("Chef::Config.windows_home_path is now deprecated.  Consider using Chef::Util::PathHelper.home instead.")
+      PathHelper.home
     end
 
     # returns a platform specific path to the user home dir if set, otherwise default to current directory.
-    default( :user_home ) { env['HOME'] || windows_home_path || env['USERPROFILE'] || Dir.pwd }
+    default( :user_home ) { PathHelper.home || Dir.pwd }
 
     # Enable file permission fixup for selinux. Fixup will be done
     # only if selinux is enabled in the system.

diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
@@ -21,6 +21,7 @@
 require 'erubis'
 require 'chef/knife/bootstrap/chef_vault_handler'
 require 'chef/knife/bootstrap/client_builder'
+require 'chef/util/path_helper'
 
 class Chef
   class Knife
@@ -268,7 +269,7 @@ def find_template
         bootstrap_files = []
         bootstrap_files << File.join(File.dirname(__FILE__), 'bootstrap/templates', "#{template}.erb")
         bootstrap_files << File.join(Knife.chef_config_dir, "bootstrap", "#{template}.erb") if Chef::Knife.chef_config_dir
-        bootstrap_files << File.join(ENV['HOME'], '.chef', 'bootstrap', "#{template}.erb") if ENV['HOME']
+        Chef::Util::PathHelper.home('.chef', 'bootstrap', "#{template}.erb") {|p| bootstrap_files << p}
         bootstrap_files << Gem.find_files(File.join("chef","knife","bootstrap","#{template}.erb"))
         bootstrap_files.flatten!
 

diff --git a/lib/chef/knife/core/subcommand_loader.rb b/lib/chef/knife/core/subcommand_loader.rb
@@ -28,9 +28,15 @@ class SubcommandLoader
       attr_reader :chef_config_dir
       attr_reader :env
 
-      def initialize(chef_config_dir, env=ENV)
-        @chef_config_dir, @env = chef_config_dir, env
+      def initialize(chef_config_dir, env=nil)
+        @chef_config_dir = chef_config_dir
         @forced_activate = {}
+
+        # Deprecated and un-used instance variable.
+        @env = env
+        unless env.nil?
+          Chef::Log.deprecation("The env argument to Chef::Knife::SubcommandLoader is deprecated. If you are using env to inject/mock HOME, consider mocking Chef::Util::PathHelper.home instead.")
+        end
       end
 
       # Load all the sub-commands
@@ -49,7 +55,9 @@ def site_subcommands
         end
 
         # finally search ~/.chef/plugins/knife/*.rb
-        user_specific_files.concat Dir.glob(File.join(Chef::Util::PathHelper.escape_glob(env['HOME'], '.chef', 'plugins', 'knife'), '*.rb')) if env['HOME']
+        Chef::Util::PathHelper.home('.chef', 'plugins', 'knife') do |p|
+          user_specific_files.concat Dir.glob(File.join(Chef::Util::PathHelper.escape_glob(p), '*.rb'))
+        end
 
         user_specific_files
       end
@@ -140,15 +148,15 @@ def find_subcommands_via_rubygems
       end
 
       def have_plugin_manifest?
-        ENV["HOME"] && File.exist?(plugin_manifest_path)
+        plugin_manifest_path && File.exist?(plugin_manifest_path)
       end
 
       def plugin_manifest
         Chef::JSONCompat.from_json(File.read(plugin_manifest_path))
       end
 
       def plugin_manifest_path
-        File.join(ENV['HOME'], '.chef', 'plugin_manifest.json')
+        Chef::Util::PathHelper.home('.chef', 'plugin_manifest.json')
       end
 
       private

diff --git a/lib/chef/knife/exec.rb b/lib/chef/knife/exec.rb
@@ -17,6 +17,7 @@
 #
 
 require 'chef/knife'
+require 'chef/util/path_helper'
 
 class Chef::Knife::Exec < Chef::Knife
 
@@ -42,7 +43,7 @@ def run
 
     # Default script paths are chef-repo/.chef/scripts and ~/.chef/scripts
     config[:script_path] << File.join(Chef::Knife.chef_config_dir, 'scripts') if Chef::Knife.chef_config_dir
-    config[:script_path] << File.join(ENV['HOME'], '.chef', 'scripts') if ENV['HOME']
+    Chef::Util::PathHelper.home('.chef', 'scripts') { |p| config[:script_path] << p }
 
     scripts = Array(name_args)
     context = Object.new

diff --git a/lib/chef/knife/ssh.rb b/lib/chef/knife/ssh.rb
@@ -31,6 +31,7 @@ class Ssh < Knife
         require 'chef/search/query'
         require 'chef/mixin/shell_out'
         require 'chef/mixin/command'
+        require 'chef/util/path_helper'
         require 'mixlib/shellout'
       end
 
@@ -342,8 +343,10 @@ def interactive
 
       def screen
         tf = Tempfile.new("knife-ssh-screen")
-        if File.exist? "#{ENV["HOME"]}/.screenrc"
-          tf.puts("source #{ENV["HOME"]}/.screenrc")
+        Chef::Util::PathHelper.home('.screenrc') do |screenrc_path|
+          if File.exist? screenrc_path
+            tf.puts("source #{screenrc_path}")
+          end
         end
         tf.puts("caption always '%-Lw%{= BW}%50>%n%f* %t%{-}%+Lw%<'")
         tf.puts("hardstatus alwayslastline 'knife ssh #{@name_args[0]}'")

diff --git a/lib/chef/provider/service/macosx.rb b/lib/chef/provider/service/macosx.rb
@@ -33,8 +33,7 @@ def self.gather_plist_dirs
                          /Library/LaunchDaemons
                          /System/Library/LaunchAgents
                          /System/Library/LaunchDaemons }
-
-          locations << "#{ENV['HOME']}/Library/LaunchAgents" if ENV['HOME']
+          Chef::Util::PathHelper.home('Library', 'LaunchAgents') { |p| locations << p }
           locations
         end
 

diff --git a/lib/chef/shell.rb b/lib/chef/shell.rb
@@ -29,6 +29,7 @@
 require 'chef/shell/shell_session'
 require 'chef/shell/ext'
 require 'chef/json_compat'
+require 'chef/util/path_helper'
 
 # = Shell
 # Shell is Chef in an IRB session. Shell can interact with a Chef server via the
@@ -101,7 +102,7 @@ def self.irb_conf
   end
 
   def self.configure_irb
-    irb_conf[:HISTORY_FILE] = "~/.chef/chef_shell_history"
+    irb_conf[:HISTORY_FILE] = Chef::Util::PathHelper.home(".chef", "chef_shell_history")
     irb_conf[:SAVE_HISTORY] = 1000
 
     irb_conf[:IRB_RC] = lambda do |conf|
@@ -295,18 +296,19 @@ def parse_opts
     private
 
     def config_file_for_shell_mode(environment)
+      dot_chef_dir = Chef::Util::PathHelper.home('.chef')
       if config[:config_file]
         config[:config_file]
-      elsif environment && ENV['HOME']
+      elsif environment
         Shell.env = environment
-        config_file_to_try = ::File.join(ENV['HOME'], '.chef', environment, 'chef_shell.rb')
+        config_file_to_try = ::File.join(dot_chef_dir, environment, 'chef_shell.rb')
         unless ::File.exist?(config_file_to_try)
           puts "could not find chef-shell config for environment #{environment} at #{config_file_to_try}"
           exit 1
         end
         config_file_to_try
-      elsif ENV['HOME'] && ::File.exist?(File.join(ENV['HOME'], '.chef', 'chef_shell.rb'))
-        File.join(ENV['HOME'], '.chef', 'chef_shell.rb')
+      elsif dot_chef_dir && ::File.exist?(File.join(dot_chef_dir, 'chef_shell.rb'))
+        File.join(dot_chef_dir, 'chef_shell.rb')
       elsif config[:solo]
         Chef::Config.platform_specific_path("/etc/chef/solo.rb")
       elsif config[:client]

diff --git a/lib/chef/util/path_helper.rb b/lib/chef/util/path_helper.rb
@@ -142,6 +142,82 @@ def self.escape_glob(*parts)
       def self.relative_path_from(from, to)
         pathname = Pathname.new(Chef::Util::PathHelper.cleanpath(to)).relative_path_from(Pathname.new(Chef::Util::PathHelper.cleanpath(from)))
       end
+
+      # Retrieves the "home directory" of the current user while trying to ascertain the existence
+      # of said directory.  The path returned uses / for all separators (the ruby standard format).
+      # If the home directory doesn't exist or an error is otherwise encountered, nil is returned.
+      # 
+      # If a set of path elements is provided, they are appended as-is to the home path if the
+      # homepath exists. 
+      # 
+      # If an optional block is provided, the joined path is passed to that block if the home path is
+      # valid and the result of the block is returned instead.
+      #
+      # Home-path discovery is performed once.  If a path is discovered, that value is memoized so
+      # that subsequent calls to home_dir don't bounce around.
+      #
+      # See self.all_homes.
+      def self.home(*args)
+        @@home_dir ||= self.all_homes { |p| break p }
+        if @@home_dir
+          path = File.join(@@home_dir, *args)
+          block_given? ? (yield path) : path
+        end
+      end
+
+      # See self.home.  This method performs a similar operation except that it yields all the different
+      # possible values of 'HOME' that one could have on this platform.  Hence, on windows, if
+      # HOMEDRIVE\HOMEPATH and USERPROFILE are different, the provided block will be called twice.
+      # This method goes out and checks the existence of each location at the time of the call.
+      #
+      # The return is a list of all the returned values from each block invocation or a list of paths
+      # if no block is provided.
+      def self.all_homes(*args)
+        paths = []
+        if Chef::Platform.windows?
+          # By default, Ruby uses the the following environment variables to determine Dir.home:
+          # HOME
+          # HOMEDRIVE HOMEPATH
+          # USERPROFILE
+          # Ruby only checks to see if the variable is specified - not if the directory actually exists.
+          # On Windows, HOMEDRIVE HOMEPATH can point to a different location (such as an unavailable network mounted drive)
+          # while USERPROFILE points to the location where the user application settings and profile are stored.  HOME
+          # is not defined as an environment variable (usually).  If the home path actually uses UNC, then the prefix is
+          # HOMESHARE instead of HOMEDRIVE.
+          #
+          # We instead walk down the following and only include paths that actually exist.
+          # HOME
+          # HOMEDRIVE HOMEPATH
+          # HOMESHARE HOMEPATH
+          # USERPROFILE
+
+          paths << ENV['HOME']
+          paths << ENV['HOMEDRIVE'] + ENV['HOMEPATH'] if ENV['HOMEDRIVE'] && ENV['HOMEPATH']
+          paths << ENV['HOMESHARE'] + ENV['HOMEPATH'] if ENV['HOMESHARE'] && ENV['HOMEPATH']
+          paths << ENV['USERPROFILE']
+        end
+        paths << Dir.home
+
+        # Depending on what environment variables we're using, the slashes can go in any which way.
+        # Just change them all to / to keep things consistent.
+        # Note: Maybe this is a bad idea on some unixy systems where \ might be a valid character depending on
+        # the particular brand of kool-aid you consume.  This code assumes that \ and / are both
+        # path separators on any system being used.
+        paths = paths.map { |home_path| home_path.gsub(path_separator, ::File::SEPARATOR) if home_path }
+
+        # Filter out duplicate paths and paths that don't exist.
+        valid_paths = paths.select { |home_path| home_path && Dir.exists?(home_path) }
+        valid_paths = valid_paths.uniq
+
+        # Join all optional path elements at the end.
+        # If a block is provided, invoke it - otherwise just return what we've got.
+        joined_paths = valid_paths.map { |home_path| File.join(home_path, *args) }
+        if block_given?
+          joined_paths.each { |p| yield p }
+        else
+          joined_paths
+        end
+      end
     end
   end
 end

diff --git a/lib/chef/workstation_config_loader.rb b/lib/chef/workstation_config_loader.rb
@@ -19,6 +19,7 @@
 require 'chef/config_fetcher'
 require 'chef/config'
 require 'chef/null_logger'
+require 'chef/util/path_helper'
 
 class Chef
 
@@ -112,9 +113,9 @@ def locate_local_config
         candidate_configs << File.join(chef_config_dir, 'knife.rb')
       end
       # Look for $HOME/.chef/knife.rb
-      if env['HOME']
-        candidate_configs << File.join(env['HOME'], '.chef', 'config.rb')
-        candidate_configs << File.join(env['HOME'], '.chef', 'knife.rb')
+      Chef::Util::PathHelper.home('.chef') do |dot_chef_dir|
+        candidate_configs << File.join(dot_chef_dir, 'config.rb')
+        candidate_configs << File.join(dot_chef_dir, 'knife.rb')
       end
 
       candidate_configs.find do | candidate_config |

diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb
@@ -360,18 +360,12 @@ def to_platform(*args)
       describe "Chef::Config[:user_home]" do
         it "should set when HOME is provided" do
           expected = to_platform("/home/kitten")
-          allow(Chef::Config).to receive(:env).and_return({ 'HOME' => expected })
-          expect(Chef::Config[:user_home]).to eq(expected)
-        end
-
-        it "should be set when only USERPROFILE is provided" do
-          expected = to_platform("/users/kitten")
-          allow(Chef::Config).to receive(:env).and_return({ 'USERPROFILE' => expected })
+          allow(Chef::Util::PathHelper).to receive(:home).and_return(expected)
           expect(Chef::Config[:user_home]).to eq(expected)
         end
 
         it "falls back to the current working directory when HOME and USERPROFILE is not set" do
-          allow(Chef::Config).to receive(:env).and_return({})
+          allow(Chef::Util::PathHelper).to receive(:home).and_return(nil)
           expect(Chef::Config[:user_home]).to eq(Dir.pwd)
         end
       end

diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
@@ -115,23 +115,17 @@ def configure_chef_config_dir
       end
 
       def configure_env_home
-        ENV['HOME'] = "/env/home"
+        allow(Chef::Util::PathHelper).to receive(:home).with(".chef", "bootstrap", "example.erb").and_yield(env_home_template_path)
       end
 
       def configure_gem_files
         allow(Gem).to receive(:find_files).and_return([ gem_files_template_path ])
       end
 
       before(:each) do
-        @original_home = ENV['HOME']
-        ENV['HOME'] = nil
         expect(File).to receive(:exists?).with(bootstrap_template).and_return(false)
       end
 
-      after(:each) do
-        ENV['HOME'] = @original_home
-      end
-
       context "when file is available everywhere" do
         before do
           configure_chef_config_dir
@@ -161,7 +155,7 @@ def configure_gem_files
         end
       end
 
-      context "when file is available in ENV['HOME']" do
+      context "when file is available in home directory" do
         before do
           configure_chef_config_dir
           configure_env_home
@@ -178,9 +172,27 @@ def configure_gem_files
       end
 
       context "when file is available in Gem files" do
+        before do
+          configure_chef_config_dir
+          configure_env_home
+          configure_gem_files
+
+          expect(File).to receive(:exists?).with(builtin_template_path).and_return(false)
+          expect(File).to receive(:exists?).with(chef_config_dir_template_path).and_return(false)
+          expect(File).to receive(:exists?).with(env_home_template_path).and_return(false)
+          expect(File).to receive(:exists?).with(gem_files_template_path).and_return(true)
+        end
+
+        it "should load the template from Gem files" do
+          expect(knife.find_template).to eq(gem_files_template_path)
+        end
+      end
+
+      context "when file is available in Gem files and home dir doesn't exist" do
         before do
           configure_chef_config_dir
           configure_gem_files
+          allow(Chef::Util::PathHelper).to receive(:home).with(".chef", "bootstrap", "example.erb").and_return(nil)
 
           expect(File).to receive(:exists?).with(builtin_template_path).and_return(false)
           expect(File).to receive(:exists?).with(chef_config_dir_template_path).and_return(false)