"Passing arguments to packages" functionality is broken

[bradland]

The following example will fail:

package :stub do
  runner "touch #{opts[:file_name]}"
end

policy :test_policy, :roles => :app do
  requires :stub, :file_name => 'arg-file.txt', :another_opt => 'value'
end

deployment { delivery :capistrano }

The failure is caused by opts[:file_name] being nil. The runner complains: touch: missing file operand.

I narrowed the simplest point of failure down to policy.rb#L98, which is inside Policy#process. If you change the argument passed to Package#instance at this location from *args to just args, the functionality works; provided the arguments passed to Policy#requires are a hash. However, I'm not sure that tells the whole story.

Restricting Policy#requires arguments to hashes is somewhat inconsistent with expectations elsewhere. For example, on policy.rb#L98, *args is passed to Package#instance. We can see that Package#instance appears to expect an arbitrary set of arguments:

(Excerpt from package.rb)

  def instance(*args)
    p=Package.new(name, @metadata) {}
    p.opts = defaults.merge(args.extract_options!)
    p.args = args
    p.instance_variable_set("@block", @block)
    p.instance_eval &@block
    p
  end

The method extract_options! comes from ActiveSupport, and generally expects an arbitrary array of arguments. If we try to pass arbitrary arguments to Policy#requires, we get other failures. The following will fail:

package :stub do
  runner "touch #{opts[:file_name]}"
end

policy :test_policy, :roles => :app do
  requires :stub, 1, 2, 3, :file_name => 'arg-file.txt', :another_opt => 'value'
end

deployment { delivery :capistrano }

With the error (excerpt):

sprinkle-0.7.2/lib/sprinkle/policy.rb:71:in `requires': wrong number of arguments (5 for 2) (ArgumentError)

This failure is because: Policy#requires(package, opts={}). The second argument must be a hash. If we change the second argument to *opts, the class PackageRepository begins to fall apart (trace excerpt):

sprinkle-0.7.2/lib/sprinkle/package/package_repository.rb:43:in `[]': can't convert Symbol into Integer (TypeError)

Ref: https://github.com/sprinkle-tool/sprinkle/blob/master/lib/sprinkle/package/package_repository.rb#L43

I didn't debug past this point, because I felt like the change started to break more than it fixes.

Because Package#instance is currently broken, the optimal solution would seem to be a small refactor there, abandoning the idea of separate opts and args, then specify that Policy#requires only accepts a hash as the second argument. Of course, it's your call. It's a single line change, so I didn't bother with a pull request.

[joshgoebel]

Please test against master. Does the latest commit resolve all your issues?

[bradland]

That did the trick!