Merge pull request #634 from yast/autoinst_clone_cleaning

Autoinst clone cleaning

package/autoyast2.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Fri Jun 26 11:21:11 UTC 2020 - Josef Reidinger <jreidinger@suse.com>
+
+- Cloning does not depend on the SetModified API call(bsc#1172552)
+- 4.3.17
+
 -------------------------------------------------------------------
 Tue Jun 23 20:15:14 UTC 2020 - Josef Reidinger <jreidinger@suse.com>
 

package/autoyast2.spec

@@ -22,7 +22,7 @@
 %endif
 
 Name:           autoyast2
-Version:        4.3.16
+Version:        4.3.17
 Release:        0
 Summary:        YaST2 - Automated Installation
 License:        GPL-2.0-only

src/include/autoinstall/conftree.rb

@@ -20,7 +20,6 @@ def initialize_autoinstall_conftree(_include_target)
       Yast.import "Stage"
       Yast.import "Icon"
       Yast.import "AutoinstSoftware"
-      Yast.import "AutoinstClone"
 
       @title = _("Autoinstallation - Configuration")
       @show_source = false

src/modules/AutoinstClone.rb

@@ -35,50 +35,6 @@ def main
       @additional = []
     end
 
-    # Detects whether the current system uses multipath
-    # @return [Boolean] if in use
-    def multipath_in_use?
-      !Y2Storage::StorageManager.instance.probed.multipaths.empty?
-    end
-
-    # General options
-    #
-    # @return [Hash] general options
-    def General
-      Yast.import "Mode"
-      Mode.SetMode("normal")
-
-      general = {}
-      general["mode"] = { "confirm" => false }
-      general["storage"] = { "start_multipath" => true } if multipath_in_use?
-
-      Mode.SetMode("autoinst_config")
-      general
-    end
-
-    # Clone a Resource
-    #
-    # @param _resource    [String] resource. Not used.
-    # @param resource_map [Hash] resources map
-    # @return [Array]
-    def CommonClone(_resource, resource_map)
-      auto = Ops.get_string(resource_map, "X-SuSE-YaST-AutoInstClient", "")
-
-      # Do not read settings from system in first stage, autoyast profile
-      # should contain only proposed and user modified values.
-      # Exception: Storage and software module have autoyast modules which are
-      #            defined in autoyast itself.
-      #            So, these modules have to be called.
-      if !Stage.initial ||
-          ["software_auto", "storage_auto"].include?(auto)
-        Call.Function(auto, ["Read"])
-      end
-      # Flagging YAST module for export
-      Call.Function(auto, ["SetModified"])
-
-      true
-    end
-
     # Create a list of clonable resources
     #
     # @return [Array<Yast::Term>] list to be used in widgets (sorted by its label)
@@ -104,12 +60,11 @@ def createClonableList
 
     # Builds the profile
     #
-    # @return [void]
-    # @see ProfileClass.Prepare
+    # @return [void] returns void and sets profile in ProfileClass.current
+    # @see ProfileClass.create
+    # @see ProfileClass.current for result
     def Process
       log.info "Additional resources: #{@additional}"
-      Profile.Reset
-      Profile.prepare = true
       Mode.SetMode("autoinst_config")
 
       Y2ModuleConfig.ModuleMap.each do |def_resource, resource_map|
@@ -119,27 +74,61 @@ def Process
 
         next unless @additional.include?(resource)
 
-        log.info "Now cloning: #{resource}"
         time_start = Time.now
-        CommonClone(def_resource, resource_map)
+        read_module(resource_map)
         log.info "Cloning #{resource} took: #{(Time.now - time_start).round} sec"
       end
 
-      if @additional.include?("general")
-        Call.Function("general_auto", ["Import", General()])
-        Call.Function("general_auto", ["SetModified"])
-      end
+      Call.Function("general_auto", ["Import", General()]) if @additional.include?("general")
 
-      Profile.Prepare
+      Profile.create(@additional)
       nil
     end
 
-    publish variable: :Profile, type: "map"
-    publish variable: :base, type: "list <string>"
     publish variable: :additional, type: "list <string>"
-    publish function: :General, type: "map ()"
     publish function: :createClonableList, type: "list ()"
     publish function: :Process, type: "void ()"
+
+  private
+
+    # Detects whether the current system uses multipath
+    # @return [Boolean] if in use
+    def multipath_in_use?
+      !Y2Storage::StorageManager.instance.probed.multipaths.empty?
+    end
+
+    # General options
+    #
+    # @return [Hash] general options
+    def General
+      Yast.import "Mode"
+      Mode.SetMode("normal")
+
+      general = {}
+      general["mode"] = { "confirm" => false }
+      general["storage"] = { "start_multipath" => true } if multipath_in_use?
+
+      Mode.SetMode("autoinst_config")
+      general
+    end
+
+    # Reads module if it is appropriate
+    #
+    # @param resource_map [Hash] resources map
+    # @return [void]
+    def read_module(resource_map)
+      auto = Ops.get_string(resource_map, "X-SuSE-YaST-AutoInstClient", "")
+
+      # Do not read settings from system in first stage, autoyast profile
+      # should contain only proposed and user modified values.
+      # Exception: Storage and software module have autoyast modules which are
+      #            defined in autoyast itself.
+      #            So, these modules have to be called.
+      if !Stage.initial ||
+          ["software_auto", "storage_auto"].include?(auto)
+        Call.Function(auto, ["Read"])
+      end
+    end
   end
 
   AutoinstClone = AutoinstCloneClass.new

src/modules/Profile.rb

@@ -290,6 +290,8 @@ def Import(profile)
     end
 
     # Prepare Profile for saving and remove empty data structs
+    # This is mainly for editing profile when there is some parts we do not write ourself
+    # For creating new one from given set of modules see {#create}
     # @return [void]
     def Prepare
       return if !@prepare
@@ -299,74 +301,27 @@ def Prepare
         _("This may take a while")
       )
 
-      e = []
-
-      Builtins.foreach(@ModuleMap) do |p, d|
-        #
-        # Set resource name, if not using default value
-        #
-        resource = Ops.get_string(d, "X-SuSE-YaST-AutoInstResource", "")
-        resource = p if resource == ""
-        tomerge = Ops.get_string(d, "X-SuSE-YaST-AutoInstMerge", "")
-        module_auto = Ops.get_string(d, "X-SuSE-YaST-AutoInstClient", "none")
-        if Convert.to_boolean(WFM.CallFunction(module_auto, ["GetModified"]))
-          resource_data = WFM.CallFunction(module_auto, ["Export"])
-
-          s = 0
-          if tomerge == ""
-            s = if Ops.get_string(d, "X-SuSE-YaST-AutoInstDataType", "map") == "map"
-              Builtins.size(Convert.to_map(resource_data))
-            else
-              Builtins.size(Convert.to_list(resource_data))
-            end
-          end
-
-          if s != 0 || tomerge != ""
-            if Ops.greater_than(Builtins.size(tomerge), 0)
-              i = 0
-              tomergetypes = Ops.get_string(
-                d,
-                "X-SuSE-YaST-AutoInstMergeTypes",
-                ""
-              )
-              _MergeTypes = Builtins.splitstring(tomergetypes, ",")
-
-              Builtins.foreach(Builtins.splitstring(tomerge, ",")) do |res|
-                rd = Convert.convert(
-                  resource_data,
-                  from: "any",
-                  to:   "map <string, any>"
-                )
-
-                value =
-                  if !rd.key?(res)
-                    nil
-                  elsif Ops.get_string(_MergeTypes, i, "map") == "map"
-                    Ops.get_map(rd, res, {})
-                  else
-                    Ops.get_list(rd, res, [])
-                  end
-                Ops.set(@current, res, value) if value
-                i = Ops.add(i, 1)
-              end
-            else
-              Ops.set(@current, resource, resource_data)
-            end
-          elsif s == 0
-            e = Builtins.add(e, resource)
-          end
-        end
-      end
-
-      Builtins.foreach(@current) do |k, v|
-        Ops.set(@current, k, v) if !Builtins.haskey(@current, k) && !Builtins.contains(e, k)
-      end
+      edit_profile
 
       Popup.ClearFeedback
       @prepare = false
       nil
     end
 
+    # Sets Profile#current to exported values created from given set of modules
+    # @return [Hash] value set to Profile#current
+    def create(modules)
+      Popup.Feedback(
+        _("Collecting configuration data..."),
+        _("This may take a while")
+      ) do
+        @current = {}
+        edit_profile(modules)
+      end
+
+      @current
+    end
+
     # Reset profile to initial status
     # @return [void]
     def Reset
@@ -824,6 +779,46 @@ def resource_aliases_map
       Yast.import "Y2ModuleConfig"
       Y2ModuleConfig.resource_aliases_map
     end
+
+    # Edits profile for given modules. If nil is passed, it used GetModfied method.
+    def edit_profile(modules = nil)
+      @ModuleMap.each_pair do |name, module_map|
+        #
+        # Set resource name, if not using default value
+        #
+        resource = module_map["X-SuSE-YaST-AutoInstResource"] || name
+        tomerge = module_map["X-SuSE-YaST-AutoInstMerge"] || ""
+        # TODO: is none valid or better use exception?
+        module_auto = module_map["X-SuSE-YaST-AutoInstClient"] || "none"
+        export = if modules
+          modules.include?(resource) || modules.include?(name)
+        else
+          WFM.CallFunction(module_auto, ["GetModified"])
+        end
+        next unless export
+
+        resource_data = WFM.CallFunction(module_auto, ["Export"])
+
+        if tomerge == ""
+          s = (resource_data || {}).size
+          if s > 0
+            @current[resource] = resource_data
+          else
+            @current.delete(resource)
+          end
+        else
+          tomerge.split(",").each_with_index do |res, _i|
+            value = resource_data[res]
+            if !value
+              log.warn "key #{res} expected to be exported from #{resource}"
+              next
+            end
+            # FIXME: no deleting for merged keys, is it correct?
+            @current[res] = value unless value.empty?
+          end
+        end
+      end
+    end
   end
 
   Profile = ProfileClass.new