I would like to rewrite the way chef loads itself, but before I begin, I would like everyone who's interested to have an opportunity to offer their opinions on some of the sticky issues surrounding this.
First, some background: currently, chef uses require() throughout the source tree, with each file require()-ing the files that it needs to work. As one example, cookbook_loader.rb has the following at the top, just below the license:
require 'chef/config'
require 'chef/cookbook'
require 'chef/cookbook/metadata'
While this approach has the advantage of communicating file dependencies with the dependent code, I think it needs to be rewritten because many of the require statements overlap and ruby's exact behavior for overlapping require statements varies from version to version. Most notably, the code as it is now causes a NameError for uninitialized constants in ruby 1.9 and, less importantly, in some 1.8.x rubies before 1.8.5. In addition to compatibility issues, the current file load process causes some files to be loaded multiple times, leading to ruby warnings and kludgy `unless defined?(Const)` workarounds.
Before making any changes, I'd love to hear from everyone about the following issues:
* Relative load paths vs. Absolute
There is an open ticket, CHEF-557 [1], which states that relative load paths are evil because other libraries with identical relative paths may take precedence over a file in chef and be loaded instead. For example, "not_chef/lib/chef/node.rb" could be loaded instead of chef's node.rb. I don't foresee this particular issue being common, but this approach does have the benefit of making multiple loads of the same file basically impossible and prevents all load path related problems (by ignoring the load path entirely). The code would look like:
CHEF_LIB_ROOT = File.expand_path(File.dirname(__FILE__))
require CHEF_LIB_ROOT + "/chef/node.rb"
Alternatively, some argue for the relative approach, with the same "you're doing it wrong" zeal. One example is posted on the Riding Rails blog [2]. The argument here is that the package manager, usually rubygems, will take care of the load path, so stop worrying about it.
Personally, I lean a little towards the relative approach, but I've used both in my personal code. The absolute paths are certainly more verbose but less error prone. The inherent verbosity of this approach could be reduced by defining a method to encapsulate the require and absolute path, such as `Chef.requires "chef/node"` or, defined on Object, `chef_requires "chef/node"
Some of you I've talked to already have strong opinions about this; I'll happily do either.
* Namespace issues
First of all, I'd like for Chef-the-namespace to be a module. This is much more flexible than as a class. For example, spec_helper.rb for the client/solo (the chef/chef directory) could have `include Chef` and eliminate the need for to prefix every constant with Chef:: (i.e., specs could use Node instead of Chef::Node).
I also prefer the pattern for classes such as Resource and Provider to have a "Base" class that subclasses inherit from. For example, I'd prefer to have
module Chef
module Resource
class Base
# move everything in Chef::Resource here
end
end
end
module Chef
module Resource
class Package < Chef::Resource::Base
# ...
end
end
end
The advantage of this approach is that Chef::Resource is just a namespace module, so chef/resource.rb can require chef/resource/base and then require chef/resource/* and there won't be any errors from undefined constants. This keeps file loads grouped together by function, and they don't need to be "snuck in" at the bottom of the file.
* Don't require rubygems
Finally, it should be possible to move any require "rubygems" into the executables and out of the lib/ dir. This would put chef in line with accepted practices and hopefully make packaging a bit easier.
Looking forward to hearing your opinions on this.
Daniel DeLeo