Merge 5d4742d into f309444

package/yast2-packager.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Mon Jun  4 11:42:42 UTC 2018 - lslezak@suse.cz
+
+- Fixed installing packages needed for accessing the installation
+  repositories (e.g. "nfs-client" for "nfs://" repositories),
+  additionally evaluate also the add-on repositories, not just the
+  base product repository (bsc#1063980)
+- 3.2.27
+
 -------------------------------------------------------------------
 Fri Apr 13 08:26:41 UTC 2018 - gsouza@suse.com
 

package/yast2-packager.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-packager
-Version:        3.2.26
+Version:        3.2.27
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/modules/Packages.rb

@@ -9,6 +9,8 @@
 # html_escape()
 require "erb"
 require "fileutils"
+require "uri"
+require "cgi"
 
 module Yast
   class PackagesClass < Module
@@ -1005,27 +1007,57 @@ def boardPackages
       deep_copy(packages)
     end
 
-
-    # Compute packages required to access the repository
+    # Compute packages required to access the repositories
     # @return [Array](string) list of the required packages
     def sourceAccessPackages
-      # TODO: rather check all registered repositories...
-      ret = []
+      packages = []
+      schemes = repo_schemes
 
-      instmode = Linuxrc.InstallInf("InstMode")
-      Builtins.y2milestone("Installation mode: %1", instmode)
+      # /sbin/mount.cifs is required to mount a SMB/CIFS share
+      packages << "cifs-mount" if schemes.include?("smb") || schemes.include?("cifs")
 
-      if instmode == "smb" || instmode == "cifs"
-        # /sbin/mount.cifs is required to mount a SMB/CIFS share
-        ret = ["cifs-mount"]
-      elsif instmode == "nfs"
-        # portmap is required to mount an NFS export
-        ret = ["nfs-client"]
-      end
+      # portmap is required to mount an NFS export
+      packages << "nfs-client" if schemes.include?("nfs")
+
+      log.info("Packages for accessing the repositories: #{packages.inspect}")
+      packages
+    end
 
-      Builtins.y2milestone("Packages for accessing the repository: %1", ret)
+    # Return the URL schemes for the currently defined repositories (only enabled
+    # repositories are evaluated). For ISO repositories the base URL schema
+    # is returned (i.e. the location of the ISO), invalid URLs are ignored.
+    # @return [Array<String>] the list of the URL schemes, empty if
+    #   no repository is defined
+    def repo_schemes
+      schemes = []
+
+      # all enabled repositories
+      Pkg.SourceGetCurrent(true).each do |repo|
+        url = Pkg.SourceURL(repo)
+
+        begin
+          uri = URI(url)
+
+          # handle the ISO scheme specifically
+          # Note: scheme is converted to lowercase by Ruby
+          if uri.scheme == "iso" && uri.query
+            # parse the query string into a hash, extract the base URL from the query
+            # CGI.parse output is a hash: key => [val1, val2, ...]
+            params = CGI.parse(uri.query)
+            # expect only one "url" parameter
+            scheme = URI(params["url"].first).scheme if params["url"] && !params["url"].empty?
+          else
+            scheme = uri.scheme
+          end
 
-      deep_copy(ret)
+          schemes << scheme if scheme && !schemes.include?(scheme)
+        # normally should not happen, the URLs from libzypp should be always valid
+        rescue URI::InvalidURIError => e
+          log.error(e.message)
+        end
+      end
+
+      schemes
     end
 
     # Additional kernel packages from control file

test/packages_test.rb

@@ -36,7 +36,8 @@ def load_zypp(file_name)
 
 PRODUCTS_FROM_ZYPP = load_zypp('products.yml').freeze
 
-describe Yast::Packages do
+describe "Yast::Packages" do
+  subject { Yast::Packages }
   before(:each) do
     log.info "--- test ---"
   end
@@ -1210,4 +1211,118 @@ def product(properties = {})
       expect{URI.parse(@base_url.value)}.to_not raise_error
     end
   end
+
+  # helper for the #repo_schemes tests to mock the repository configuration
+  def expect_source_urls(mapping)
+    expect(Yast::Pkg).to receive(:SourceGetCurrent).with(true).and_return(mapping.keys)
+
+    mapping.each do |id, url|
+      expect(Yast::Pkg).to receive(:SourceURL).with(id).and_return(url)
+    end
+  end
+
+  describe "#repo_schemes" do
+    it "returns empty list if no repository is defined" do
+      expect_source_urls({})
+      expect(subject.repo_schemes).to eq([])
+    end
+
+    it "returns all used schemes" do
+      expect_source_urls(
+        0 => "http://example.com",
+        1 => "https://example.com",
+        2 => "ftp://example.com",
+        3 => "dir:///packages",
+        4 => "dvd:///"
+      )
+      expect(subject.repo_schemes).to eq(["http", "https", "ftp", "dir", "dvd"])
+    end
+
+    it "returns unique list" do
+      expect_source_urls(
+        0 => "http://example.com",
+        1 => "http://example2.com",
+        7 => "ftp://example.com",
+        8 => "ftp://example2.com"
+      )
+      expect(subject.repo_schemes).to eq(["http", "ftp"])
+    end
+
+    it "returns the scheme of the base URL for ISO scheme" do
+      expect_source_urls(
+        # ISO over NFS, see "man zypper"
+        0 => "iso:/subdir?iso=DVD1.iso&url=nfs://server/dir&mnt=/nfs&filesystem=udf"
+      )
+      expect(subject.repo_schemes).to eq(["nfs"])
+    end
+
+    it "converts the scheme names to lower case" do
+      expect_source_urls(
+        0 => "HTTP://example.com",
+        8 => "FTP://example2.com"
+      )
+      expect(subject.repo_schemes).to eq(["http", "ftp"])
+    end
+
+    it "ignores invalid URL" do
+      expect_source_urls(0 => ":")
+      expect(subject.repo_schemes).to eq([])
+    end
+
+    it "ignores incomplete ISO URL (missing 'url' parameter)" do
+      expect_source_urls(0 => "iso:/subdir?iso=DVD1.iso&mnt=/nfs&filesystem=udf")
+      expect(subject.repo_schemes).to eq([])
+    end
+
+    it "ignores incomplete ISO URL (empty 'url' parameter)" do
+      expect_source_urls(0 => "iso:/subdir?iso=DVD1.iso&url=&mnt=/nfs&filesystem=udf")
+      expect(subject.repo_schemes).to eq([])
+    end
+
+    it "ignores invalid ISO URL (invalid 'url' parameter)" do
+      expect_source_urls(
+        0 => "iso:/subdir?iso=DVD1.iso&url=:&filesystem=udf"
+      )
+      expect(subject.repo_schemes).to eq([])
+    end
+  end
+
+  describe "#sourceAccessPackages" do
+    it "returns empty list if no repository is defined" do
+      expect(subject).to receive(:repo_schemes).and_return([])
+      expect(subject.sourceAccessPackages).to eq([])
+    end
+
+    # these do not need any extra package to access them
+    it "returns empty list for http(s), ftp, hd, cd, dvd and dir schemes" do
+      schemes = ["http", "https", "ftp", "hd", "cd", "dvd", "dir"]
+      expect(subject).to receive(:repo_schemes).and_return(schemes)
+      expect(subject.sourceAccessPackages).to eq([])
+    end
+
+    it "returns 'nfs-client' for nfs scheme" do
+      schemes = ["nfs"]
+      expect(subject).to receive(:repo_schemes).and_return(schemes)
+      expect(subject.sourceAccessPackages).to eq(["nfs-client"])
+    end
+
+    it "returns 'cifs-mount' for smb scheme" do
+      schemes = ["smb"]
+      expect(subject).to receive(:repo_schemes).and_return(schemes)
+      expect(subject.sourceAccessPackages).to eq(["cifs-mount"])
+    end
+
+    it "returns 'cifs-mount' for cifs scheme" do
+      schemes = ["cifs"]
+      expect(subject).to receive(:repo_schemes).and_return(schemes)
+      expect(subject.sourceAccessPackages).to eq(["cifs-mount"])
+    end
+
+    it "returns 'cifs-mount' and 'nfs-client' for smb and nfs schemes" do
+      schemes = ["cifs", "nfs"]
+      expect(subject).to receive(:repo_schemes).and_return(schemes)
+      # sort the result to make it order independent
+      expect(subject.sourceAccessPackages.sort).to eq(["cifs-mount", "nfs-client"])
+    end
+  end
 end