diff options
author | Tim Smith <tsmith@chef.io> | 2017-11-03 19:02:23 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-11-03 19:02:23 -0700 |
commit | 97e895278bdbc7517f4634c4b83c36592ba94e5a (patch) | |
tree | 5c94363c0bb01b2b06abc20cf5b6055e0ed5f913 | |
parent | f81cee42f95155ea371f14bc5f1d34f5e223357b (diff) | |
parent | 190e17dbd1e9ee2b21f9acd60844a5661f80a329 (diff) | |
download | chef-97e895278bdbc7517f4634c4b83c36592ba94e5a.tar.gz |
Merge pull request #6558 from chef/fix_pkgs
Fix the invalid version comparison in apt/dpkg providers
-rw-r--r-- | lib/chef/provider/package.rb | 3 | ||||
-rw-r--r-- | lib/chef/provider/package/apt.rb | 17 | ||||
-rw-r--r-- | lib/chef/provider/package/dpkg.rb | 16 | ||||
-rw-r--r-- | spec/unit/provider/package/apt_spec.rb | 34 |
4 files changed, 66 insertions, 4 deletions
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb index 95d16dd666..7116bc9045 100644 --- a/lib/chef/provider/package.rb +++ b/lib/chef/provider/package.rb @@ -477,6 +477,9 @@ class Chef elsif candidate_version.nil? Chef::Log.debug("#{new_resource} #{package_name} has no candidate_version to upgrade to") target_version_array.push(nil) + elsif current_version.nil? + Chef::Log.debug("#{new_resource} has no existing installed version. Installing install #{candidate_version}") + target_version_array.push(candidate_version) elsif version_compare(current_version, candidate_version) == 1 && !new_resource.allow_downgrade Chef::Log.debug("#{new_resource} #{package_name} has installed version #{current_version}, which is newer than available version #{candidate_version}. Skipping...)") target_version_array.push(nil) diff --git a/lib/chef/provider/package/apt.rb b/lib/chef/provider/package/apt.rb index da86016621..3f32f9d380 100644 --- a/lib/chef/provider/package/apt.rb +++ b/lib/chef/provider/package/apt.rb @@ -127,11 +127,20 @@ class Chef private + # compare 2 versions to each other to see which is newer. + # this differs from the standard package method because we + # need to be able to parse debian version strings which contain + # tildes which Gem cannot properly parse + # + # @return [Integer] 1 if v1 > v2. 0 if they're equal. -1 if v1 < v2 def version_compare(v1, v2) - gem_v1 = v1.gsub(/[_+]/, "+" => "-", "_" => "-") unless v1.nil? - gem_v2 = v2.gsub(/[_+]/, "+" => "-", "_" => "-") unless v2.nil? - - Gem::Version.new(gem_v1) <=> Gem::Version.new(gem_v2) + if !shell_out_compact_timeout("dpkg", "--compare-versions", v1.to_s, "gt", v2.to_s).error? + 1 + elsif !shell_out_compact_timeout("dpkg", "--compare-versions", v1.to_s, "eq", v2.to_s).error? + 0 + else + -1 + end end # Runs command via shell_out with magic environment to disable diff --git a/lib/chef/provider/package/dpkg.rb b/lib/chef/provider/package/dpkg.rb index 89a57affac..cf92e6d3e7 100644 --- a/lib/chef/provider/package/dpkg.rb +++ b/lib/chef/provider/package/dpkg.rb @@ -106,6 +106,22 @@ class Chef private + # compare 2 versions to each other to see which is newer. + # this differs from the standard package method because we + # need to be able to parse debian version strings which contain + # tildes which Gem cannot properly parse + # + # @return [Integer] 1 if v1 > v2. 0 if they're equal. -1 if v1 < v2 + def version_compare(v1, v2) + if !shell_out_compact_timeout("dpkg", "--compare-versions", v1.to_s, "gt", v2.to_s).error? + 1 + elsif !shell_out_compact_timeout("dpkg", "--compare-versions", v1.to_s, "eq", v2.to_s).error? + 0 + else + -1 + end + end + def read_current_version_of_package(package_name) Chef::Log.debug("#{new_resource} checking install state of #{package_name}") status = shell_out_compact_timeout!("dpkg", "-s", package_name, returns: [0, 1]) diff --git a/spec/unit/provider/package/apt_spec.rb b/spec/unit/provider/package/apt_spec.rb index 46ae7fcadc..ff14edcffd 100644 --- a/spec/unit/provider/package/apt_spec.rb +++ b/spec/unit/provider/package/apt_spec.rb @@ -475,6 +475,40 @@ mpg123 1.12.1-0ubuntu1 end end + describe "#action_install" do + it "should run dpkg to compare versions if an existing version is installed" do + allow(@provider).to receive(:get_current_versions).and_return("1.4.0") + allow(@new_resource).to receive(:allow_downgrade).and_return(false) + expect(@provider).to receive(:shell_out_compact_timeout).with( + "dpkg", "--compare-versions", "1.4.0", "gt", "0.8.12-7" + ).and_return(double(error?: false)) + @provider.run_action(:upgrade) + end + + it "should install the package if the installed version is older" do + allow(@provider).to receive(:get_current_versions).and_return("0.4.0") + allow(@new_resource).to receive(:allow_downgrade).and_return(false) + expect(@provider).to receive(:version_compare).and_return(-1) + expect(@provider).to receive(:shell_out!).with( + "apt-get", "-q", "-y", "install", "irssi=0.8.12-7", + :env => { "DEBIAN_FRONTEND" => "noninteractive" }, + :timeout => @timeout + ) + @provider.run_action(:upgrade) + end + + it "should not compare versions if an existing version is not installed" do + allow(@provider).to receive(:get_current_versions).and_return(nil) + allow(@new_resource).to receive(:allow_downgrade).and_return(false) + expect(@provider).not_to receive(:version_compare) + expect(@provider).to receive(:shell_out!).with( + "apt-get", "-q", "-y", "install", "irssi=0.8.12-7", + :env => { "DEBIAN_FRONTEND" => "noninteractive" }, + :timeout => @timeout + ) + @provider.run_action(:upgrade) + end + end end end end |