[Aptitude-svn-commit] r4045 - in branches/aptitude-0.3/aptitude: . src src/cmdline

Daniel Burrows dburrows at costa.debian.org
Sun Sep 4 18:09:09 UTC 2005


Author: dburrows
Date: Sun Sep  4 18:09:06 2005
New Revision: 4045

Modified:
   branches/aptitude-0.3/aptitude/ChangeLog
   branches/aptitude-0.3/aptitude/src/cmdline/cmdline_do_action.cc
   branches/aptitude-0.3/aptitude/src/cmdline/cmdline_upgrade.cc
   branches/aptitude-0.3/aptitude/src/download.cc
   branches/aptitude-0.3/aptitude/src/download.h
   branches/aptitude-0.3/aptitude/src/ui.cc
Log:
Split do_install_run.

Modified: branches/aptitude-0.3/aptitude/ChangeLog
==============================================================================
--- branches/aptitude-0.3/aptitude/ChangeLog	(original)
+++ branches/aptitude-0.3/aptitude/ChangeLog	Sun Sep  4 18:09:06 2005
@@ -1,3 +1,11 @@
+2005-09-04  Daniel Burrows  <dburrows at debian.org>
+
+	* src/cmdline/cmdline_do_action.cc, src/cmdline/cmdline_upgrade.cc, src/download.cc, src/download.h, src/ui.cc:
+
+	  Split the download-and-install routines the same way that update
+	  was split.  NB: both splits should be refined in the future by
+	  creating classes that encapsulate the algorithms.
+
 2005-09-04 Clytie Siddall <clytie at riverland.net.au>
 
 	* Update vietnamese translation

Modified: branches/aptitude-0.3/aptitude/src/cmdline/cmdline_do_action.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/cmdline/cmdline_do_action.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/cmdline/cmdline_do_action.cc	Sun Sep  4 18:09:06 2005
@@ -6,12 +6,15 @@
 
 #include "cmdline_action.h"
 #include "cmdline_common.h"
+#include "cmdline_progress.h"
 #include "cmdline_prompt.h"
 #include "cmdline_resolver.h"
 #include "cmdline_show_broken.h"
 #include "cmdline_simulate.h"
 #include "cmdline_util.h"
 
+#include "../download_manager.h"
+
 #include <generic/apt.h>
 #include <generic/config_signal.h>
 
@@ -234,10 +237,33 @@
 	  return 0;
 	}
 
-      int rval=do_install_run(&progress, true, download_only)?0:-1;
+      download_manager *m = gen_cmdline_download_progress();
+      pkgAcquire *acq;
+      pkgPackageManager *pm;
+
+      bool success = prepare_install_run(&progress, true,
+					 download_only, m, acq, pm);
+      install_run_result run_res;
+
+      if(success)
+	{
+	  do
+	    {
+	      pkgAcquire::RunResult res = acq->Run();
+	      run_res = finish_install_run(&progress, true,
+					   download_only,
+					   m, acq, pm, res);
+	    } while(run_res == install_do_again);
+	}
+
+      delete acq;
+      delete pm;
+      delete m;
+
+      int rval = run_res == install_success ? 0 : -1;
 
       if(_error->PendingError())
-	rval=-1;
+	rval = -1;
 
       _error->DumpErrors();
 

Modified: branches/aptitude-0.3/aptitude/src/cmdline/cmdline_upgrade.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/cmdline/cmdline_upgrade.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/cmdline/cmdline_upgrade.cc	Sun Sep  4 18:09:06 2005
@@ -4,11 +4,14 @@
 
 #include "cmdline_upgrade.h"
 
+#include "cmdline_progress.h"
 #include "cmdline_prompt.h"
 #include "cmdline_show_broken.h"
 #include "cmdline_simulate.h"
 #include "cmdline_util.h"
 
+#include "../download_manager.h"
+
 #include <aptitude.h>
 #include <download.h>
 
