Extract poll and finish methods of install progress

common/rinstallprogress.cc

@@ -30,6 +30,7 @@
 
 #include <apt-pkg/install-progress.h>
 #include <apt-pkg/packagemanager.h>
+#include <optional>
 #include <stdlib.h>
 #include <string>
 #include <sys/types.h>
@@ -70,13 +71,12 @@ const char *RInstallProgress::getResultStr(pkgPackageManager::OrderResult res)
    return "Unknown install result.";
 }
 
-pkgPackageManager::OrderResult RInstallProgress::start(pkgPackageManager *pm,
-                                                       int numPackages,
-                                                       int numPackagesTotal)
+std::optional<pkgPackageManager::OrderResult> RInstallProgress::start(
+   pkgPackageManager *pm,
+   int numPackages,
+   int numPackagesTotal)
 {
    pkgPackageManager::OrderResult res;
-   int ret;
-   pid_t _child_id;
 
    // cout << "RInstallProgress::start()" << endl;
 
@@ -136,19 +136,26 @@ pkgPackageManager::OrderResult RInstallProgress::start(pkgPackageManager *pm,
    }
 #endif
 
-   startUpdate();
-   while (waitpid(_child_id, &ret, WNOHANG) == 0)
-      updateInterface();
+   return std::nullopt;
+}
 
-   res = (pkgPackageManager::OrderResult)WEXITSTATUS(ret);
+std::optional<pkgPackageManager::OrderResult> RInstallProgress::poll()
+{
+   int ret = 0;
+   pid_t pid = waitpid(_child_id, &ret, WNOHANG);
+   if (pid == 0) {
+      return std::nullopt;
+   }
 
-   finishUpdate();
+   // WEXITSTATUS of a signal-killed child is 0, i.e. Completed
+   pkgPackageManager::OrderResult res =
+      (pid > 0 && WIFEXITED(ret))
+         ? (pkgPackageManager::OrderResult)WEXITSTATUS(ret)
+         : pkgPackageManager::Failed;
 
 #ifdef HAVE_RPM
    close(_childin);
 #endif
 
    return res;
 }
-
-// vim:sts=4:sw=4

common/rinstallprogress.h

@@ -28,11 +28,14 @@
 
 #include <apt-pkg/packagemanager.h>
 
+#include <optional>
 #include <string>
 
 class RInstallProgress
 {
  protected:
+   pid_t _child_id;
+
    int _stdout;
    int _stderr;
    int _childin;
@@ -49,21 +52,28 @@ class RInstallProgress
    static std::string errorMsg;
    static std::string incompleteMsg;
 
+   // get a str feed to the user with the result of the install run
+   virtual const char *getResultStr(pkgPackageManager::OrderResult);
+
+ public:
+   // std::nullopt means the child is still running; a plain OrderResult
+   // would be ambiguous here as Incomplete is a real child result
+   // (media swap needed)
+   virtual std::optional<pkgPackageManager::OrderResult> start(
+      pkgPackageManager *pm,
+      int numPackages = 0,
+      int numPackagesTotal = 0);
+   virtual std::optional<pkgPackageManager::OrderResult> poll();
+   virtual void finish()
+   {}
+
    virtual void startUpdate()
    {}
    virtual void updateInterface()
    {}
    virtual void finishUpdate()
    {}
 
- public:
-   // get a str feed to the user with the result of the install run
-   virtual const char *getResultStr(pkgPackageManager::OrderResult);
-   virtual pkgPackageManager::OrderResult start(pkgPackageManager *pm,
-                                                int numPackages = 0,
-                                                int numPackagesTotal = 0);
-
-
    RInstallProgress()
       : _donePackagesTotal(0), _numPackagesTotal(0), _updateFinished(false)
    {}

common/rpackagelister.cc

@@ -69,6 +69,7 @@
 #include <dirent.h>
 #include <iostream>
 #include <map>
+#include <optional>
 #include <regex.h>
 #include <set>
 #include <sstream>
@@ -1492,23 +1493,25 @@ bool RPackageLister::commitChanges(pkgAcquireStatus *status,
          _("Ignoring invalid record(s) in sources.list file!"));
    }
 
