Improve selection of required packages (bnc#895070, bnc#896178)

src/modules/Packages.rb

@@ -16,6 +16,12 @@ class PackagesClass < Module
 
     # All known types of resolvables
     RESOLVABLE_TYPES = [:product, :patch, :package, :pattern, :language]
+    # Minimum set of packages required to enable VNC server
+    VNC_BASE_PACKAGES = ["xorg-x11", "xorg-x11-Xvnc", "xorg-x11-fonts", "xinetd"]
+    # Default window manager for VNC if none is installed
+    DEFAULT_WM = "icewm"
+    # Minimum set of packages required for installation with remote X11 server
+    REMOTE_X11_BASE_PACKAGES = [ "xorg-x11-server", "xorg-x11-fonts", "icewm" ]
 
     def main
       Yast.import "UI"
@@ -937,17 +943,9 @@ def graphicPackages
     def modePackages
       packages = []
 
-      if Linuxrc.vnc
-        packages.concat [ "xorg-x11-Xvnc", "xorg-x11-fonts", "icewm", "xinetd" ]
-        packages << "yast2-x11" if Mode.autoinst
-      end
-
+      packages.concat(vnc_packages) if Linuxrc.vnc
       #this means we have a remote X server
-      if Linuxrc.display_ip
-        packages.concat [ "xorg-x11-server", "xorg-x11-fonts", "icewm" ]
-        packages << "yast2-x11" if Mode.autoinst
-      end
-
+      packages.concat(remote_x11_packages) if Linuxrc.display_ip
       packages << "sbl" if Linuxrc.braille
       packages << "openssh" if Linuxrc.usessh
 
@@ -2548,6 +2546,29 @@ def log_software_selection
       nil
     end
 
+    # List of packages expected to be installed in order to enable
+    # remote administration (VNC)
+    #
+    # @return Array<String>
+    def vnc_packages
+      packages = VNC_BASE_PACKAGES.dup
+      # At least one windowmanager must be installed (#427044)
+      # If none is there, use a fallback
+      packages << DEFAULT_WM unless has_window_manager?
+      packages << "yast2-x11" if Mode.autoinst
+      packages
+    end
+
+    # List of packages expected to be installed in order to use
+    # a remote X11 server
+    #
+    # @return Array<String>
+    def remote_x11_packages
+      packages = REMOTE_X11_BASE_PACKAGES.dup
+      packages << "yast2-x11" if Mode.autoinst
+      packages
+    end
+
   private
 
     # Reads product feature defined by parameters, logs what it gets
@@ -2639,6 +2660,8 @@ def report_missing_pattern(pattern_name)
     publish :function => :SelectKernelPackages, :type => "void ()"
     publish :function => :default_patterns, :type => "list <string> ()"
     publish :function => :log_software_selection, :type => "void ()"
+    publish :function => :vnc_packages, :type => "list <string> ()"
+    publish :function => :remote_x11_packages, :type => "list <string> ()"
 
     private
 
@@ -2676,6 +2699,13 @@ def products_to_update(installed_products, removed_products)
     def kept_products(products)
       products.select { |product| product["status"] == :installed }
     end
+
+    # Checks if a window manager is installed or selected for installation
+    #
+    # @return [Boolean] true if there is a window manager
+    def has_window_manager?
+      Pkg.IsSelected("windowmanager") || Pkg.IsProvided("windowmanager")
+    end
   end
 
   Packages = PackagesClass.new

test/packages_test.rb

@@ -10,6 +10,8 @@
 Yast.import "SCR"
 Yast.import "Product"
 Yast.import "ProductFeatures"
+Yast.import "Linuxrc"
+Yast.import "Pkg"
 
 SCR_STRING_PATH = Yast::Path.new(".target.string")
 SCR_BASH_PATH = Yast::Path.new(".target.bash")
@@ -378,4 +380,152 @@ def pattern(properties = {})
     end
   end
 
+  describe "#vnc_packages" do
+    let(:packages) { Yast::Packages.vnc_packages.sort.uniq }
+    let(:base_packages) { ["xinetd", "xorg-x11", "xorg-x11-Xvnc", "xorg-x11-fonts"] }
+    let(:base_packages_and_wm) { ["icewm"] + base_packages }
+
+    context "during installation" do
+      before do
+        allow(Yast::Pkg).to receive(:IsProvided).and_return false
+        allow(Yast::Mode).to receive(:mode).and_return "installation"
+      end
+
+      context "with window manager already selected" do
+        before do
+          allow(Yast::Pkg).to receive(:IsSelected).and_return true
+        end
+
+        it "includes xinetd and xorg" do
+          expect(packages).to eq(base_packages)
+        end
+      end
+
+      context "without window manager selected" do
+        before do
+          allow(Yast::Pkg).to receive(:IsSelected).and_return false
+        end
+
+        it "includes xinetd, xorg and icewm" do
+          expect(packages).to eq(base_packages_and_wm)
+        end
+      end
+    end
+
+    context "during autoinstallation" do
+      before do
+        allow(Yast::Pkg).to receive(:IsProvided).and_return false
+        allow(Yast::Mode).to receive(:mode).and_return "autoinstallation"
+      end
+
+      context "with window manager already selected" do
+        before do
+          allow(Yast::Pkg).to receive(:IsSelected).and_return true
+        end
+
+        it "includes xinetd, xorg and yast2-x11" do
+          expect(packages).to eq(base_packages + ["yast2-x11"])
+        end
+      end
+
+      context "without window manager selected" do
+        before do
+          allow(Yast::Pkg).to receive(:IsSelected).and_return false
+        end
+
+        it "includes xinetd, xorg and icewm" do
+          expect(packages).to eq(base_packages_and_wm + ["yast2-x11"])
+        end
+      end
+    end
+
+    context "in normal mode" do
+      before do
+        allow(Yast::Pkg).to receive(:IsSelected).and_return false
+        allow(Yast::Mode).to receive(:mode).and_return "normal"
+      end
+
+      context "with window manager already installed" do
+        before do
+          allow(Yast::Pkg).to receive(:IsProvided).and_return true
+        end
+
+        it "includes xinetd and xorg" do
+          expect(packages).to eq(base_packages)
+        end
+      end
+
+      context "without window manager installed" do
+        before do
+          allow(Yast::Pkg).to receive(:IsProvided).and_return false
+        end
+
+        it "includes xinetd, xorg and icewm" do
+          expect(packages).to eq(base_packages_and_wm)
+        end
+      end
+    end
+  end
+
+  describe "#modePackages" do
+    before do
+      allow(Yast::Linuxrc).to receive(:vnc).and_return vnc
+      allow(Yast::Linuxrc).to receive(:display_ip).and_return display_ip
+      allow(Yast::Linuxrc).to receive(:braille).and_return braille
+      allow(Yast::Linuxrc).to receive(:usessh).and_return usessh
+      allow(Yast::Mode).to receive(:mode).and_return mode
+    end
+    let(:packages) { Yast::Packages.modePackages.sort.uniq }
+
+    context "on a boring local regular installation" do
+      let(:vnc) { false }
+      let(:display_ip) { false }
+      let(:braille) { false }
+      let(:usessh) { false }
+      let(:mode) { "installation" }
+
+      it "returns an empty array" do
+        expect(packages).to be_empty
+      end
+    end
+
+    context "over ssh with a remote X server" do
+      let(:vnc) { false }
+      let(:display_ip) { true }
+      let(:braille) { false }
+      let(:usessh) { true }
+      let(:xorg_icewm_and_ssh) {
+        ["icewm", "openssh", "xorg-x11-fonts", "xorg-x11-server"]
+      }
+
+      context "during installation" do
+        let(:mode) { "installation" }
+
+        it "includes xorg, icewm and openssh" do
+          expect(packages).to eq(xorg_icewm_and_ssh)
+        end
+      end
+
+      context "during autoinstallation" do
+        let(:mode) { "autoinstallation" }
+
+        it "includes xorg, icewm, openssh and yast2-x11" do
+          expect(packages).to eq(xorg_icewm_and_ssh + ["yast2-x11"])
+        end
+      end
+    end
+
+    context "on vnc installation" do
+      let(:vnc) { true }
+      let(:display_ip) { false }
+      let(:braille) { false }
+      let(:usessh) { false }
+      let(:mode) { "installation" }
+
+      it "relies on #vnc_packages" do
+        expect(Yast::Packages).to receive(:vnc_packages).and_return %w(five names)
+        expect(packages).to eq(%w(five names))
+      end
+    end
+  end
 end