@@ -16,6 +19,7 @@
 
 #include <apt-pkg/depcache.h>
 #include <apt-pkg/error.h>
+#include <apt-pkg/packagemanager.h>
 #include <apt-pkg/progress.h>
 
 int cmdline_upgrade(int argc, char *argv[],
@@ -100,10 +104,40 @@
 	  return 0;
 	}
 
-      int rval=do_install_run(&progress, true, download_only)?0:-1;
+
+
+      // Split this into common code?
+      download_manager *m = gen_cmdline_download_progress();
+      pkgAcquire *acq;
+      pkgPackageManager *pm;
+
+      bool success = prepare_install_run(&progress, true,
+					 download_only, m,
+					 acq, pm);
+      install_run_result run_res;
+
+      if(success)
+	{
+	  do
+	    {
+	      pkgAcquire::RunResult res = acq->Run();
+	      run_res = finish_install_run(&progress, true,
+					   download_only,
+					   m, acq, pm, res);
+	    } while(run_res == install_do_again);
+	}
+
+      delete acq;
+      delete pm;
+      delete m;
+
+
+
+
+      int rval = run_res == install_success ? 0 : -1;
 
       if(_error->PendingError())
-	rval=-1;
+	rval = -1;
 
       _error->DumpErrors();
 

Modified: branches/aptitude-0.3/aptitude/src/download.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/download.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/download.cc	Sun Sep  4 18:09:06 2005
@@ -1,10 +1,23 @@
 // download.cc
 //
-//  Copyright 2000 Daniel Burrows
+//   Copyright (C) 2000-2005 Daniel Burrows
 //
-//  Contains the scary code to download various things.  A little hairier than
-// it has to be, partly because I didn't understand how it worked at the time
-// that I wrote it.
+//   This program is free software; you can redistribute it and/or
+//   modify it under the terms of the GNU General Public License as
+//   published by the Free Software Foundation; either version 2 of
+//   the License, or (at your option) any later version.
+//
+//   This program is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+//   General Public License for more details.
+//
+//   You should have received a copy of the GNU General Public License
+//   along with this program; see the file COPYING.  If not, write to
+//   the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+//   Boston, MA 02111-1307, USA.
+
+#include "download.h"
 
 #include "aptitude.h"
 #include "ui.h"
@@ -64,17 +77,6 @@
 
 using namespace std;
 
-// This is a somewhat odd class, but it's useful for allowing downloads to be
-// aborted.
-class downloader:public sigc::trackable
-{
-  bool aborted;
-public:
-  downloader():aborted(false) {}
-  void abort() {aborted=true;}
-  bool get_aborted() {return aborted;}
-};
-
 pkgAcquire * prepare_pkglist_update(OpProgress *load_progress,
 				    bool text_download,
 				    pkgAcquireStatus *log)
@@ -276,11 +278,12 @@
   return true;
 }
 
-bool do_install_run(OpProgress *load_progress,
-		    bool text_download, bool download_only)
+bool prepare_install_run(OpProgress *load_progress,
+			 bool text_download, bool download_only,
+			 pkgAcquireStatus *log,
+			 pkgAcquire * &acq,
+			 pkgPackageManager * &pm_out)
 {
-  downloader abortable;
-
   if(!(*apt_cache_file)->save_selection_list(*load_progress))
     return false;
 
@@ -298,193 +301,197 @@
 	}
     }
 
-  download_manager *download_progress;
-  if(!text_download)
-    download_progress=gen_download_progress(false,
-					    _("Downloading packages"),
-					    _("View the progress of the package download"),
-					    _("Package Download"),
-					    arg(sigc::mem_fun(abortable,
-							      &downloader::abort)));
-  else
-    download_progress=gen_cmdline_download_progress();
-
-  pkgAcquire fetcher(download_progress);
-
   // Get source lists.
-  pkgSourceList list;
-  if(!list.ReadMainList())
+  pkgSourceList src_list;
+  if(!src_list.ReadMainList())
     {
       _error->Error(_("Couldn't read source list"));
 
-      delete download_progress;
       return false;
     }
 
