has_compiler fact not set about puppet-homebrew HOT 14 CLOSED

Hey! I don't have the time at the moment to dive into exactly why this happens, but I've worked around the issue for 1.3.1. If you update to v1.3.1, it should avoid trying to download and install the package. If the issue persists, please let me know.

Leaving this issue open as a TODO.

from puppet-homebrew.

Thanks for the quick response.

I now get:

Error: Unknown function warn at .../modules/homebrew/manifests/compiler.pp:19 on node ...

(repeats twice)
Where line 19 reads:

warn('No Command Line Tools detected and no download source set. If Command Line Tools is installed, this may be a false positive. If not, please set download sources or install manually.')

In any case - the provider still doesn't work because it doesn't set $::has_compiler.

from puppet-homebrew.

One interesting point is that by commenting out the following, the first confine :operatingsystem => :darwin started matching:

  # /usr/bin/cc exists in Mavericks, but it's not real
#  confine :macosx_productversion_major => '10.9'
#  setcode do
#    (File.exists?('/Applications/Xcode.app') or File.exists?('/Library/Developer/CommandLineTools/')) and
#    (File.exists?('/usr/bin/cc') || system('/usr/bin/xcrun -find cc >/dev/null 2>&1'))
#  end

It now still fails on the missing warn function but at least a puts which I added inside the confine is now reached.

Perhaps this is something to do with setting has_weight?

from puppet-homebrew.

I think I have a fix; facter is annoying. Could you replace you has_compiler.rb file with

Facter.add(:has_compiler) do
  setcode do
    warn('Unsupported OS.')
    nil
  end
end

Facter.add(:has_compiler) do
  confine :operatingsystem => :darwin
  setcode do
    File.exists?('/usr/bin/cc') || system('/usr/bin/xcrun -find cc >/dev/null 2>&1')
  end
end

Facter.add(:has_compiler) do
  # /usr/bin/cc exists in Mavericks, but it's not real
  confine :operatingsystem => :darwin, :macosx_productversion_major => '10.9'
  setcode do
    (File.exists?('/Applications/Xcode.app') or File.exists?('/Library/Developer/CommandLineTools/')) and
    (File.exists?('/usr/bin/cc') || system('/usr/bin/xcrun -find cc >/dev/null 2>&1'))
  end
end

and let me know if that works out?

from puppet-homebrew.

(also: thanks for pointing out the warn issue. TIL puppet is not ruby; the puppet warning function maps to warn in ruby)

from puppet-homebrew.

OK, I tried this and there are some mixed rerults:

1. ::has_compiler is now set to true
2. its value isn't recognised as true even though it prints it that way. ie. here is the code after I added a couple of debug warning calls (and fixed warn):

class homebrew::compiler {

  warning("has_compiler: \"${::has_compiler}\"")
  if $::has_compiler == true {
    warning("has_compiler is true")
  } elsif versioncmp($::macosx_productversion_major, '10.7') < 0 {
    warning('Please install the Command Line Tools bundled with XCode manually!')
  } else {

    warning("has_compiler is not true and OSX is newer")
    if ($homebrew::command_line_tools_package and $homebrew::command_line_tools_source) {

      notice('Installing Command Line Tools.')

      package { $homebrew::command_line_tools_package:
        ensure   => present,
        provider => pkgdmg,
        source   => $homebrew::command_line_tools_source,
      }

    } else {
      warning('No Command Line Tools detected and no download source set. If Command Line Tools is installed, this may be a false positive. If not, please set download sources or install manually.')
    }

  }

}

And here is the output:

Warning: Scope(Class[Homebrew::Compiler]): has_compiler: "true"
Warning: Scope(Class[Homebrew::Compiler]): has_compiler is not true and OSX is newer
Warning: Scope(Class[Homebrew::Compiler]): No Command Line Tools detected and no download source set. If Command Line Tools is installed, this may be a false positive. If not, please set download sources or install manually.

The $::has_compiler == true seems to fail even though it prints true in the warning just before it. I suspect something about string vs. 'true' constant.

Changing this condition to:

  if $::has_compiler {

made the code pass, but I'd like to understand why it fails without this change (or actually, why do we need this empty if at all).

from puppet-homebrew.

Hmm, interestingly it "works for me" (tm) on OSX 10.10 with puppet 3.8.6 and facter 2.4.6. I don't have an OSX 10.11 box, I wonder if that's the problem. Sorry for this taking so long to resolve, it looks like puppet is treating the symbols differently than I am expecting... I will continue looking into this tomorrow.

My best guess as to why your change passes is since puppet is treating the true boolean as a string (?), and all non-empty strings are truthy, if $::has_compiler is thus truthy and succeeds. Unfortunately, we couldn't use that in the codebase since I suspect the string "false" would also be truthy... thus when you do not have a compiler, if $::has_compiler would still be true.

As for why we need the if, we just don't want to either throw the pre-10.7 warning nor try to install the CLI tools if the compiler already exists on the path; trying to install a second version could lead to all sorts of unexpected behaviour.

Thanks for helping to track this down!

from puppet-homebrew.

Yes I suspect something similar (about the "truthfulness" of "true" vs. "false").

My question is about the empty "if" - why not reverse it and wrap the other conditions inside, e.g.:

  warning("has_compiler: \"${::has_compiler}\"")
  if ! $::has_compiler {
    if versioncmp($::macosx_productversion_major, '10.7') < 0 {
      warning('Please install the Command Line Tools bundled with XCode manually!')
    } else {
      ...
    }
  }

from puppet-homebrew.

For a couple reasons; the first being that the codebase I forked this from did it that way :P The second being that I agree with it stylistically; nested if statements can quickly get out of control and are somewhat against the spirit of ruby, where return unless condition is the nicest way of doing things -- ie. rather than

if condition
   # SNIP ...
   return final_result
end

we prefer

return unless condition

# SNIP ...
return final_result

Clearly both do the same thing, but one is less nested and thus takes up less horizontal real-estate and is -- theoretically -- easier to read.

from puppet-homebrew.

Please test with #20 -- if that works for you, I'll go ahead and cut a new release.

from puppet-homebrew.

Works.

I haven't started the Vagrant box from scratch because of time constraints (other parts of the Puppet manifest take ages and I'm digging other problems right now), but the module didn't show any of the problems it had before.

Thanks.

from puppet-homebrew.

Released as 1.3.2, should now be available on the Forge. Please let me know if you have any other issues!

from puppet-homebrew.

I've updated to 1.3.2 and it works fine. Still have to see how it will behave when starting from scratch but I'll report this separately if it turns out to be a problem. Thanks very much for your response!

A separate issue might be that the module defaults to user 'root' - current version of Homebrew refuses to run as root. I already setup a dedicated user 'homebrew' so I could just specify it when importing the class. I'm not sure what I suggest to do about this - perhaps drop the default and force picking a user?

from puppet-homebrew.

Nice catch, fix for this will be in the next release (see PR #21). Thanks again!

from puppet-homebrew.