Preserve the old clobbering behavior for backwards compatibility, and put ALL packages in the list

Signed-off-by: Joshua Justice <jjustice6@bloomberg.net>

2 files changed, 38 insertions, 13 deletions

diff --git a/lib/ohai/plugins/packages.rb b/lib/ohai/plugins/packages.rb
index 79ac9494..a748913a 100644
--- a/lib/ohai/plugins/packages.rb
+++ b/lib/ohai/plugins/packages.rb
@@ -45,16 +45,35 @@ Ohai.plugin(:Packages) do
       format = '%{NAME}\t%|EPOCH?{%{EPOCH}}:{0}|\t%{VERSION}\t%{RELEASE}\t%{INSTALLTIME}\t%{ARCH}\n'
       so = shell_out("rpm -qa --qf '#{format}'")
       pkgs = so.stdout.lines
-
+      
       pkgs.each do |pkg|
         name, epoch, version, release, installdate, arch = pkg.split
         if packages[name]
+          puts "we are in the special block with #{packages[name]}"
           # We have more than one package with this exact name!
-          # Create an "other_versions" array for tracking all versions of packages with this name.
+          # Create an "versions" array for tracking all versions of packages with this name.
           # The top-level package information will be the first one returned by rpm -qa,
-          # all others go in this list, with the same information they'd normally have.
-          packages[name]["other_versions"] ||= []
-          packages[name]["other_versions"] << Mash.new({ "epoch" => epoch, "version" => version, "release" => release, "installdate" => installdate, "arch" => arch })
+          # all versions go in this list, with the same information they'd normally have.
+          if packages[name]["versions"].nil?
+            # add the data of the first package to the list, so that all versions are in the list.
+            packages[name]["versions"] = []
+            packages[name]["versions"] << Mash.new({ "epoch" => packages[name]["epoch"],
+                                                     "version" => packages[name]["version"],
+                                                     "release" => packages[name]["release"],
+                                                     "installdate" => packages[name]["installdate"],
+                                                     "arch" => packages[name]["arch"] })
+          end
+          packages[name]["versions"] << Mash.new({ "epoch" => epoch, "version" => version, "release" => release, "installdate" => installdate, "arch" => arch }) # Add this package version to the list
+          # When this was originally written, it didn't account for multiple versions of the same package
+          # so it just kept clobbering the package data if it encountered multiple versions
+          # of the same package. As a result, the last duplicate returned by rpm -qa was what was present;
+          # here we clobber that data for compatibility. Note that we can't overwrite the entire hash
+          # without losing the versions array.
+          packages[name]["epoch"] = epoch
+          packages[name]["version"] = version
+          packages[name]["release"] = release
+          packages[name]["installdate"] = installdate
+          packages[name]["arch"] =arch 
         else
           packages[name] = { "epoch" => epoch, "version" => version, "release" => release, "installdate" => installdate, "arch" => arch }
         end
diff --git a/spec/unit/plugins/packages_spec.rb b/spec/unit/plugins/packages_spec.rb
index 60fe7ed9..0cde53b3 100644
--- a/spec/unit/plugins/packages_spec.rb
+++ b/spec/unit/plugins/packages_spec.rb
@@ -103,16 +103,22 @@ describe Ohai::System, "plugin packages" do
 
     it "handles multiple packages with the same name" do
       expect(plugin[:packages]["kernel"][:version]).to eq("3.10.0")
-      expect(plugin[:packages]["kernel"][:release]).to eq("862.2.3.el7")
+      expect(plugin[:packages]["kernel"][:release]).to eq("862.el7")
       expect(plugin[:packages]["kernel"][:epoch]).to eq("0")
-      expect(plugin[:packages]["kernel"][:installdate]).to eq("1526310781")
+      expect(plugin[:packages]["kernel"][:installdate]).to eq("1521745632")
       expect(plugin[:packages]["kernel"][:arch]).to eq("x86_64")
-      # and now the "other" version installed:
-      expect(plugin[:packages]["kernel"]["other_versions"].first[:version]).to eq("3.10.0")
-      expect(plugin[:packages]["kernel"]["other_versions"].first[:release]).to eq("862.el7")
-      expect(plugin[:packages]["kernel"]["other_versions"].first[:epoch]).to eq("0")
-      expect(plugin[:packages]["kernel"]["other_versions"].first[:installdate]).to eq("1521745632")
-      expect(plugin[:packages]["kernel"]["other_versions"].first[:arch]).to eq("x86_64")
+      # and now the version list:
+      expect(plugin[:packages]["kernel"]["versions"].first[:version]).to eq("3.10.0")
+      expect(plugin[:packages]["kernel"]["versions"].first[:release]).to eq("862.2.3.el7")
+      expect(plugin[:packages]["kernel"]["versions"].first[:epoch]).to eq("0")
+      expect(plugin[:packages]["kernel"]["versions"].first[:installdate]).to eq("1526310781")
+      expect(plugin[:packages]["kernel"]["versions"].first[:arch]).to eq("x86_64")
+      expect(plugin[:packages]["kernel"]["versions"].last[:version]).to eq("3.10.0")
+      expect(plugin[:packages]["kernel"]["versions"].last[:release]).to eq("862.el7")
+      expect(plugin[:packages]["kernel"]["versions"].last[:epoch]).to eq("0")
+      expect(plugin[:packages]["kernel"]["versions"].last[:installdate]).to eq("1521745632")
+      expect(plugin[:packages]["kernel"]["versions"].last[:arch]).to eq("x86_64")
+            
     end
   end