(PUP-11348) Call regular ENV methods

joshcooper · joshcooper · commit 5021c97ff38f · 2023-04-14T10:05:08.000-07:00
Don't call deprecated environment methods in Puppet::Util
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
@@ -369,11 +369,12 @@ def self.initialize_default_settings!(settings)
           be set in `[server]`, `[agent]`, or an environment config section.",
         :call_hook => :on_define_and_write,
         :hook             => proc do |value|
-          Puppet::Util.set_env('PATH', '') if Puppet::Util.get_env('PATH').nil?
-          Puppet::Util.set_env('PATH', value) unless value == 'none'
-          paths = Puppet::Util.get_env('PATH').split(File::PATH_SEPARATOR)
+          ENV['PATH'] = '' if ENV['PATH'].nil?
+          ENV['PATH'] = value unless value == 'none'
+          paths = ENV['PATH'].split(File::PATH_SEPARATOR)
           Puppet::Util::Platform.default_paths.each do |path|
-            Puppet::Util.set_env('PATH', Puppet::Util.get_env('PATH') + File::PATH_SEPARATOR + path) unless paths.include?(path)
+            next if paths.include?(path)
+            ENV['PATH'] = ENV['PATH'] + File::PATH_SEPARATOR + path
           end
           value
         end
diff --git a/lib/puppet/file_system/uniquefile.rb b/lib/puppet/file_system/uniquefile.rb
@@ -150,7 +150,7 @@ def try_convert_to_hash(h)
 
   def tmpdir
     tmp = '.'
