Merge 146d6c5 into 89c3c5e

package/autoyast2.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Fri Aug 17 18:11:33 UTC 2018 - knut.anderssen@suse.com
+
+- AutoInstallRules: increased default maxdepth for not crashing
+  with a big software package list (bsc#1104655)
+- 4.0.60
+
 -------------------------------------------------------------------
 Fri Aug 17 09:46:05 CEST 2018 - schubi@suse.de
 

package/autoyast2.spec

@@ -22,7 +22,7 @@
 %endif
 
 Name:           autoyast2
-Version:        4.0.59
+Version:        4.0.60
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/modules/AutoInstallRules.rb

@@ -7,6 +7,7 @@
 #
 # $Id$
 require "yast"
+require "yast2/popup"
 require "y2storage"
 
 module Yast
@@ -874,94 +875,79 @@ def GetRules
       end
     end
 
+    #TODO: Move the responsibility of merging profiles to a specific class
+    # removing also the duplication of code between this module and the
+    # AutoinstClass one.
+
+    MERGE_CMD = "/usr/bin/xsltproc".freeze
+    MERGE_DEFAULTS = "--novalid --maxdepth 10000 --param replace \"'false'\"".freeze
+    MERGE_XSLT_PATH = "/usr/share/autoinstall/xslt/merge.xslt".freeze
+
+    # Merges the given profiles
+    #
+    # @param base_profile [String] the base profile file path
+    # @param with [String] the profile to be merged file path
+    # @param to [String] the resulting control file path
+    # @return [Hash] stdout and stderr output
+    def merge_profiles(base_profile, with, to)
+      dontmerge_str = ""
+      AutoinstConfig.dontmerge.each_with_index do |dm, i|
+        dontmerge_str << " --param dontmerge#{i+1} \"'#{dm}'\""
+      end
+      merge_command =
+        "#{MERGE_CMD} #{MERGE_DEFAULTS}" \
+        "#{dontmerge_str} --param with \"'#{with}'\" " \
+        "--output \"#{to}\" " \
+        "#{MERGE_XSLT_PATH} #{base_profile}"
+
+      out = SCR.Execute(path(".target.bash_output"), merge_command, {})
+      log.info("Merge command: #{merge_command}")
+      log.info("Merge stdout: #{out["stdout"]}, stderr: #{out["stderr"]}")
+      out
+    end
 
     # Merge Rule results
     # @param [String] result_profile the resulting control file path
     # @return [Boolean] true on success
     def Merge(result_profile)
-      tmpdir = AutoinstConfig.tmpDir
+      base_profile = File.join(AutoinstConfig.tmpdir, "base_profile.xml")
+      merge_profile = File.join(AutoinstConfig.tmpdir, "result.xml")
       ok = true
       skip = false
       error = false
-
-      base_profile = Ops.add(tmpdir, "/base_profile.xml")
-
-      Builtins.foreach(@tomerge) do |file|
-        Builtins.y2milestone("Working on file: %1", file)
-        current_profile = Ops.add(
-          Ops.add(AutoinstConfig.local_rules_location, "/"),
-          file
-        )
+      @tomerge.each do |file|
+        log.info("Working on file: %1", file)
+        current_profile = File.join(AutoinstConfig.local_rules_location, file)
         if !skip
-          if !XML_cleanup(current_profile, Ops.add(tmpdir, "/base_profile.xml"))
-            Builtins.y2error("Error reading XML file")
+          if !XML_cleanup(current_profile, base_profile)
+            log.error("Error reading XML file")
             message = _(
               "The XML parser reported an error while parsing the autoyast profile. The error message is:\n"
             )
-            message = Ops.add(message, XML.XMLError)
-            Popup.Error(message)
+            message += XML.XMLError
+            Yast2::Popup.show(message, headline: :error)
             error = true
           end
           skip = true
         elsif !error
-          _MergeCommand = "/usr/bin/xsltproc --novalid --param replace \"'false'\" "
-          dontmerge_str = ""
-          i = 1
-          Builtins.foreach(AutoinstConfig.dontmerge) do |dm|
-            dontmerge_str = Ops.add(
-              dontmerge_str,
-              Builtins.sformat(" --param dontmerge%1 \"'%2'\" ", i, dm)
-            )
-            i = Ops.add(i, 1)
-          end
-          _MergeCommand = Ops.add(_MergeCommand, dontmerge_str)
-
-          _MergeCommand = Ops.add(_MergeCommand, "--param with ")
-          _MergeCommand = Ops.add(
-            Ops.add(Ops.add(_MergeCommand, "\"'"), current_profile),
-            "'\"  "
-          )
-          _MergeCommand = Ops.add(
-            Ops.add(Ops.add(_MergeCommand, "--output "), tmpdir),
-            "/result.xml"
-          )
-          _MergeCommand = Ops.add(
-            _MergeCommand,
-            " /usr/share/autoinstall/xslt/merge.xslt "
-          )
-          _MergeCommand = Ops.add(Ops.add(_MergeCommand, base_profile), " ")
+          xsltret = merge_profiles(base_profile, current_profile, merge_profile)
 
-          Builtins.y2milestone("Merge command: %1", _MergeCommand)
-          xsltret = Convert.to_map(
-            SCR.Execute(path(".target.bash_output"), _MergeCommand)
-          )
-          Builtins.y2milestone("Merge result: %1", xsltret)
-          if Ops.get_integer(xsltret, "exit", -1) != 0 ||
-              Ops.get_string(xsltret, "stderr", "") != ""
-            Builtins.y2error("Merge Failed")
-            StdErrLog(Ops.get_string(xsltret, "stderr", ""))
+          log.info("Merge result: %1", xsltret)
+          if xsltret["exit"] != 0 || xsltret.fetch("stderr", "") != ""
+            log.error("Merge Failed")
+            StdErrLog(xsltret.fetch("stderr", ""))
             ok = false
           end
 
-          XML_cleanup(
-            Ops.add(tmpdir, "/result.xml"),
-            Ops.add(tmpdir, "/base_profile.xml")
-          )
+          XML_cleanup(merge_profile, base_profile)
         else
-          Builtins.y2error("Error while merging control files")
+          log.error("Error while merging control files")
         end
       end
 
       return !error if error
 
-      SCR.Execute(
-        path(".target.bash"),
-        Ops.add(
-          Ops.add(Ops.add("cp ", tmpdir), "/base_profile.xml "),
-          result_profile
-        )
-      )
-
+      SCR.Execute(path(".target.bash"), "cp #{base_profile} #{result_profile}")
       Builtins.y2milestone("Ok=%1", ok)
       @dontmergeIsDefault = true
       AutoinstConfig.dontmerge = deep_copy(@dontmergeBackup)

src/modules/AutoinstClass.rb

@@ -112,6 +112,9 @@ def AutoinstClass
       nil
     end
 
+    MERGE_CMD = "/usr/bin/xsltproc".freeze
+    MERGE_DEFAULTS = "--novalid --maxdepth 10000 --param replace \"'false'\"".freeze
+
     # Merge classes
     #
     def MergeClasses(configuration, base_profile, resultFileName)
@@ -120,7 +123,7 @@ def MergeClasses(configuration, base_profile, resultFileName)
         dontmerge_str << " --param dontmerge#{i+1} \"'#{dm}'\" "
       end
       merge_command =
-        "/usr/bin/xsltproc --novalid --param replace \"'false'\" #{dontmerge_str} --param with " \
+        "#{MERGE_CMD} #{MERGE_DEFAULTS} #{dontmerge_str} --param with " \
         "\"'#{findPath(configuration["name"], configuration["class"])}'\"  " \
         "--output #{File.join(AutoinstConfig.tmpDir, resultFileName)}  " \
         "#{MERGE_XSLT_PATH} #{base_profile} "

test/AutoInstallRules_test.rb

@@ -208,4 +208,75 @@
       end
     end
   end
+
+  describe '#merge_profiles' do
+    let(:base_profile_path) { File.join(FIXTURES_PATH, 'profiles', 'partitions.xml') }
+    let(:tmp_dir) { File.join(root_path, 'tmp') }
+    let(:expected_xml) { File.read(expected_xml_path) }
+    let(:output_path) { File.join(tmp_dir, 'output.xml') }
+    let(:to_merge_path) { File.join(FIXTURES_PATH, 'classes', 'swap', 'largeswap.xml') }
+    let(:output_xml) { File.read(output_path) }
+    let(:dontmerge) { [] }
+    let(:merge_xslt_path) { File.join(root_path, 'xslt', 'merge.xslt') }
+    let(:xsltproc_command) {
+      "/usr/bin/xsltproc --novalid --maxdepth 10000 --param replace \"'false'\" " \
+      "--param with \"'#{to_merge_path}'\" "\
+      "--output \"#{output_path}\" " \
+      "#{merge_xslt_path} #{base_profile_path}"
+    }
+
+    before(:each) do
+      stub_const("Yast::AutoInstallRulesClass::MERGE_XSLT_PATH", merge_xslt_path)
+    end
+
+    around(:each) do |example|
+      FileUtils.rm_rf(tmp_dir) if Dir.exist?(tmp_dir)
+      FileUtils.mkdir(tmp_dir)
+      example.run
+      FileUtils.rm_rf(tmp_dir)
+    end
+
+    before(:each) do
+      allow(Yast::AutoinstConfig).to receive(:tmpDir).and_return(tmp_dir)
+      allow(Yast::AutoinstConfig).to receive(:dontmerge).and_return(dontmerge)
+      subject.Files
+    end
+
+    it 'executes xsltproc and returns a hash with info about the result' do
+      expect(Yast::SCR).to receive(:Execute).
+        with(Yast::Path.new(".target.bash_output"), xsltproc_command, {}).and_call_original
+      out = subject.merge_profiles(base_profile_path, to_merge_path, output_path)
+      expect(out).to eq({ 'exit' => 0, 'stderr' => '', 'stdout' => '' })
+    end
+
+    context 'when all elements must be merged' do
+      let(:expected_xml_path) { File.join(root_path, 'test', 'fixtures', 'output', 'partitions-merged.xml')  }
+
+      it 'merges elements from the base profile and the rule profile' do
+        expect(Yast::SCR).to receive(:Execute).
+          with(Yast::Path.new(".target.bash_output"), xsltproc_command, {}).and_call_original
+        out = subject.merge_profiles(base_profile_path, to_merge_path, output_path)
+        expect(output_xml).to eq(expected_xml)
+      end
+    end
+
+    context 'when some elements are not intended to be merged' do
+      let(:expected_xml_path) { File.join(root_path, 'test', 'fixtures', 'output', 'partitions-dontmerge.xml')  }
+      let(:dontmerge) { ['partition'] }
+      let(:xsltproc_command) {
+        "/usr/bin/xsltproc --novalid --maxdepth 10000 --param replace \"'false'\" " \
+        "--param dontmerge1 \"'partition'\" " \
+        "--param with \"'#{to_merge_path}'\" "\
+        "--output \"#{output_path}\" " \
+        "#{merge_xslt_path} #{base_profile_path}"
+      }
+
+      it 'does not merge those elements' do
+        expect(Yast::SCR).to receive(:Execute).
+          with(Yast::Path.new(".target.bash_output"), xsltproc_command, {}).and_call_original
+        out = subject.merge_profiles(base_profile_path, to_merge_path, output_path)
+        expect(output_xml).to eq(expected_xml)
+      end
+    end
+  end
 end

test/AutoinstClass_test.rb

@@ -222,7 +222,7 @@
     let(:merge_xslt_path) { File.join(ROOT_PATH, 'xslt', 'merge.xslt') }
     let(:conf_to_merge) { { "class" => "swap", "name" => "largeswap.xml" } }
     let(:xsltproc_command) {
-      "/usr/bin/xsltproc --novalid --param replace \"'false'\"  " \
+      "/usr/bin/xsltproc --novalid --maxdepth 10000 --param replace \"'false'\"  " \
       "--param with \"'#{subject.findPath("largeswap.xml", "swap")}'\"  "\
       "--output #{File.join(tmp_dir, "output.xml")}  " \
       "#{merge_xslt_path} #{base_profile_path} "
@@ -267,7 +267,7 @@
       let(:expected_xml_path) { File.join(ROOT_PATH, 'test', 'fixtures', 'output', 'partitions-dontmerge.xml')  }
       let(:dontmerge) { ['partition'] }
       let(:xsltproc_command) {
-        "/usr/bin/xsltproc --novalid --param replace \"'false'\"  " \
+        "/usr/bin/xsltproc --novalid --maxdepth 10000 --param replace \"'false'\"  " \
         "--param dontmerge1 \"'partition'\"  " \
         "--param with \"'#{subject.findPath("largeswap.xml", "swap")}'\"  "\
         "--output #{File.join(tmp_dir, "output.xml")}  " \