Please review pull request #166: (#12311) use 'ensure' to restore env vars in Resolution.with_env opened by (cprice-puppet)
Description:
This will make sure that the environment variables get restored to their original values, even if the caller's yield block throws an exception or uses a "return" statement.
- Opened: Thu Feb 02 23:58:36 UTC 2012
- Based on: puppetlabs:master (e980343562d82b334dda4775df0850a7e89deba4)
- Requested merge: cprice-puppet:bug/master/12311-with-env-needs-ensure (4341807e2d1ddddfe4355756922bd7bb530e966a)
Diff follows:
diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb
index 690a8cb..7e266c7 100644
--- a/lib/facter/util/resolution.rb
+++ b/lib/facter/util/resolution.rb
@@ -45,6 +45,8 @@ def self.with_env(values)
end
# execute the caller's block, capture the return value
rv = yield
+ # use an ensure block to make absolutely sure we restore the variables
+ ensure
# restore the old values
values.each do |var, value|
if old.include?(var)
@@ -89,14 +91,10 @@ def self.exec(code, interpreter = nil)
else
path = %x{which #{binary} 2>/dev/null}.chomp
# we don't have the binary necessary
- # we need to use "next" here, rather than "return"... because "return" would break out of the with_env wrapper
- # without performing the necessary cleanup of environment variables
- next if path == "" or path.match(/Command not found\./)
+ return if path == "" or path.match(/Command not found\./)
end
- # we need to use "next" here, rather than "return"... because "return" would break out of the with_env wrapper
- # without performing the necessary cleanup of environment variables
- next unless FileTest.exists?(path)
+ return unless FileTest.exists?(path)
end
out = nil
@@ -105,22 +103,16 @@ def self.exec(code, interpreter = nil)
out = %x{#{code}}.chomp
rescue Errno::ENOENT => detail
# command not found on Windows
- # we need to use "next" here, rather than "return"... because "return" would break out of the with_env wrapper
- # without performing the necessary cleanup of environment variables
- next
+ return
rescue => detail
$stderr.puts detail
- # we need to use "next" here, rather than "return"... because "return" would break out of the with_env wrapper
- # without performing the necessary cleanup of environment variables
- next
+ return
end
- # we need to use "next" here, rather than "return"... because "return" would break out of the with_env wrapper
- # without performing the necessary cleanup of environment variables
if out == ""
- next
+ return
else
- next out
+ return out
end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
