Navigation Menu

Skip to content

Commit

Permalink
(#12012) Remove global override of LANG environment variable
Browse files Browse the repository at this point in the history
'LANG' environment variable must be set to 'C' in order to reliably parse output of certain commands when building up core Facter facts (to prevent locale differences from affecting the format of the output).  However, it does not need to happen at a global-ish scope whenever facter is 'require'd;  move this behavior from the facter.rb into the local scope of the Facter::Util::Resolution.exec method; restore original value of environment variable after executing any individual command.

Note that, while this behavior is better than the previous behavior (global override), it is still not possible for a custom facter fact to be written to expect any other locale besides 'C'.  Eventually might prefer to find a solution that allows callers to override this behavior if desired.
  • Loading branch information
Chris Price committed Jan 20, 2012
1 parent 26918b3 commit 810c465
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 39 deletions.
4 changes: 0 additions & 4 deletions lib/facter.rb
Expand Up @@ -39,10 +39,6 @@ module Util; end
# puts Facter['operatingsystem']
#

# Set LANG to force i18n to C
#
ENV['LANG'] = 'C'

GREEN = ""
RESET = ""
@@debug = 0
Expand Down
78 changes: 43 additions & 35 deletions lib/facter/util/resolution.rb
Expand Up @@ -64,47 +64,55 @@ def self.with_env(values)
def self.exec(code, interpreter = nil)
Facter.warnonce "The interpreter parameter to 'exec' is deprecated and will be removed in a future version." if interpreter

# Try to guess whether the specified code can be executed by looking at the
# first word. If it cannot be found on the PATH defer on resolving the fact
# by returning nil.
# This only fails on shell built-ins, most of which are masked by stuff in
# /bin or of dubious value anyways. In the worst case, "sh -c 'builtin'" can
# be used to work around this limitation
#
# Windows' %x{} throws Errno::ENOENT when the command is not found, so we
# can skip the check there. This is good, since builtins cannot be found
# elsewhere.
if have_which and !Facter::Util::Config.is_windows?
path = nil
binary = code.split.first
if code =~ /^\//
path = binary
else
path = %x{which #{binary} 2>/dev/null}.chomp
# we don't have the binary necessary
return nil if path == "" or path.match(/Command not found\./)

## Set LANG to force i18n to C for the duration of this exec; this ensures that any code that parses the
## output of the command can expect it to be in a consistent / predictable format / locale
with_env "LANG" => "C" do

# Try to guess whether the specified code can be executed by looking at the
# first word. If it cannot be found on the PATH defer on resolving the fact
# by returning nil.
# This only fails on shell built-ins, most of which are masked by stuff in
# /bin or of dubious value anyways. In the worst case, "sh -c 'builtin'" can
# be used to work around this limitation
#
# Windows' %x{} throws Errno::ENOENT when the command is not found, so we
# can skip the check there. This is good, since builtins cannot be found
# elsewhere.
if have_which and !Facter::Util::Config.is_windows?
path = nil
binary = code.split.first
if code =~ /^\//
path = binary
else
path = %x{which #{binary} 2>/dev/null}.chomp
# we don't have the binary necessary
return nil if path == "" or path.match(/Command not found\./)
end

return nil unless FileTest.exists?(path)
end

return nil unless FileTest.exists?(path)
end
out = nil

out = nil
begin
out = %x{#{code}}.chomp
rescue Errno::ENOENT => detail
# command not found on Windows
return nil
rescue => detail
$stderr.puts detail
return nil
end

begin
out = %x{#{code}}.chomp
rescue Errno::ENOENT => detail
# command not found on Windows
return nil
rescue => detail
$stderr.puts detail
return nil
end
if out == ""
return nil
else
return out
end

if out == ""
return nil
else
return out
end

end

# Add a new confine to the resolution mechanism.
Expand Down

0 comments on commit 810c465

Please sign in to comment.