search

sprinkle-tool / sprinkle

Sprinkle is a software provisioning tool you can use to build remote servers with. eg. to install a Rails, or Sinatra stack on a brand new slice directly after its been created
https://github.com/sprinkle-tool/sprinkle
MIT License
1.15k stars 138 forks source link

Bad error message on missing package #151

Closed troelskn closed 11 years ago

troelskn commented 11 years ago

Requiring an unmet dependency gives a rather obscure error message:

Selecting  for virtual package foobar
/Users/troelskn/.rvm/gems/ruby-1.9.3-p327/gems/sprinkle-0.7.4/lib/sprinkle/policy.rb:99:in `block in process': undefined method `instance' for nil:NilClass (NoMethodError)

I would suggest adding an explicit check and meaningful error message in package/chooser.rb, as in:

module Sprinkle::Package
  class Chooser #:nodoc:
    def self.select_package(name, packages)
      raise "No available choices for virtual package #{name}" if packages.empty?
joshgoebel commented 11 years ago

This should probably be consistent and raise a MissingPackage error anytime a require package cannot be found... and you have two scopes: packages inside policies and packages inside packages. If you want to fix both cases and submit a pull request that'd be great.

joshgoebel commented 11 years ago

Might have to do a bit more research though - right now it appears there is already a raise anytime we search the package database.... Look at Policy line 96...

joshgoebel commented 11 years ago

Though maybe the problem it find_all returns an array now which is always non-nil?

troelskn commented 11 years ago

You're right. Changing this line:

raise "Package definition not found for key: #{p}" unless package

into:

raise "Package definition not found for key: #{p}" unless package && package.any?

gives a useful error. Do you want a pull for this?

joshgoebel commented 11 years ago

Sure. I'm not sure package && is required... Look and see if the library always returns an array.

troelskn commented 11 years ago

Looks right. I'll wrap the error message in a typed exception while I'm at it.

troelskn commented 11 years ago

Closing this as duplicate of #154