-    for dir in [ Puppet::Util.get_env('TMPDIR'), Puppet::Util.get_env('TMP'), Puppet::Util.get_env('TEMP'), @@systmpdir, '/tmp']
+    for dir in [ ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp']
       stat = File.stat(dir) if dir
       if stat && stat.directory? && stat.writable?
         tmp = dir
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
@@ -549,8 +549,8 @@ def hash
 
   # not private so it can be called in tests
   def self.extralibs()
-    if Puppet::Util.get_env('PUPPETLIB')
-      split_path(Puppet::Util.get_env('PUPPETLIB'))
+    if ENV['PUPPETLIB']
+      split_path(ENV['PUPPETLIB'])
     else
       []
     end
diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
@@ -55,9 +55,9 @@ def self.provider_command
   # that contains the content of PATH but without puppet/bin dir.
   # This is used to pass a custom PATH and execute commands in a controlled environment
   def self.windows_path_without_puppet_bin
-    @path ||= Puppet::Util.get_env('PATH').split(File::PATH_SEPARATOR)
-                                          .reject { |dir| dir =~ /puppet\\bin$/ }
-                                          .join(File::PATH_SEPARATOR)
+    @path ||= ENV['PATH'].split(File::PATH_SEPARATOR)
+                         .reject { |dir| dir =~ /puppet\\bin$/ }
+                         .join(File::PATH_SEPARATOR)
   end
 
   private_class_method :windows_path_without_puppet_bin
@@ -73,7 +73,7 @@ def self.execute_gem_command(command, command_options, custom_environment = {})
     validate_command(command)
     cmd = [command] << command_options
 
-    custom_environment = {'HOME'=>Puppet::Util.get_env('HOME')}.merge(custom_environment)
+    custom_environment = {'HOME'=>ENV['HOME']}.merge(custom_environment)
 
     if Puppet::Util::Platform.windows?
       custom_environment[:PATH] = windows_path_without_puppet_bin
diff --git a/lib/puppet/provider/package/puppet_gem.rb b/lib/puppet/provider/package/puppet_gem.rb
@@ -7,9 +7,9 @@
   confine :true => Puppet.runtime[:facter].value(:aio_agent_version)
 
   def self.windows_gemcmd
-    puppet_dir = Puppet::Util.get_env('PUPPET_DIR')
+    puppet_dir = ENV['PUPPET_DIR']
     if puppet_dir
-      File.join(Puppet::Util.get_env('PUPPET_DIR').to_s, 'bin', 'gem.bat')
+      File.join(ENV['PUPPET_DIR'].to_s, 'bin', 'gem.bat')
     else
       File.join(Gem.default_bindir, 'gem.bat')
     end
diff --git a/lib/puppet/test/test_helper.rb b/lib/puppet/test/test_helper.rb
@@ -69,13 +69,7 @@ def self.initialize()
     # @return nil
     def self.before_all_tests()
       # The process environment is a shared, persistent resource.
-      # Can't use Puppet.features.microsoft_windows? as it may be mocked out in a test.  This can cause test recurring test failures
-      if (!!File::ALT_SEPARATOR)
-        mode = :windows
-      else
-        mode = :posix
-      end
-      $old_env = Puppet::Util.get_environment(mode)
+      $old_env = ENV.to_hash
     end
 
     # Call this method once, at the end of a test run, when no more tests
@@ -188,19 +182,12 @@ def self.after_each_test()
       end
       $saved_indirection_state = nil
 
-      # Can't use Puppet.features.microsoft_windows? as it may be mocked out in a test.  This can cause test recurring test failures
-      if (!!File::ALT_SEPARATOR)
-        mode = :windows
-      else
-        mode = :posix
-      end
       # Restore the global process environment.  Can't just assign because this
       # is a magic variable, sadly, and doesn't do that™.  It is sufficiently
       # faster to use the compare-then-set model to avoid excessive work that it
       # justifies the complexity.  --daniel 2012-03-15
-      unless Puppet::Util.get_environment(mode) == $old_env
-        Puppet::Util.clear_environment(mode)
-        $old_env.each {|k, v| Puppet::Util.set_env(k, v, mode) }
+      if ENV.to_hash != $old_env
+        ENV.replace($old_env)
       end
 
       # Clear all environments
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
@@ -37,7 +37,6 @@ def default_env
   def create_erb(content)
     ERB.new(content, trim_mode: '-')
   end
-
   module_function :create_erb
 
   # @param name [String] The name of the environment variable to retrieve
@@ -258,16 +257,16 @@ def which(bin)
     if absolute_path?(bin)
       return bin if FileTest.file? bin and FileTest.executable? bin
     else
-      exts = Puppet::Util.get_env('PATHEXT')
+      exts = ENV['PATHEXT']
       exts = exts ? exts.split(File::PATH_SEPARATOR) : %w[.COM .EXE .BAT .CMD]
-      Puppet::Util.get_env('PATH').split(File::PATH_SEPARATOR).each do |dir|
+      ENV['PATH'].split(File::PATH_SEPARATOR).each do |dir|
         begin
           dest = File.expand_path(File.join(dir, bin))
         rescue ArgumentError => e
           # if the user's PATH contains a literal tilde (~) character and HOME is not set, we may get
           # an ArgumentError here.  Let's check to see if that is the case; if not, re-raise whatever error
           # was thrown.
-          if e.to_s =~ /HOME/ and (Puppet::Util.get_env('HOME').nil? || Puppet::Util.get_env('HOME') == "")
+          if e.to_s =~ /HOME/ and (ENV['HOME'].nil? || ENV['HOME'] == "")
             # if we get here they have a tilde in their PATH.  We'll issue a single warning about this and then
             # ignore this path element and carry on with our lives.
             #TRANSLATORS PATH and HOME are environment variables and should not be translated
diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb
@@ -148,7 +148,7 @@
       Puppet::Util.withenv( {"PATH" => path }, :windows) do
         Puppet.settings[:path] = "none" # this causes it to ignore the setting
 
-        expect(Puppet::Util.get_env('Path')).to eq(path)
+        expect(ENV['Path']).to eq(path)
       end
     end
   end
diff --git a/spec/unit/provider/package/gem_spec.rb b/spec/unit/provider/package/gem_spec.rb
@@ -51,8 +51,8 @@
 
         before do
           allow(Puppet::Util::Platform).to receive(:windows?).and_return(true)
-          allow(Puppet::Util).to receive(:get_env)
-          allow(Puppet::Util).to receive(:get_env).with('PATH').and_return(path)
+          allow(ENV).to receive(:[]).and_call_original
+          allow(ENV).to receive(:[]).with('PATH').and_return(path)
           allow(described_class).to receive(:validate_command).with(provider_gem_cmd)
           stub_const('::File::PATH_SEPARATOR', ';')
         end
diff --git a/spec/unit/provider/package/puppet_gem_spec.rb b/spec/unit/provider/package/puppet_gem_spec.rb
@@ -35,8 +35,8 @@
   describe '.windows_gemcmd' do
     context 'when PUPPET_DIR is not set' do
       before do
-        allow(Puppet::Util).to receive(:get_env).and_call_original
-        allow(Puppet::Util).to receive(:get_env).with('PUPPET_DIR').and_return(nil)
+        allow(ENV).to receive(:[]).and_call_original
+        allow(ENV).to receive(:[]).with('PUPPET_DIR').and_return(nil)
         allow(Gem).to receive(:default_bindir).and_return('default_gem_bin')
       end
 
@@ -48,8 +48,8 @@
 
     context 'when PUPPET_DIR is set' do
       before do
-        allow(Puppet::Util).to receive(:get_env).and_call_original
-        allow(Puppet::Util).to receive(:get_env).with('PUPPET_DIR').and_return('puppet_dir')
+        allow(ENV).to receive(:[]).and_call_original
+        allow(ENV).to receive(:[]).with('PUPPET_DIR').and_return('puppet_dir')
       end
 
       it 'uses Gem.default_bindir' do
diff --git a/spec/unit/test/test_helper_spec.rb b/spec/unit/test/test_helper_spec.rb
@@ -4,14 +4,14 @@
   context "#after_each_test" do
     it "restores the original environment" do
       varname = 'test_helper_spec-test_variable'
-      Puppet::Util.set_env(varname, "\u16A0")
+      ENV[varname] = "\u16A0"
 
-      expect(Puppet::Util.get_env(varname)).to eq("\u16A0")
+      expect(ENV[varname]).to eq("\u16A0")
 
       # Prematurely trigger the after_each_test method
       Puppet::Test::TestHelper.after_each_test
 
-      expect(Puppet::Util::get_env(varname)).to be_nil
+      expect(ENV[varname]).to be_nil
     end
   end
-end
+end
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
@@ -676,8 +676,8 @@ def withenv_utf8(&block)
       end
 
       it "should walk the search PATH returning the first executable" do
-        allow(Puppet::Util).to receive(:get_env).with('PATH').and_return(File.expand_path('/bin'))
-        allow(Puppet::Util).to receive(:get_env).with('PATHEXT').and_return(nil)
+        allow(ENV).to receive(:[]).with('PATH').and_return(File.expand_path('/bin'))
+        allow(ENV).to receive(:[]).with('PATHEXT').and_return(nil)
 
         expect(Puppet::Util.which('foo')).to eq(path)
       end
@@ -693,8 +693,8 @@ def withenv_utf8(&block)
 
       describe "when a file extension is specified" do
         it "should walk each directory in PATH ignoring PATHEXT" do
-          allow(Puppet::Util).to receive(:get_env).with('PATH').and_return(%w[/bar /bin].map{|dir| File.expand_path(dir)}.join(File::PATH_SEPARATOR))
-          allow(Puppet::Util).to receive(:get_env).with('PATHEXT').and_return('.FOOBAR')
+          allow(ENV).to receive(:[]).with('PATH').and_return(%w[/bar /bin].map{|dir| File.expand_path(dir)}.join(File::PATH_SEPARATOR))
+          allow(ENV).to receive(:[]).with('PATHEXT').and_return('.FOOBAR')
 
           expect(FileTest).to receive(:file?).with(File.join(File.expand_path('/bar'), 'foo.CMD')).and_return(false)
 
@@ -705,8 +705,8 @@ def withenv_utf8(&block)
       describe "when a file extension is not specified" do
         it "should walk each extension in PATHEXT until an executable is found" do
           bar = File.expand_path('/bar')
-          allow(Puppet::Util).to receive(:get_env).with('PATH').and_return("#{bar}#{File::PATH_SEPARATOR}#{base}")
-          allow(Puppet::Util).to receive(:get_env).with('PATHEXT').and_return(".EXE#{File::PATH_SEPARATOR}.CMD")
+          allow(ENV).to receive(:[]).with('PATH').and_return("#{bar}#{File::PATH_SEPARATOR}#{base}")
+          allow(ENV).to receive(:[]).with('PATHEXT').and_return(".EXE#{File::PATH_SEPARATOR}.CMD")
 
           expect(FileTest).to receive(:file?).ordered().with(File.join(bar, 'foo.EXE')).and_return(false)
           expect(FileTest).to receive(:file?).ordered().with(File.join(bar, 'foo.CMD')).and_return(false)
@@ -717,8 +717,8 @@ def withenv_utf8(&block)
         end
 
         it "should walk the default extension path if the environment variable is not defined" do
-          allow(Puppet::Util).to receive(:get_env).with('PATH').and_return(base)
-          allow(Puppet::Util).to receive(:get_env).with('PATHEXT').and_return(nil)
+          allow(ENV).to receive(:[]).with('PATH').and_return(base)
+          allow(ENV).to receive(:[]).with('PATHEXT').and_return(nil)
 
           %w[.COM .EXE .BAT].each do |ext|
             expect(FileTest).to receive(:file?).ordered().with(File.join(base, "foo#{ext}")).and_return(false)
@@ -729,8 +729,8 @@ def withenv_utf8(&block)
         end
 
         it "should fall back if no extension matches" do
-          allow(Puppet::Util).to receive(:get_env).with('PATH').and_return(base)
-          allow(Puppet::Util).to receive(:get_env).with('PATHEXT').and_return(".EXE")
+          allow(ENV).to receive(:[]).with('PATH').and_return(base)
+          allow(ENV).to receive(:[]).with('PATHEXT').and_return(".EXE")
 
           allow(FileTest).to receive(:file?).with(File.join(base, 'foo.EXE')).and_return(false)
           allow(FileTest).to receive(:file?).with(File.join(base, 'foo')).and_return(true)