+  pkgAcquire *fetcher = new pkgAcquire(log);
+
   // Make a package manager, get ready to download
-  pkgDPkgPM pm(*apt_cache_file);
-  if(!pm.GetArchives(&fetcher, &list, apt_package_records) || _error->PendingError())
+  pkgDPkgPM *pm = new pkgDPkgPM(*apt_cache_file);
+  if(!pm->GetArchives(fetcher, &src_list, apt_package_records) || _error->PendingError())
     {
       _error->Error(_("Internal error: couldn't generate list of packages to download"));
 
-      delete download_progress;
+      delete fetcher;
+      delete pm;
       return false;
     }
 
-  // FIXME: check for free space
-
-  // FIXME: console-apt checks for errors here.  I don't see anything that
-  // could cause errors :)
+  acq = fetcher;
+  pm_out = pm;
+  return true;
+}
 
-  // FIXME: the control logic in the following loop is very tortuous and
-  //       should be simplified.
+static void log_changes()
+{
+  vector<string> logs;
 
-  // This variable is set at the end of the loop iff the package run was
-  // *not incomplete*.  However, there are several other exits from the loop;
-  // in fact, the apt-get equivalent to this just does "while(1)" and breaks
-  // out as necessary.
-  bool done=false;
-  bool ok=true;
-
-  while(!done)
-    {
-      pkgAcquire::RunResult res=fetcher.Run(aptcfg->FindI(PACKAGE "::UI::Download-Poll-Interval", 50000));
-      if(res!=pkgAcquire::Continue || abortable.get_aborted())
-	// We failed or were cancelled
-	{
-	  ok=(res==pkgAcquire::Continue);
-	  break;
-	}
+  string main_log=aptcfg->Find(PACKAGE "::Log", "/var/log/" PACKAGE);
 
-      // Break out if download_only is on...not the best :(
-      if(download_only)
-	return true;
-
-      bool failed=false;
-      // Check for failures.  Viva la undocumentation!
-      for(pkgAcquireIterator i=fetcher.ItemsBegin();
-	  i!=fetcher.ItemsEnd();
-	  ++i)
-	{
-	  if((*i)->Status==pkgAcquire::Item::StatDone &&
-	     (*i)->Complete)
-	    continue;
+  if(!main_log.empty())
+    logs.push_back(main_log);
 
-	  if((*i)->Status==pkgAcquire::Item::StatIdle)
-	    continue;
+  const Configuration::Item *parent=aptcfg->Tree(PACKAGE "::Log");
 
-	  failed=true;
-	  break;
-	}
+  if(parent!=NULL)
+    for(const Configuration::Item *curr=parent->Child;
+	curr!=NULL; curr=curr->Next)
+      {
+	if(!curr->Value.empty())
+	  logs.push_back(curr->Value);
+      }
 
-      if(failed && !pm.FixMissing())
+  if(!logs.empty())
+    {
+      loglist changed_packages;
+      for(pkgCache::PkgIterator i=(*apt_cache_file)->PkgBegin(); !i.end(); i++)
 	{
-	  _error->Error(_("Unable to correct for unavailable packages"));
-	  ok=false;
-	  break;
+	  pkg_action_state s=find_pkg_state(i);
+	  if(s!=pkg_unchanged)
+	    changed_packages.push_back(logitem(i, s));
 	}
 
-      vector<string> logs;
+      changed_packages.sort(log_sorter());
 
-      string main_log=aptcfg->Find(PACKAGE "::Log", "/var/log/" PACKAGE);
+      for(vector<string>::iterator i=logs.begin(); i!=logs.end(); ++i)
+	do_log(*i, changed_packages);
+    }
+}
 
-      if(!main_log.empty())
-	logs.push_back(main_log);
+static
+install_run_result execute_install_run(OpProgress *load_progress,
+				       bool text_download,
+				       bool download_only,
+				       download_manager *manager,
+				       pkgAcquire *fetcher,
+				       pkgPackageManager *pm,
+				       pkgAcquire::RunResult res)
+{
+  if(res != pkgAcquire::Continue)
+    return install_failure;
+  else if(download_only)
+    return install_success;
 
-      const Configuration::Item *parent=aptcfg->Tree(PACKAGE "::Log");
+  bool failed=false;
+  for(pkgAcquireIterator i=fetcher->ItemsBegin();
+      i!=fetcher->ItemsEnd();
+      ++i)
+    {
+      if((*i)->Status==pkgAcquire::Item::StatDone &&
+	 (*i)->Complete)
+	continue;
 
-      if(parent!=NULL)
-	for(const Configuration::Item *curr=parent->Child;
-	    curr!=NULL; curr=curr->Next)
-	  {
-	    if(!curr->Value.empty())
-	      logs.push_back(curr->Value);
-	  }
+      if((*i)->Status==pkgAcquire::Item::StatIdle)
+	continue;
 
-      if(!logs.empty())
-	{
-	  loglist changed_packages;
-	  for(pkgCache::PkgIterator i=(*apt_cache_file)->PkgBegin(); !i.end(); i++)
-	    {
-	      pkg_action_state s=find_pkg_state(i);
-	      if(s!=pkg_unchanged)
-		changed_packages.push_back(logitem(i, s));
-	    }
-
-	  changed_packages.sort(log_sorter());
+      failed=true;
+      break;
+    }
 
-	  for(vector<string>::iterator i=logs.begin(); i!=logs.end(); ++i)
-	    do_log(*i, changed_packages);
-	}
+  if(failed && !pm->FixMissing())
+    {
+      _error->Error(_("Unable to correct for unavailable packages"));
+      return install_failure;
+    }
 
-      if(!text_download)
-	vscreen_suspend();
+  log_changes();
 
-      //  Ewww!  This is a race condition waiting to happen!
-      //  Ok, that's a little strong ;-)  But there's a potential problem if
-      // someone tries to lock the file in the miniscule amount of time between
-      // our releasing the lock and dpkg's taking it.  Not sure we can do much
-      // about this, though..
-      apt_cache_file->ReleaseLock();
+  if(!text_download)
+    vscreen_suspend();
 
-      switch(pm.DoInstall(aptcfg->FindI("APT::Status-Fd", -1)))
+  // Note that someone could grab the lock before dpkg takes it;
+  // without a more complicated synchronization protocol (and I don't
+  // control the code at dpkg's end), them's the breaks.
+  apt_cache_file->ReleaseLock();
+
+  install_run_result rval = install_success;
+
+  switch(pm->DoInstall(aptcfg->FindI("APT::Status-Fd", -1)))
+    {
+    case pkgPackageManager::Failed:
+      _error->DumpErrors();
+      cerr << _("A package failed to install.  Trying to recover:") << endl;
+      system("dpkg --configure -a");
+      _error->Discard();
+      
+      rval = install_failure;
+      // Fallthrough!
+    case pkgPackageManager::Completed:
+      if(!text_download)
 	{
-	case pkgPackageManager::Failed:
-	  _error->DumpErrors();
-	  cerr<<_("Ack!  Something bad happened while installing packages.  Trying to recover:")<<endl;
-	  // and this is really a hack:
-	  system("dpkg --configure -a");
-	  _error->Discard();
-	  ok=false;
-
-	  // Fallthrough!
-	case pkgPackageManager::Completed:
-	  if(!text_download)
-	    {
-	      cerr<<_("Press return to continue.\n");
-	      getchar();
-	    }
+	  cerr << _("Press return to continue.\n");
+	  getchar();
+	}
+
+      break;
 
-	  done=true;
+    case pkgPackageManager::Incomplete:
+      rval = install_do_again;
+      break;
+    }
 
-	  break;
 
-	case pkgPackageManager::Incomplete:
-	  break;
-	}
+  // libapt-pkg likes to stomp on SIGINT and SIGQUIT.  Restore them
+  // here in the simplest possible way.
+  vscreen_install_sighandlers();
 
+  if(!text_download)
+    vscreen_resume();
 
-      // libapt-pkg likes to stomp on SIGINT and SIGQUIT.  Restore them
-      // here in the simplest possible way.
-      vscreen_install_sighandlers();
+  fetcher->Shutdown();
 
-      if(!text_download)
-	vscreen_resume();
+  // Get source lists.
+  pkgSourceList src_list;
+  if(!src_list.ReadMainList())
+    {
+      _error->Error(_("Couldn't read source list"));
 
-      fetcher.Shutdown();
+      return install_failure;
+    }
 
-      if(!pm.GetArchives(&fetcher, &list, apt_package_records))
-	{
-	  ok=false;
-	  break;
-	}
+  if(!pm->GetArchives(fetcher, &src_list, apt_package_records))
+    return install_failure;
 
-      if(!apt_cache_file->GainLock())
-	// This really shouldn't happen.
-	{
-	  _error->Error(_("Could not regain the system lock!  (Perhaps another apt or dpkg is running?)"));
-	  ok=false;
-	  break;
-	}
+  if(!apt_cache_file->GainLock())
+    // This really shouldn't happen.
+    {
+      _error->Error(_("Could not regain the system lock!  (Perhaps another apt or dpkg is running?)"));
+      return install_failure;
     }
 
-  download_progress->Complete();
-  apt_reload_cache(load_progress, true);
+  return rval;
+}
 
