FreeBSD package provider should not need the port tree to handle binary packages

Fixed both cases (for installing from a local file as well as from a remote site) when no ports tree is present.
Pull Request #256: https://github.com/opscode/chef/pull/256

I have servers where there is no ports tree installed. This fix enables the package provider to:

1. install packages that are located in the filesystem (the version is parsed from the package name)
2. install packages that are at a PACKAGESITE or PACKAGEROOT (the version is irrelevant here, if not specified in the recipe, as FreeBSD binary package repositories take care of this through symlinks)

N.B. I'm aware of btm's comment to CHEF-1413 regarding PACKAGESITE vs. PACKAGEROOT; this is the patch that will parse the :source and do the right thing.

:source "ftp://ftp3.FreeBSD.org" is handled as a PACKAGEROOT, i.e. 'pkg_add -r' uses automatic directory logic to build the full URL
:source "ftp://ftp3.FreeBSD.org/" is handled as a PACKAGESITE, and "#{package_name}.tbz" is tacked on to the end of this URL

Dimitri,

We'd like to avoid using 'rescue' as a conditional. Can't we look for the ports tree using File.exists? against port_path instead of running a command that would fail?

In either case, it isn't clear what you're rescuing. It would be a better practice if we rescued the most specific exception that we could.

Hi Bryan,

You're absolutely right. I've removed the 'rescue' conditionals and replaced them with something more reasonable.

The pull request has also been updated.

Thanks,

• Dimitri

Just to be clear: the pull request should reflect commit 09883fc.

Alright, we see the fixes in 3c1c6dbdc6732496df33cd43f30b67728f30f586. It looks like the pull request here has a merge in it, so we'll have to cherry pick out the relevant commits.

Marked to be merged, thanks!

Hi Bryan,

Sorry about all the crap that came in with the update to the pull request - I tried to do an update from master and then a rebase, as per the instructions on the wiki, so there wouldn't be so much to merge and cherry picking wouldn't be necessary. I guess it didn't quite work out the way it was meant to...

To make this ticket complete, you'll need the following commits:

3c1c6dbdc6732496df33cd43f30b67728f30f586
542eeb1ab5e70a6c8c96d78b26958a65d82144ac
09883fc18e578b32d9c3c9632ab995e63a8ba3ac

3c1c6db alone won't do it, due to the use of port_path, which bails when there is no ports tree installed.
542eeb1 has got the hardcoded existence check, to see if a ports tree is installed
09883fc is a nice-to-have to fix the case where you have multiple package versions under your :source; it picks the latest

I've created a new branch that is rebased against master and contains the five commits here:

https://github.com/btm/chef/tree/CHEF-1251

However, there are some failing tests. Can you take this branch and look at the failing tests to ensure we aren't causing a regression?

Failures:

  1) Chef::Provider::Package::Freebsd load_current_resource when determining the current package state should return a version if the package is installed
     Failure/Error: @provider.load_current_resource
     Chef::Exceptions::Package:
       Couldn't find a candidate version.
     # ./spec/../lib/chef/provider/package/freebsd.rb:93:in `load_current_resource'
     # ./spec/unit/provider/package/freebsd_spec.rb:46

  2) Chef::Provider::Package::Freebsd load_current_resource when determining the current package state should return nil if the package is not installed
     Failure/Error: @provider.load_current_resource
     Chef::Exceptions::Package:
       Couldn't find a candidate version.
     # ./spec/../lib/chef/provider/package/freebsd.rb:93:in `load_current_resource'
     # ./spec/unit/provider/package/freebsd_spec.rb:52

  3) Chef::Provider::Package::Freebsd load_current_resource when determining the current package state should return a candidate version if it exists
     Failure/Error: @provider.load_current_resource
     Chef::Exceptions::Package:
       Couldn't find a candidate version.
     # ./spec/../lib/chef/provider/package/freebsd.rb:93:in `load_current_resource'
     # ./spec/unit/provider/package/freebsd_spec.rb:58

  4) Chef::Provider::Package::Freebsd load_current_resource when querying for package state and attributes should figure out the package name
     Failure/Error: @provider.should_receive(:shell_out!).with("make -V PKGNAME", {:cwd=>"/usr/ports/shells/zsh", :env=>nil, :returns=>[0, 1]}).and_return(make_v)
       (#<Chef::Provider::Package::Freebsd:0x7ffac5d32f30>).shell_out!("make -V PKGNAME", {:returns=>[0, 1], :cwd=>"/usr/ports/shells/zsh", :env=>nil})
           expected: 1 time
           received: 0 times
     # ./spec/unit/provider/package/freebsd_spec.rb:110

Finished in 26.76 seconds
4337 examples, 4 failures, 4 pending

Have you had a chance to look at this Dimitri?

Hi Bryan,

No, I'm sorry I haven't had a chance to look at this yet. I don't know how the specs are written, but it looks to me as if an installed ports tree is assumed (especially the output of number 4 above).

I should be able to look at this tomorrow and give you some qualified feedback.

OK, I've got three of those tests passing now, with the following fix:

https://github.com/aglarond/chef/commit/97bf957d096899ab42f74e1fc7509eadba9b9d26

Thanks for catching that regression. I'll use the specs from now on before closing a ticket. Maybe there should be an additional section under http://wiki.opscode.com/display/chef/How+to+Contribute, indicating that we should run these specs first.

The test "load_current_resource when querying for package state and attributes should figure out the package name" is dependent on an installed ports tree and will fail in this use case. The comment in the spec file seems to acknowledge this:

#Not happy with the form of these tests as they are far too closely tied to the implementation and so very fragile.

With a ports tree installed, this test also passes.

Let me know if I should create a new pull request for this, or just how you'd like to handle the commit.

Reresolving due to updated patch

Merged to master, thanks Dimitri