Adds 'chef generate helpers [COOKBOOK] HELPERS_NAME'

[burtlo]

Description

I wanted to provide support for generating a library helper file in the libraries folder.

Issues Resolved

None

Check List

• New functionality includes tests
• All tests pass
• RELEASE_NOTES.md has been updated if required (not required for bugfixes, required for API changes)
• All commits have been signed-off for the Developer Certificate of Origin. See https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

Signed-off-by: Franklin Webber franklin@chef.io

[coderanger]

So two concerns, first that if we do move forward with advising people that helper == global DSL modification we should use the more technically correct form of either injecting the mixin into one of the DSL modules or into Recipe+Resource+Provider.

Second: I really don't like the idea of advising users, especially new users, to make global DSL changes. It's really footgun-y in a way that is hard to unwind later on. I personally advise people use the more lexical-ish extend pattern, but that's got some sharp edges if you don't know what is going on.

[coderanger]

Also really really never run global DSL changes from a recipe, it causes basically no end of subtle bugs :(

[kameghamegha]

I had noticed that something like the following (which is placed in the library file, in this case under the end for module OperatingSystemHelpers) seems to have been adopted as being the best practice. Are you suggesting something else? (I'm not sure of the syntax for what you are suggesting @coderanger )

Chef::Recipe.send(:include, OperatingSystemHelpers)
Chef::Resource.send(:include, OperatingSystemHelpers)
Chef::Provider.send(:include, OperatingSystemHelpers)

[coderanger]

You don't need the send anymore because include is a public method in Ruby 2.3, and yes that is one way to implement global DSL changes. But you probably shouldn't be doing that unless you've got a really really good reason.

[kameghamegha]

What is your suggestion then? :) I'm unaware of other ways (because I'm only just learning all of this). And maybe an explanation about why not to do this?

[coderanger]

https://coderanger.net/chef-tips/#3 is the pattern I recommend to people currently, working on something better but haven't had the cycles.

[burtlo]

Updates to the template based on conversation by @coderanger and @kameghamegha.

[coderanger]

If we want to support this, this is probably as good as it can be. I won't block it if others feel strongly but I'm still concerned about the level of potential confusion. Specifically this code will not work:

extend MyCookbook::Helpers

my_resource 'foo' do
  something my_helper_method
end

because the helper is only available from the resource context. If you know Ruby internals this makes some level of sense, but if you're treating it more like a Rails-style helper DSL it's less obvious.

[burtlo]

I definitely agree @coderanger. I think we could address this by bettering the documentation that is mentioned at the top (https://docs.chef.io/libraries.html).

[coderanger]

@burtlo The other solution is write a better pattern, but it is out of scope for this PR :) It would be possible to make something fancy either within Chef or as a library cookbook (my plan is the latter because that's how I roll but if someone else gets there first ...) which sets up the module so that it hooks define_resource and also dumps the helper into all resource objects in that recipe. Would give better lexical behavior, but this is better than telling people to figure it out themselves, at least this will mostly fail towards user frustration rather than it-works-but-youll-regret it :D

[tduffield]

@burtlo if you rebase this on latest master, I think we can get this merged.

[burtlo]

I have rebased on master with no conflicts. It seems Github here is not pleased though.