-  if(aptcfg->FindB(PACKAGE "::Forget-New-On-Install", false))
-    do_forget_new();
+install_run_result finish_install_run(OpProgress *load_progress,
+				      bool text_download,
+				      bool download_only,
+				      download_manager *manager,
+				      pkgAcquire *fetcher,
+				      pkgPackageManager *pm,
+				      pkgAcquire::RunResult res)
+{
+  install_run_result run_res = execute_install_run(load_progress,
+						   text_download,
+						   download_only,
+						   manager,
+						   fetcher,
+						   pm,
+						   res);
 
-  delete download_progress;
+  if(run_res != install_do_again)
+    {
+      manager->Complete();
+      apt_reload_cache(load_progress, true);
+
+      if(aptcfg->FindB(PACKAGE "::Forget-New-On-Install", false))
+	do_forget_new();
+    }
 
-  return ok;
+  return run_res;
 }

Modified: branches/aptitude-0.3/aptitude/src/download.h
==============================================================================
--- branches/aptitude-0.3/aptitude/src/download.h	(original)
+++ branches/aptitude-0.3/aptitude/src/download.h	Sun Sep  4 18:09:06 2005
@@ -13,6 +13,7 @@
 
 class OpProgress;
 class pkgAcquire;
+class pkgPackageManager;
 class download_manager;
 
 /** Do the work to prepare an Acquire object for a package-list update
@@ -40,11 +41,80 @@
 			   pkgAcquire *fetcher,
 			   pkgAcquire::RunResult res);
 
-bool do_install_run(OpProgress *load_progress,
-		    bool text_download, bool download_only);
-// Installs packages.  load_progress is a progress object used to display the
-// progress when the package cache is reloaded.
-//
-// Returns true if everything completed successfully, false otherwise.
+
+/** Prepare for an installation run.
+ *
+ *  \param load_progress a progress bar used to display the progress
+ *         of saving the cache.
+ *
+ *  \param text_download if \b true, this is a command-line download;
+ *                       otherwise, the curses engine will be suspended
+ *                       and restarted.
+ *
+ *  \param download_only if \b true, the routine will exit as soon
+ *                       as the download completes.
+ *
+ *  \param log the status object with which the download will be
+ *             displayed.
+ *
+ *  \param fetcher an output parameter in which the new Acquire
+ *                 instance will be stored.
+ *
+ *  \param pm an output parameter in which the new package manager
+ *            object will be stored.
+ */
+bool prepare_install_run(OpProgress *load_progress,
+			 bool text_download,
+			 bool download_only,
+			 pkgAcquireStatus *log,
+			 pkgAcquire * &fetcher,
+			 pkgPackageManager * &pm);
+
+/** Indicates how an install run completed. */
+enum install_run_result
+  {
+    /** The install succeeded. */
+    install_success,
+
+    /** The install failed. */
+    install_failure,
+
+    /** The download-and-install sequence should be repeated; for
+     *  instance, because a different CD needs to be inserted into the
+     *  CD drive.
+     */
+    install_do_again
+  };
+
+/** Finish an install run after a download; call this from the main
+ *  thread.
+ *
+ *  \param load_progress a progress bar used to display the progres
+ *                       when re-loading the cache.
+ *
+ *  \param text_download if \b true, this is a command-line download.
+ *
+ *  \param download_only if \b true, the routine will exit as soon
+ *                       as the download completes.
+ *
+ *  \param manager the manager of this download
+ *
+ *  \param fetcher the acquire object created by prepare_install_run
+ *
+ *  \param pm the packagemanager object created by prepare_install_run
+ *
+ *  \param res the result of the install run
+ *
+ *  \return install_success or install_failure if the run was a 
+ *  full success or a failure; install_do_again if it should be
+ *  performed again (for instance, due to a media change).
+ */
+install_run_result finish_install_run(OpProgress *load_progress,
+				      bool text_download,
+				      bool download_only,
+				      download_manager *manager,
+				      pkgAcquire *fetcher,
+				      pkgPackageManager *pm,
+				      pkgAcquire::RunResult res);
 
 #endif

Modified: branches/aptitude-0.3/aptitude/src/ui.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/ui.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/ui.cc	Sun Sep  4 18:09:06 2005
@@ -30,6 +30,7 @@
 #include <apt-pkg/clean.h>
 #include <apt-pkg/configuration.h>
 #include <apt-pkg/error.h>
+#include <apt-pkg/packagemanager.h>
 
 #include <errno.h>
 #include <fcntl.h>
@@ -809,6 +810,29 @@
 // (er, can I disentangle this by rearranging the routines?  I think maybe I
 //  can to some degree)
 
+
+
+/** Used to indicate that a download was cancelled. */
+class downloader:public sigc::trackable
+{
+  bool aborted;
+public:
+  downloader() : aborted(false)
+  {
+  }
+
+  void abort()
+  {
+    aborted=true;
+  }
+
+  bool get_aborted()
+  {
+    return aborted;
+  }
+};
+
+
 void install_or_remove_packages()
 {
   active_download=true;
@@ -816,10 +840,38 @@
   if(active_preview.valid())
     active_preview->destroy();
 
-  vs_progress_ref p=gen_progress_bar();
+  vs_progress_ref p = gen_progress_bar();
 
-  do_install_run(p.unsafe_get_ref(), false, false);
+  downloader abort_state;
 
+  download_manager *m = gen_download_progress(false,
+					      _("Downloading packages"),
+					      _("View the progress of the package download"),
+					      _("Package Download"),
+					      arg(sigc::mem_fun(abort_state,
+								&downloader::abort)));
+
+  pkgAcquire *acq;
+  pkgPackageManager *pm;
+  bool ok = prepare_install_run(p.unsafe_get_ref(),
+				false, false, m, acq, pm);
+
+  if(ok)
+    {
+      install_run_result run_res;
+
+      do
+	{
+	  pkgAcquire::RunResult res = acq->Run();
+	  run_res = finish_install_run(p.unsafe_get_ref(),
+				       false, false, m,
+				       acq, pm, res);
+	} while(run_res == install_do_again &&
+		!abort_state.get_aborted());
+    }
+
+  delete acq;
+  delete pm;
   p->destroy();
 
   active_download=false;



More information about the Aptitude-svn-commit mailing list