-   pkgPackageManager *rPM;
-   rPM = _system->CreatePM(_cache->deps());
+   std::unique_ptr<pkgPackageManager> rPM{_system->CreatePM(_cache->deps())};
 
    if (!rPM->GetArchives(&fetcher, _cache->list(), _records) ||
-       _error->PendingError())
-      goto gave_wood;
+       _error->PendingError()) {
+      return false;
+   }
 
    // ripped from apt-get
    while (1) {
       bool Transient = false;
 
 #ifdef HAVE_RPM
-      if (fetcher.Run() == pkgAcquire::Failed)
-         goto gave_wood;
+      if (fetcher.Run() == pkgAcquire::Failed) {
+         return false;
+      }
 #else
-      if (fetcher.Run(50000) == pkgAcquire::Failed)
-         goto gave_wood;
+      if (fetcher.Run(50000) == pkgAcquire::Failed) {
+         return false;
+      }
 #endif
 
       string serverError;
@@ -1560,25 +1563,24 @@ bool RPackageLister::commitChanges(pkgAcquireStatus *status,
       if (Failed) {
          string message;
 
-         if (Transient)
-            goto gave_wood;
-
-         if (numPackages == 0)
-            goto gave_wood;
+         if (Transient || numPackages == 0) {
+            return false;
+         }
 
          message = _("Some of the packages could not be retrieved from the "
                      "server(s).\n");
          if (!serverError.empty())
             message += "(" + serverError + ")\n";
          message += _("Do you want to continue, ignoring these packages?");
 
-         if (!_userDialog->confirm(message.c_str()))
-            goto gave_wood;
+         if (!_userDialog->confirm(message.c_str())) {
+            return false;
+         }
       }
       // Try to deal with missing package files
       if (Failed == true && rPM->FixMissing() == false) {
          _error->Error(_("Unable to correct missing packages"));
-         goto gave_wood;
+         return false;
       }
       // need this so that we first fetch everything and then install (for CDs)
       if (Transient == false ||
@@ -1590,29 +1592,38 @@ bool RPackageLister::commitChanges(pkgAcquireStatus *status,
          }
 
          _system->UnLockInner();
-         pkgPackageManager::OrderResult Res =
-            iprog->start(rPM, numPackages, numPackagesTotal);
+         std::optional<pkgPackageManager::OrderResult> Res;
+         Res = iprog->start(rPM.get(), numPackages, numPackagesTotal);
+         if (!Res.has_value()) {
+            iprog->startUpdate();
+            while (!(Res = iprog->poll()).has_value()) {
+               iprog->updateInterface();
+            }
+            iprog->finishUpdate();
+         }
          _system->LockInner();
-         if (Res == pkgPackageManager::Failed || _error->PendingError()) {
-            if (Transient == false)
-               goto gave_wood;
+         if (*Res == pkgPackageManager::Failed || _error->PendingError()) {
+            if (Transient == false) {
+               return false;
+            }
             Ret = false;
             // TODO: We must not discard errors here. The right
             //       solution is to use an "error stack", as
             //       implemented in apt-rpm.
             //_error->DumpErrors();
             _error->Discard();
          }
-         if (Res == pkgPackageManager::Completed)
+         if (*Res == pkgPackageManager::Completed)
             break;
 
          numPackages = 0;
       }
       // Reload the fetcher object and loop again for media swapping
       fetcher.Shutdown();
 
-      if (!rPM->GetArchives(&fetcher, _cache->list(), _records))
-         goto gave_wood;
+      if (!rPM->GetArchives(&fetcher, _cache->list(), _records)) {
+         return false;
+      }
    }
 
    // cout << _("Finished.")<<endl;
@@ -1624,12 +1635,7 @@ bool RPackageLister::commitChanges(pkgAcquireStatus *status,
    if (_config->FindB("Synaptic::Log::Changes", true))
       writeCommitLog();
 
-   delete rPM;
    return Ret;
-
-gave_wood:
-   delete rPM;
-   return false;
 }
 
 void RPackageLister::writeCommitLog()

gtk/rgdebinstallprogress.cc

@@ -42,15 +42,9 @@
 #   include <cstring>
 #   include <ctime>
 #   include <fcntl.h>
-#   include <gdk/gdk.h>
-#   include <gdk/gdkkeysyms-compat.h>
-#   include <glib.h>
-#   include <glib/gtypes.h>
-#   include <gobject/gclosure.h>
 #   include <gtk/gtk.h>
-#   include <gtk/gtkcssprovider.h>
 #   include <iostream>
-#   include <pango/pango-font.h>
+#   include <optional>
 #   include <pty.h>
 #   include <signal.h>
 #   include <stdio.h>
@@ -71,7 +65,8 @@ void RGDebInstallProgress::child_exited(VteTerminal *vteterminal,
 {
    RGDebInstallProgress *me = (RGDebInstallProgress *)data;
 
-   me->res = (pkgPackageManager::OrderResult)WEXITSTATUS(ret);
+   me->res = WIFEXITED(ret) ? (pkgPackageManager::OrderResult)WEXITSTATUS(ret)
+                            : pkgPackageManager::Failed;
    me->child_has_exited = true;
 }
 
@@ -481,7 +476,7 @@ gboolean RGDebInstallProgress::key_press_event(GtkWidget *widget,
    RGDebInstallProgress *me = (RGDebInstallProgress *)user_data;
 
    // user pressed ctrl-c
-   if (event->keyval == GDK_c && event->state & GDK_CONTROL_MASK) {
+   if (event->keyval == GDK_KEY_c && event->state & GDK_CONTROL_MASK) {
       gchar *summary = _("Ctrl-c pressed");
       char *msg = _("This will abort the operation and may leave the system "
                     "in a broken state. Are you sure you want to do that?");
@@ -501,15 +496,15 @@ gboolean RGDebInstallProgress::key_press_event(GtkWidget *widget,
          case GTK_RESPONSE_NO:
             return true;
       }
-   } else if (event->keyval == GDK_C &&
+   } else if (event->keyval == GDK_KEY_C &&
               event->state & (GDK_CONTROL_MASK | GDK_SHIFT_MASK)) {
       // ctrl+shift+C copy to clipboard to mimic gnome-terminal behavior
       me->terminalAction(me->_term, EDIT_COPY);
       return true;
-   } else if (event->keyval == GDK_a && event->state & GDK_CONTROL_MASK) {
+   } else if (event->keyval == GDK_KEY_a && event->state & GDK_CONTROL_MASK) {
       me->terminalAction(me->_term, EDIT_SELECT_ALL);
       return true;
-   } else if (event->keyval == GDK_A &&
+   } else if (event->keyval == GDK_KEY_A &&
               event->state & (GDK_CONTROL_MASK | GDK_SHIFT_MASK)) {
       me->terminalAction(me->_term, EDIT_SELECT_NONE);
       return true;
@@ -664,7 +659,7 @@ void RGDebInstallProgress::updateInterface()
    }
 }
 
-pkgPackageManager::OrderResult RGDebInstallProgress::start(
+std::optional<pkgPackageManager::OrderResult> RGDebInstallProgress::start(
    pkgPackageManager *pm,
    int numPackages,
    int numPackagesTotal)
@@ -675,7 +670,6 @@ pkgPackageManager::OrderResult RGDebInstallProgress::start(
    if (res == pkgPackageManager::Failed)
       return res;
 
-   int master;
    _child_id = forkpty(&master, NULL, NULL, NULL);
    if (_child_id < 0) {
       cerr << "vte_terminal_forkpty() failed. " << strerror(errno) << endl;
@@ -744,11 +738,13 @@ pkgPackageManager::OrderResult RGDebInstallProgress::start(
    _numPackages = numPackages;
    _numPackagesTotal = numPackagesTotal;
 
-   startUpdate();
-   while (!child_has_exited)
-      updateInterface();
+   return std::nullopt;
+}
 
-   finishUpdate();
+std::optional<pkgPackageManager::OrderResult> RGDebInstallProgress::poll()
+{
+   if (!child_has_exited)
+      return std::nullopt;
 
    ::close(_childin);
    ::close(master);

gtk/rgdebinstallprogress.h

@@ -31,11 +31,9 @@
 
 #   include <apt-pkg/packagemanager.h>
 #   include <gdk/gdk.h>
-#   include <glib-object.h>
-#   include <glib.h>
 #   include <gtk/gtk.h>
-#   include <gtk/gtkcssprovider.h>
 #   include <map>
+#   include <optional>
 #   include <string>
 #   include <sys/types.h>
 #   include <vte/vte.h>
@@ -110,7 +108,7 @@ class RGDebInstallProgress : public RInstallProgress, public RGGtkBuilderWindow
    // last time something changed
    time_t last_term_action;
 
-   pid_t _child_id;
+   int master;
    pkgPackageManager::OrderResult res;
    bool child_has_exited;
    static void child_exited(VteTerminal *vteterminal, gint ret, gpointer data);
@@ -119,15 +117,8 @@ class RGDebInstallProgress : public RInstallProgress, public RGGtkBuilderWindow
    GtkCssProvider *_cssProvider;
 
  protected:
-   virtual void startUpdate();
-   virtual void updateInterface();
-   virtual void finishUpdate();
    virtual bool close();
 
-   virtual pkgPackageManager::OrderResult start(pkgPackageManager *pm,
-                                                int numPackages = 0,
-                                                int totalPackages = 0);
-
    virtual void prepare(RPackageLister *lister);
 
    void conffile(gchar *conffile, gchar *status);
@@ -150,6 +141,16 @@ class RGDebInstallProgress : public RInstallProgress, public RGGtkBuilderWindow
  public:
    RGDebInstallProgress(RGMainWindow *main, RPackageLister *lister);
    virtual ~RGDebInstallProgress();
+
+   virtual std::optional<pkgPackageManager::OrderResult> start(
+      pkgPackageManager *pm,
+      int numPackages = 0,
+      int totalPackages = 0) override;
+   virtual std::optional<pkgPackageManager::OrderResult> poll() override;
+
+   virtual void startUpdate() override;
+   virtual void updateInterface() override;
+   virtual void finishUpdate() override;
 };
 
 #endif // WITH_DPKG_STATUSFD