[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#728153: apt-cdrom should succeed if any drive succeeds



Package: apt
Version: 0.9.14.1
Severity: important
Tags: patch

[PATCH] apt-cdrom: succeed if any drive succeeds

If there are multiple CDROM drives, `apt-cdrom add` will abort with an
error if any of the drives do not contain a Debian CD. This is
particularly a problem if apt-cdrom happens to check a drive with no CD
first. Then it will abort without even searching the other drives.

This patch modifies apt-cdrom to return success if any of the drives
succeeded. If failures occur, apt-cdrom will still continue trying all
the drives and report the last failure (if none of them succeeded).

The 'ident' command was also changed to match the new 'add' behavior.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
The version in sid (0.9.14.1) has the exact same problem. I've modified
the patch for this version. I also implemented David's recommendation to
print out all warnings/errors. Note that the program will return with
exit code 0 if at least one drive is successful. This is important.

I also made the changes for ident. ident was much more broken for
multi-drives than add. ident now behaves exactly like add.

 apt-pkg/cdrom.cc     |   26 ++++++++++++--
 cmdline/apt-cdrom.cc |   76 +++++++++++++++++++++++++++++++----------
 2 files changed, 83 insertions(+), 19 deletions(-)

diff -pur a/apt-pkg/cdrom.cc b/apt-pkg/cdrom.cc
--- a/apt-pkg/cdrom.cc
+++ b/apt-pkg/cdrom.cc
@@ -577,8 +577,30 @@ bool pkgCdrom::Ident(string &ident, pkgC
 		      CDROM.c_str());
       log->Update(msg.str());
    }
-   if (MountCdrom(CDROM) == false)
-      return _error->Error("Failed to mount the cdrom.");
+
+   // Unmount the CD and get the user to put in the one they want
+   if (_config->FindB("APT::CDROM::NoMount",false) == false)
+   {
+      if(log != NULL)
+	 log->Update(_("Unmounting CD-ROM\n"), STEP_UNMOUNT);
+      UnmountCdrom(CDROM);
+
+      if(log != NULL)
+      {
+	 log->Update(_("Waiting for disc...\n"), STEP_WAIT);
+	 if(!log->ChangeCdrom()) {
+	    // user aborted
+	    return false; 
+	 }
+      }
+
+      // Mount the new CDROM
+      if(log != NULL)
+	 log->Update(_("Mounting CD-ROM...\n"), STEP_MOUNT);
+
+      if (MountCdrom(CDROM) == false)
+	 return _error->Error("Failed to mount the cdrom.");
+   }
 
    // Hash the CD to get an ID
    if (log != NULL)
diff -pur a/cmdline/apt-cdrom.cc b/cmdline/apt-cdrom.cc
--- a/cmdline/apt-cdrom.cc
+++ b/cmdline/apt-cdrom.cc
@@ -111,10 +111,12 @@ OpProgress* pkgCdromTextStatus::GetOpPro
 };
 									/*}}}*/
 // SetupAutoDetect       						/*{{{*/
-bool AutoDetectCdrom(pkgUdevCdromDevices &UdevCdroms, unsigned int &i)
+bool AutoDetectCdrom(pkgUdevCdromDevices &UdevCdroms, unsigned int &i, bool &automounted)
 {
    bool Debug =  _config->FindB("Debug::Acquire::cdrom", false);
 
+   automounted = false;
+
    vector<struct CdromDevice> v = UdevCdroms.Scan();
    if (i >= v.size())
       return false;
@@ -137,6 +139,8 @@ bool AutoDetectCdrom(pkgUdevCdromDevices
 	 mkdir(AptMountPoint.c_str(), 0750);
       if(MountCdrom(AptMountPoint, v[i].DeviceName) == false)
 	 _error->Warning(_("Failed to mount '%s' to '%s'"), v[i].DeviceName.c_str(), AptMountPoint.c_str());
+      else
+	 automounted = true;
       _config->Set("Acquire::cdrom::mount", AptMountPoint);
       _config->Set("APT::CDROM::NoMount", true);
    }
@@ -160,17 +164,35 @@ bool DoAdd(CommandLine &)
 
    bool AutoDetect = _config->FindB("Acquire::cdrom::AutoDetect", true);
    unsigned int count = 0;
+   string AptMountPoint = _config->FindDir("Dir::Media::MountPath");
+   bool automounted = false;
    if (AutoDetect && UdevCdroms.Dlopen())
-      while (AutoDetectCdrom(UdevCdroms, count))
-	 res &= cdrom.Add(&log);
-   if (count == 0) {
-      res = cdrom.Add(&log);
-      if (res == false) {
-         _error->Error("%s", _(W_NO_CDROM_FOUND));
+      while (AutoDetectCdrom(UdevCdroms, count, automounted)) {
+	 if (count == 1) {
+	    // begin loop with res false to detect any success using OR
+	    res = false;
+	 }
+
+	 // dump any warnings/errors from autodetect
+	 if (_error->empty() == false)
+	    _error->DumpErrors();
+
+	 res |= cdrom.Add(&log);
+
+	 if (automounted)
+	    UnmountCdrom(AptMountPoint);
+
+	 // dump any warnings/errors from add/unmount
+	 if (_error->empty() == false)
+	    _error->DumpErrors();
       }
-   }
 
-   if(res)
+   if (count == 0)
+      res = cdrom.Add(&log);
+
+   if (res == false)
+      _error->Error("%s", _(W_NO_CDROM_FOUND));
+   else
       cout << _("Repeat this process for the rest of the CDs in your set.") << endl;
 
    return res;
@@ -187,18 +209,38 @@ bool DoIdent(CommandLine &)
    pkgCdrom cdrom;
    bool res = true;
 
-   bool AutoDetect = _config->FindB("Acquire::cdrom::AutoDetect");
+   bool AutoDetect = _config->FindB("Acquire::cdrom::AutoDetect", true);
 
    unsigned int count = 0;
+   string AptMountPoint = _config->FindDir("Dir::Media::MountPath");
+   bool automounted = false;
    if (AutoDetect && UdevCdroms.Dlopen())
-      while (AutoDetectCdrom(UdevCdroms, count))
-	 res &= cdrom.Ident(ident, &log);
-   if (count == 0) {
-      res = cdrom.Ident(ident, &log);
-      if (res == false) {
-         _error->Error("%s", _(W_NO_CDROM_FOUND));
+      while (AutoDetectCdrom(UdevCdroms, count, automounted)) {
+	 if (count == 1) {
+	    // begin loop with res false to detect any success using OR
+	    res = false;
+	 }
+
+	 // dump any warnings/errors from autodetect
+	 if (_error->empty() == false)
+	    _error->DumpErrors();
+
+	 res |= cdrom.Ident(ident, &log);
+
+	 if (automounted)
+	    UnmountCdrom(AptMountPoint);
+
+	 // dump any warnings/errors from add/unmount
+	 if (_error->empty() == false)
+	    _error->DumpErrors();
       }
-   }
+
+   if (count == 0)
+      res = cdrom.Ident(ident, &log);
+
+   if (res == false)
+      _error->Error("%s", _(W_NO_CDROM_FOUND));
+
    return res;
 }
 									/*}}}*/


Reply to: