feat: unify progress handling

rust/agama-lib/src/proxies.rs

@@ -23,6 +23,10 @@ trait Progress {
     /// TotalSteps property
     #[dbus_proxy(property)]
     fn total_steps(&self) -> zbus::Result<u32>;
+
+    /// Steps property
+    #[dbus_proxy(property)]
+    fn steps(&self) -> zbus::Result<Vec<String>>;
 }
 
 #[dbus_proxy(

rust/agama-server/src/web/common.rs

@@ -165,10 +165,21 @@ struct ProgressState<'a> {
     proxy: ProgressProxy<'a>,
 }
 
-async fn progress(State(state): State<ProgressState<'_>>) -> Result<Json<Progress>, Error> {
+/// Information about the current progress sequence.
+#[derive(Clone, Default, Serialize)]
+pub struct ProgressSequence {
+    /// Sequence steps if known in advance
+    steps: Vec<String>,
+    #[serde(flatten)]
+    progress: Progress,
+}
+
+async fn progress(State(state): State<ProgressState<'_>>) -> Result<Json<ProgressSequence>, Error> {
     let proxy = state.proxy;
     let progress = Progress::from_proxy(&proxy).await?;
-    Ok(Json(progress))
+    let steps = proxy.steps().await?;
+    let sequence = ProgressSequence { steps, progress };
+    Ok(Json(sequence))
 }
 
 #[pin_project]

service/lib/agama/dbus/interfaces/progress.rb

@@ -78,6 +78,15 @@ def progress_finished
           backend.progress.finished?
         end
 
+        # Returns the known step descriptions
+        #
+        # @return [Array<String>]
+        def progress_steps
+          return [] unless backend.progress
+
+          backend.progress.descriptions
+        end
+
         # D-Bus properties of the Progress interface
         #
         # @return [Hash]
@@ -104,6 +113,7 @@ def self.included(base)
               dbus_reader :progress_total_steps, "u", dbus_name: "TotalSteps"
               dbus_reader :progress_current_step, "(us)", dbus_name: "CurrentStep"
               dbus_reader :progress_finished, "b", dbus_name: "Finished"
+              dbus_reader :progress_steps, "as", dbus_name: "Steps"
             end
           end
         end

service/lib/agama/manager.rb

@@ -84,9 +84,11 @@ def config_phase
       service_status.busy
       installation_phase.config
 
-      start_progress(2)
-      progress.step(_("Probing Storage")) { storage.probe }
-      progress.step(_("Probing Software")) { software.probe }
+      start_progress_with_descriptions(
+        _("Probing Storage"), _("Probing Software")
+      )
+      progress.step { storage.probe }
+      progress.step { software.probe }
 
       logger.info("Config phase done")
     rescue StandardError => e
@@ -102,26 +104,31 @@ def config_phase
     def install_phase
       service_status.busy
       installation_phase.install
-      start_progress(7)
+      start_progress_with_descriptions(
+        _("Partitioning"),
+        _("Installing software"),
+        _("Configuring the system")
+      )
 
       Yast::Installation.destdir = "/mnt"
 
-      progress.step(_("Partitioning")) do
+      progress.step do
         storage.install
         proxy.propose
         # propose software after /mnt is already separated, so it uses proper
         # target
         software.propose
       end
 
-      progress.step(_("Installing Software")) { software.install }
-
-      on_target do
-        progress.step(_("Writing Users")) { users.write }
-        progress.step(_("Writing Network Configuration")) { network.install }
-        progress.step(_("Saving Language Settings")) { language.finish }
-        progress.step(_("Writing repositories information")) { software.finish }
-        progress.step(_("Finishing storage configuration")) { storage.finish }
+      progress.step { software.install }
+      progress.step do
+        on_target do
+          users.write
+          network.install
+          language.finish
+          software.finish
+          storage.finish
+        end
       end
 
       logger.info("Install phase done")

service/lib/agama/progress.rb

@@ -24,9 +24,15 @@ module Agama
   #
   # It allows to configure callbacks to be called on each step and also when the progress finishes.
   #
+  # In most cases all steps are known in advance (e.g., "probing software", "probing storage", etc.)
+  # but, in some situations, only the number of steps is known (e.g., "Installing package X").
+  #
+  # Use the Progress.with_descriptions to initialize a progress with known step descriptions and
+  # Progress.with_size when only the number of steps is known
+  #
   # @example
   #
-  #   progress = Progress.new(3)                    # 3 steps
+  #   progress = Progress.with_size(3)              # 3 steps
   #   progress.on_change { puts progress.message }  # configures callbacks
   #   progress.on_finish { puts "finished"  }       # configures callbacks
   #
@@ -45,6 +51,14 @@ module Agama
   #
   #   progress.finished?                            #=> true
   #   progress.current_step                         #=> nil
+  #
+  # @example Progress with known step descriptions
+  #
+  #   progress = Progress.with_descriptions(["Partitioning", "Installing", "Finishing"])
+  #   progress.step { partitioning }                      # next step
+  #   progress.current_step.description                   #=> "Partitioning"
+  #   progress.step("Installing packages") { installing } # overwrite the description
+  #   progress.current_step.description                   # "Installing packages"
   class Progress
     # Step of the progress
     class Step
@@ -73,11 +87,30 @@ def initialize(id, description)
     # @return [Integer]
     attr_reader :total_steps
 
+    # Step descriptions in case they are known
+    #
+    # @return [Array<String>]
+    attr_reader :descriptions
+
+    class << self
+      def with_size(size)
+        new(size: size)
+      end
+
+      def with_descriptions(descriptions)
+        new(descriptions: descriptions)
+      end
+    end
+
     # Constructor
     #
-    # @param total_steps [Integer] total number of steps
-    def initialize(total_steps)
-      @total_steps = total_steps
+    # @param descriptions [Array<String>] Steps of the progress sequence. This argument
+    #   has precedence over the `size`
+    # @param size [Integer] total number of steps of the progress sequence
+    def initialize(descriptions: [], size: nil)
+      @descriptions = descriptions || []
+      @total_steps = descriptions.size unless descriptions.empty?
+      @total_steps ||= size
       @current_step = nil
       @counter = 0
       @finished = false
@@ -99,15 +132,16 @@ def current_step
     # It calls the `on_change` callbacks and then runs the given block, if any. It also calls
     # `on_finish` callbacks after the last step.
     #
-    # @param description [String] description of the step
+    # @param description [String, nil] description of the step
     # @param block [Proc]
     #
     # @return [Object, nil] result of the given block or nil if no block is given
-    def step(description, &block)
+    def step(description = nil, &block)
       return if finished?
 
       @counter += 1
-      @current_step = Step.new(@counter, description)
+      step_description = description || description_for(@counter)
+      @current_step = Step.new(@counter, step_description)
       @on_change_callbacks.each(&:call)
 
       result = block_given? ? block.call : nil
@@ -154,5 +188,11 @@ def to_s
 
       "#{current_step.description} (#{@counter}/#{total_steps})"
     end
+
+  private
+
+    def description_for(step)
+      @descriptions[step - 1] || format(_("Step %s/%s"), step, total_steps)
+    end
   end
 end

service/lib/agama/software/callbacks/progress.rb

@@ -41,8 +41,8 @@ def initialize(pkg_count, progress)
         end
 
         def setup
-          Yast::Pkg.CallbackDonePackage(
-            fun_ref(method(:package_installed), "string (integer, string)")
+          Yast::Pkg.CallbackStartPackage(
+            fun_ref(method(:start_package), "void (string, string, string, integer, boolean)")
           )
         end
 
@@ -55,12 +55,8 @@ def fun_ref(method, signature)
           Yast::FunRef.new(method, signature)
         end
 
-        # TODO: error handling
-        def package_installed(_error, _reason)
-          @installed += 1
-          progress.step(msg)
-
-          ""
+        def start_package(package, _file, _summary, _size, _other)
+          progress.step("Installing #{package}")
         end
 
         def msg

service/lib/agama/software/manager.rb

@@ -132,11 +132,11 @@ def probe
         logger.info "Probing software"
 
         if repositories.empty?
-          start_progress(3)
+          start_progress_with_size(3)
           Yast::PackageCallbacks.InitPackageCallbacks(logger)
           progress.step(_("Initializing sources")) { add_base_repos }
         else
-          start_progress(2)
+          start_progress_with_size(2)
         end
 
         progress.step(_("Refreshing repositories metadata")) { repositories.load }
@@ -178,7 +178,7 @@ def install
         Yast::Pkg.TargetLoad
 
         steps = proposal.packages_count
-        start_progress(steps)
+        start_progress_with_size(steps)
         Callbacks::Progress.setup(steps, progress)
 
         # TODO: error handling
@@ -197,16 +197,13 @@ def install
 
       # Writes the repositories information to the installed system
       def finish
-        start_progress(1)
-        progress.step(_("Writing repositories to the target system")) do
-          Yast::Pkg.SourceSaveAll
-          Yast::Pkg.TargetFinish
-          # FIXME: Pkg.SourceCacheCopyTo works correctly only from the inst-sys
-          # (original target "/"), it does not work correctly when using
-          # "chroot" /run/agama/zypp, it needs to be reimplemented :-(
-          # Yast::Pkg.SourceCacheCopyTo(Yast::Installation.destdir)
-          registration.finish
-        end
+        Yast::Pkg.SourceSaveAll
+        Yast::Pkg.TargetFinish
+        # FIXME: Pkg.SourceCacheCopyTo works correctly only from the inst-sys
+        # (original target "/"), it does not work correctly when using
+        # "chroot" /run/agama/zypp, it needs to be reimplemented :-(
+        # Yast::Pkg.SourceCacheCopyTo(Yast::Installation.destdir)
+        registration.finish
       end
 
       # Determine whether the given tag is provided by the selected packages

service/lib/agama/storage/finisher.rb

@@ -50,7 +50,7 @@ def initialize(logger, config, security)
       # Execute the final storage actions, reporting the progress
       def run
         steps = possible_steps.select(&:run?)
-        start_progress(steps.size)
+        start_progress_with_size(steps.size)
 
         on_target do
           steps.each do |step|

service/lib/agama/storage/manager.rb

@@ -107,7 +107,7 @@ def on_probe(&block)
 
       # Probes storage devices and performs an initial proposal
       def probe
-        start_progress(4)
+        start_progress_with_size(4)
         config.pick_product(software.selected_product)
         check_multipath
         progress.step(_("Activating storage devices")) { activate_devices }
@@ -120,7 +120,7 @@ def probe
 
       # Prepares the partitioning to install the system
       def install
-        start_progress(4)
+        start_progress_with_size(4)
         progress.step(_("Preparing bootloader proposal")) do
           # first make bootloader proposal to be sure that required packages are installed
           proposal = ::Bootloader::ProposalClient.new.make_proposal({})

service/lib/agama/with_progress.rb

@@ -30,18 +30,34 @@ class NotFinishedProgress < StandardError; end
     # @return [Progress, nil]
     attr_reader :progress
 
+    # Creates a new progress with a give number of steps
+    #
+    # @param size [Integer] Number of steps
+    def start_progress_with_size(size)
+      start_progress(size: size)
+    end
+
+    # Creates a new progress with a given set of steps
+    #
+    # @param descriptions [Array<String>] Steps descriptions
+    def start_progress_with_descriptions(*descriptions)
+      start_progress(descriptions: descriptions)
+    end
+
     # Creates a new progress
     #
+    # Prefer using #start_progress_with_Size or #start_progress_with_descriptions.
+    #
     # @raise [RuntimeError] if there is an unfinished progress.
     #
-    # @param total_steps [Integer] total number of the steps for the progress.
-    def start_progress(total_steps)
+    # @param args [*Hash] Progress constructor arguments.
+    def start_progress(args)
       raise NotFinishedProgress if progress && !progress.finished?
 
       on_change_callbacks = @on_progress_change_callbacks || []
       on_finish_callbacks = @on_progress_finish_callbacks || []
 
-      @progress = Progress.new(total_steps).tap do |progress|
+      @progress = Progress.new(**args).tap do |progress|
         progress.on_change { on_change_callbacks.each(&:call) }
         progress.on_finish { on_finish_callbacks.each(&:call) }
       end