Merge pull request #420 from yast/twice_url_fix

Twice url fix

package/yast2-installation.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Aug 17 15:02:02 UTC 2016 - jreidinger@suse.com
+
+- filter out same repositories from extraurls if they differ only
+  in trailing slash (bnc#970488)
+- 3.1.210
+
 -------------------------------------------------------------------
 Tue Aug 16 15:34:43 UTC 2016 - kanderssen@suse.com
 

package/yast2-installation.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-installation
-Version:        3.1.208
+Version:        3.1.210
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/lib/installation/clients/inst_extrasources.rb

@@ -19,29 +19,28 @@
 # current contact information at www.novell.com.
 # ------------------------------------------------------------------------------
 
+Yast.import "UI"
+Yast.import "Pkg"
+Yast.import "GetInstArgs"
+Yast.import "Mode"
+Yast.import "PackageLock"
+Yast.import "ProductFeatures"
+# We need the constructor
+Yast.import "ProductControl"
+Yast.import "Installation"
+Yast.import "Icon"
+Yast.import "NetworkService"
+Yast.import "PackagesUI"
+Yast.import "Label"
+
 module Yast
   # This client loads the target and initializes the package manager.
   # Adds all sources defined in control file (software->extra_urls)
   # and stores them at the end.
   class InstExtrasourcesClient < Client
     def main
-      Yast.import "UI"
-      Yast.import "Pkg"
-
       textdomain "installation"
 
-      Yast.import "GetInstArgs"
-      Yast.import "Mode"
-      Yast.import "PackageLock"
-      Yast.import "ProductFeatures"
-      # We need the constructor
-      Yast.import "ProductControl"
-      Yast.import "Installation"
-      Yast.import "Icon"
-      Yast.import "NetworkService"
-      Yast.import "PackagesUI"
-      Yast.import "Label"
-
       # local sources that have been attached under /mnt during upgrade
       @local_urls = {}
 
@@ -213,12 +212,7 @@ def main
     # @param [Array<String>] registered URLs of already registered repositories (they will be ignored to not register the same repository one more)
     # @return [Array<Hash>] of URLs to register
     def GetURLsToRegister(registered)
-      registered = deep_copy(registered)
-      urls_from_control_file = Convert.convert(
-        ProductFeatures.GetFeature("software", "extra_urls"),
-        from: "any",
-        to:   "list <map>"
-      )
+      urls_from_control_file = ProductFeatures.GetFeature("software", "extra_urls")
 
       if urls_from_control_file.nil?
         Builtins.y2milestone(
@@ -229,7 +223,9 @@ def GetURLsToRegister(registered)
       end
 
       urls_from_control_file = Builtins.filter(urls_from_control_file) do |one_url|
-        if Builtins.contains(registered, Ops.get_string(one_url, "baseurl", ""))
+        url = one_url["baseurl"] || ""
+        normalized_url = url.end_with?("/") ? url.chop : url
+        if registered.include?(normalized_url)
           Builtins.y2milestone(
             "Already registered: %1",
             Ops.get_string(one_url, "baseurl", "")
@@ -337,29 +333,26 @@ def RegisterRepos(url_list)
 
     # Returns list of already registered repositories.
     #
-    # @return [Array<String>] of registered repositories
+    # @return [Array<String>] of registered repositories without trailing '/'
     def RegisteredUrls
       # get all registered installation sources
       srcs = Pkg.SourceGetCurrent(false)
 
-      ret = []
-      Builtins.foreach(srcs) do |src|
+      result = srcs.each_with_object([]) do |src, ret|
         general = Pkg.SourceGeneralData(src)
-        url = Ops.get_string(general, "url", "")
-        ret = Builtins.add(ret, url) if !url.nil? && url != ""
-        if Mode.update && Builtins.regexpmatch(url, "^dir:[/]+mnt[/]+")
-          Ops.set(@local_urls, src, url)
-        end
+        url = general["url"]
+        next if url.nil? || url.empty?
+        @local_urls[src] = url if Mode.update && url =~ /^dir:\/+mnt\/+/
         # check for USB sources which should be disabled
-        if Builtins.issubstring(url, "device=/dev/disk/by-id/usb-")
-          Ops.set(@usb_sources, src, url)
-        end
+        @usb_sources[src] = url if url.include?("device=/dev/disk/by-id/usb-")
+        url.chop! if url.end_with?("/") # remove trailing slash to normalize path (see bnc#970488)
+        ret << url
       end
 
       # remove duplicates
-      ret = Builtins.toset(ret)
+      result.uniq!
 
-      Builtins.y2milestone("Registered sources: %1", ret)
+      Builtins.y2milestone("Registered sources: %1", result)
 
       Builtins.y2milestone(
         "Registered local sources under /mnt: %1",
@@ -368,7 +361,7 @@ def RegisteredUrls
 
       Builtins.y2milestone("Registered USB sources: %1", @usb_sources)
 
-      deep_copy(ret)
+      result
     end
 
     # Initialize the package manager

test/Makefile.am

@@ -1,4 +1,5 @@
 TESTS = \
+  inst_extrasources_test.rb \
   inst_functions_test.rb \
   inst_finish_test.rb \
   inst_instsys_cleanup_client_test.rb \

test/inst_extrasources_test.rb

@@ -0,0 +1,56 @@
+#! /usr/bin/env rspec
+
+require_relative "test_helper"
+
+require "installation/clients/inst_extrasources"
+
+describe Yast::InstExtrasourcesClient do
+  describe "#RegisteredUrls" do
+    before do
+      # fake main run, to avoid huge stubbing
+      subject.instance_variable_set(:"@local_urls", {})
+      subject.instance_variable_set(:"@usb_sources", {})
+
+      allow(Yast::Pkg).to receive(:SourceGetCurrent).with(false).and_return([0, 1, 2, 3])
+      allow(Yast::Pkg).to receive(:SourceGeneralData).with(0).and_return("url" => "http://test.com/")
+      allow(Yast::Pkg).to receive(:SourceGeneralData).with(1)
+        .and_return("url" => "usb://device=/dev/disk/by-id/usb-15")
+      allow(Yast::Pkg).to receive(:SourceGeneralData).with(2).and_return("url" => "dir:///mnt/path")
+      allow(Yast::Pkg).to receive(:SourceGeneralData).with(3).and_return({})
+    end
+
+    it "returns list of urls for registered repositories without trailing slash" do
+      expect(subject.RegisteredUrls).to eq [
+        "http://test.com", "usb://device=/dev/disk/by-id/usb-15", "dir:///mnt/path"
+      ]
+    end
+
+    it "fills list of local_urls in update Mode" do
+      allow(Yast::Mode).to receive(:update).and_return(true)
+      subject.RegisteredUrls
+
+      expect(subject.instance_variable_get(:"@local_urls")).to eq(2 => "dir:///mnt/path")
+    end
+
+    it "fills list of usb sources" do
+      subject.RegisteredUrls
+
+      expect(subject.instance_variable_get(:"@usb_sources")).to eq(
+        1 => "usb://device=/dev/disk/by-id/usb-15"
+      )
+    end
+  end
+
+  describe "#GetURLsToRegister" do
+    it "returns extra_urls entries from product " \
+       "without already registered entries passed as argument" do
+      already_registered = "http://test.com"
+      allow(Yast::ProductFeatures).to receive(:GetFeature).with("software", "extra_urls")
+        .and_return([{ "baseurl" => "http://test.com/" }, { "baseurl" => "http://test2.com" }])
+
+      expect(subject.GetURLsToRegister(already_registered)).to eq(
+        [{ "baseurl" => "http://test2.com" }]
+      )
+    end
+  end
+end