provide public interface to hold/unhold packages

We had this code lying around in apt-mark for a while now, but other
frontends need this (and similar) functionality as well, so its high
time that we provide a public interface in libapt for this stuff.

7 files changed, 219 insertions, 153 deletions

diff --git a/apt-pkg/deb/debsystem.cc b/apt-pkg/deb/debsystem.cc
index d6ef49a37..dcd115b50 100644
--- a/apt-pkg/deb/debsystem.cc
+++ b/apt-pkg/deb/debsystem.cc
@@ -302,7 +302,7 @@ void debSystem::DpkgChrootDirectory()					/*{{{*/
       _exit(100);
 }
 									/*}}}*/
-static pid_t ExecDpkg(std::vector<std::string> const &sArgs, int * const inputFd, int * const outputFd, bool const showStderr)/*{{{*/
+pid_t debSystem::ExecDpkg(std::vector<std::string> const &sArgs, int * const inputFd, int * const outputFd, bool const DiscardOutput)/*{{{*/
 {
    std::vector<const char *> Args(sArgs.size(), NULL);
    std::transform(sArgs.begin(), sArgs.end(), Args.begin(), [](std::string const &s) { return s.c_str(); });
@@ -333,7 +333,7 @@ static pid_t ExecDpkg(std::vector<std::string> const &sArgs, int * const inputFd
 	 close(external[0]);
 	 dup2(external[1], STDOUT_FILENO);
       }
-      if (showStderr == false)
+      if (DiscardOutput == true)
 	 dup2(nullfd, STDERR_FILENO);
       debSystem::DpkgChrootDirectory();
       execvp(Args[0], (char**) &Args[0]);
@@ -357,7 +357,7 @@ bool debSystem::SupportsMultiArch()					/*{{{*/
 {
    std::vector<std::string> Args = GetDpkgBaseCommand();
    Args.push_back("--assert-multi-arch");
-   pid_t const dpkgAssertMultiArch = ExecDpkg(Args, nullptr, nullptr, false);
+   pid_t const dpkgAssertMultiArch = ExecDpkg(Args, nullptr, nullptr, true);
    if (dpkgAssertMultiArch > 0)
    {
       int Status = 0;
@@ -386,7 +386,7 @@ std::vector<std::string> debSystem::SupportedArchitectures()		/*{{{*/
    std::vector<std::string> sArgs = GetDpkgBaseCommand();
    sArgs.push_back("--print-foreign-architectures");
    int outputFd = -1;
-   pid_t const dpkgMultiArch = ExecDpkg(sArgs, nullptr, &outputFd, false);
+   pid_t const dpkgMultiArch = ExecDpkg(sArgs, nullptr, &outputFd, true);
    if (dpkgMultiArch == -1)
       return archs;
 
diff --git a/apt-pkg/deb/debsystem.h b/apt-pkg/deb/debsystem.h
index efb33a3ed..5185c92d8 100644
--- a/apt-pkg/deb/debsystem.h
+++ b/apt-pkg/deb/debsystem.h
@@ -34,7 +34,7 @@ class debSystem : public pkgSystem
    public:
 
    virtual bool Lock() APT_OVERRIDE;
-   virtual bool UnLock(bool NoErrors = false) APT_OVERRIDE;   
+   virtual bool UnLock(bool NoErrors = false) APT_OVERRIDE;
    virtual pkgPackageManager *CreatePM(pkgDepCache *Cache) const APT_OVERRIDE;
    virtual bool Initialize(Configuration &Cnf) APT_OVERRIDE;
    virtual bool ArchiveSupported(const char *Type) APT_OVERRIDE;
@@ -49,6 +49,7 @@ class debSystem : public pkgSystem
    APT_HIDDEN static std::string GetDpkgExecutable();
    APT_HIDDEN static std::vector<std::string> GetDpkgBaseCommand();
    APT_HIDDEN static void DpkgChrootDirectory();
+   APT_HIDDEN static pid_t ExecDpkg(std::vector<std::string> const &sArgs, int * const inputFd, int * const outputFd, bool const DiscardOutput);
    APT_HIDDEN static bool SupportsMultiArch();
    APT_HIDDEN static std::vector<std::string> SupportedArchitectures();
 };
diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc
index bea4bec89..4f953bd50 100644
--- a/apt-pkg/policy.cc
+++ b/apt-pkg/policy.cc
@@ -222,8 +222,8 @@ pkgCache::VerIterator pkgPolicy::GetCandidateVer(pkgCache::PkgIterator const &Pk
 
    return Pref;
 }
-
-// Policy::GetCandidateVer - Get the candidate install version		/*{{{*/
+									/*}}}*/
+// Policy::GetCandidateVerNew - Get the candidate install version	/*{{{*/
 // ---------------------------------------------------------------------
 /* Evaluate the package pins and the default list to deteremine what the
    best package is. */
diff --git a/apt-pkg/statechanges.cc b/apt-pkg/statechanges.cc
new file mode 100644
index 000000000..a20319d2d
--- /dev/null
+++ b/apt-pkg/statechanges.cc
@@ -0,0 +1,130 @@
+#include <apt-pkg/pkgcache.h>
+#include <apt-pkg/cacheset.h>
+#include <apt-pkg/debsystem.h>
+#include <apt-pkg/fileutl.h>
+#include <apt-pkg/statechanges.h>
+
+#include <algorithm>
+#include <memory>
+
+namespace APT
+{
+
+class StateChanges::Private
+{
+public:
+   APT::VersionVector hold;
+   APT::VersionVector install;
+   APT::VersionVector error;
+};
+
+void StateChanges::Hold(pkgCache::VerIterator const &Ver)
+{
+   d->hold.push_back(Ver);
+}
+APT::VersionVector& StateChanges::Hold()
+{
+   return d->hold;
+}
+void StateChanges::Unhold(pkgCache::VerIterator const &Ver)
+{
+   d->install.push_back(Ver);
+}
+APT::VersionVector& StateChanges::Unhold()
+{
+   return d->install;
+}
+APT::VersionVector& StateChanges::Error()
+{
+   return d->error;
+}
+
+void StateChanges::Discard()
+{
+   d->hold.clear();
+   d->install.clear();
+   d->error.clear();
+}
+
+bool StateChanges::Save(bool const DiscardOutput)
+{
+   d->error.clear();
+   if (d->hold.empty() && d->install.empty())
+      return true;
+
+   std::vector<std::string> Args = debSystem::GetDpkgBaseCommand();
+   // ensure dpkg knows about the package so that it keeps the status we set
+   {
+      APT::VersionVector makeDpkgAvailable;
+      auto const notInstalled = [](pkgCache::VerIterator const &V) { return V.ParentPkg()->CurrentVer == 0; };
+      std::copy_if(d->hold.begin(), d->hold.end(), std::back_inserter(makeDpkgAvailable), notInstalled);
+      std::copy_if(d->install.begin(), d->install.end(), std::back_inserter(makeDpkgAvailable), notInstalled);
+
+      if (makeDpkgAvailable.empty() == false)
+      {
+	 auto const BaseArgs = Args.size();
+	 Args.push_back("--merge-avail");
+	 // FIXME: supported only since 1.17.7 in dpkg
+	 Args.push_back("-");
+	 int dummyAvail = -1;
+	 pid_t const dpkgMergeAvail = debSystem::ExecDpkg(Args, &dummyAvail, nullptr, true);
+
+	 FILE* dpkg = fdopen(dummyAvail, "w");
+	 for (auto const &V: makeDpkgAvailable)
+	    fprintf(dpkg, "Package: %s\nVersion: 0~\nArchitecture: %s\nMaintainer: Dummy Example <dummy@example.org>\n"
+		  "Description: dummy package record\n A record is needed to put a package on hold, so here it is.\n\n", V.ParentPkg().Name(), V.Arch());
+	 fclose(dpkg);
+
+	 ExecWait(dpkgMergeAvail, "dpkg --merge-avail", true);
+	 Args.erase(Args.begin() + BaseArgs, Args.end());
+      }
+   }
+   bool const dpkgMultiArch = _system->MultiArchSupported();
+
+   Args.push_back("--set-selections");
+   int selections = -1;
+   pid_t const dpkgSelections = debSystem::ExecDpkg(Args, &selections, nullptr, DiscardOutput);
+
+   FILE* dpkg = fdopen(selections, "w");
+   std::string state;
+   auto const dpkgName = [&](pkgCache::VerIterator const &V) {
+      pkgCache::PkgIterator P = V.ParentPkg();
+      if (dpkgMultiArch == false)
+	 fprintf(dpkg, "%s %s\n", P.FullName(true).c_str(), state.c_str());
+      else
+	 fprintf(dpkg, "%s:%s %s\n", P.Name(), V.Arch(), state.c_str());
+   };
+   if (d->hold.empty() == false)
+   {
+      state = "hold";
+      std::for_each(d->hold.begin(), d->hold.end(), dpkgName);
+   }
+   if (d->install.empty() == false)
+   {
+      state = "install";
+      std::for_each(d->install.begin(), d->install.end(), dpkgName);
+   }
+   fclose(dpkg);
+
+   if (ExecWait(dpkgSelections, "dpkg --set-selections") == false)
+   {
+      if (d->hold.empty())
+	 std::swap(d->install, d->error);
+      else if (d->install.empty())
+	 std::swap(d->hold, d->error);
+      else
+      {
+	 std::swap(d->hold, d->error);
+	 std::move(d->install.begin(), d->install.end(), std::back_inserter(d->error));
+	 d->install.clear();
+      }
+   }
+   return d->error.empty();
+}
+
+StateChanges::StateChanges() : d(new StateChanges::Private()) {}
+StateChanges::StateChanges(StateChanges&&) = default;
+StateChanges& StateChanges::operator=(StateChanges&&) = default;
+StateChanges::~StateChanges() = default;
+
+}
diff --git a/apt-pkg/statechanges.h b/apt-pkg/statechanges.h
new file mode 100644
index 000000000..fa60c5864
--- /dev/null
+++ b/apt-pkg/statechanges.h
@@ -0,0 +1,52 @@
+#include <apt-pkg/pkgcache.h>
+#include <apt-pkg/cacheset.h>
+#include <apt-pkg/macros.h>
+
+#include <memory>
+
+namespace APT
+{
+
+/** Simple wrapper class to abstract away the differences in storing different
+ *  states in different places potentially in different versions.
+ */
+class APT_PUBLIC StateChanges
+{
+public:
+   // getter/setter for the different states
+   APT::VersionVector& Hold();
+   void Hold(pkgCache::VerIterator const &Ver);
+   APT::VersionVector& Unhold();
+   void Unhold(pkgCache::VerIterator const &Ver);
+   APT::VersionVector& Error();
+
+   // forgets all unsaved changes
+   void Discard();
+
+   /** commit the staged changes to the database(s).
+    *
+    * Makes the needed calls to store the requested states.
+    * After this call the state containers will hold only versions
+    * for which the storing operation succeeded. Versions where the
+    * storing operation failed are collected in #Error(). Note that
+    * error is an upper bound as states are changed in batches so it
+    * isn't always clear which version triggered the failure exactly.
+    *
+    * @param DiscardOutput controls if stdout/stderr should be used
+    *   by subprocesses for (detailed) error reporting if needed.
+    * @return \b false if storing failed, true otherwise.
+    *   Note that some states might be applied even if the whole operation failed.
+    */
+   bool Save(bool const DiscardOutput = false);
+
+   StateChanges();
+   StateChanges(StateChanges&&);
+   StateChanges& operator=(StateChanges&&);
+   ~StateChanges();
+
+private:
+   class APT_HIDDEN Private;
+   std::unique_ptr<Private> d;
+};
+
+}
diff --git a/cmdline/apt-mark.cc b/cmdline/apt-mark.cc
index ef3a0d72f..9d1d0863e 100644
--- a/cmdline/apt-mark.cc
+++ b/cmdline/apt-mark.cc
@@ -15,6 +15,7 @@
 #include <apt-pkg/init.h>
 #include <apt-pkg/pkgsystem.h>
 #include <apt-pkg/strutl.h>
+#include <apt-pkg/statechanges.h>
 #include <apt-pkg/cacheiterators.h>
 #include <apt-pkg/configuration.h>
 #include <apt-pkg/depcache.h>
@@ -185,8 +186,6 @@ static bool DoHold(CommandLine &CmdL)
 
    auto const doneBegin = MarkHold ? pkgset.begin() : part;
    auto const doneEnd = MarkHold ? part : pkgset.end();
-   auto const changeBegin = MarkHold ? part : pkgset.begin();
-   auto const changeEnd = MarkHold ? pkgset.end() : part;
 
    std::for_each(doneBegin, doneEnd, [&MarkHold](pkgCache::VerIterator const &V) {
       if (MarkHold == true)
@@ -198,156 +197,27 @@ static bool DoHold(CommandLine &CmdL)
    if (doneBegin == pkgset.begin() && doneEnd == pkgset.end())
       return true;
 
-   if (_config->FindB("APT::Mark::Simulate", false) == true)
-   {
-      std::for_each(changeBegin, changeEnd, [&MarkHold](pkgCache::VerIterator const &V) {
-        if (MarkHold == false)
-           ioprintf(c1out, _("%s set on hold.\n"), V.ParentPkg().FullName(true).c_str());
-        else
-           ioprintf(c1out, _("Canceled hold on %s.\n"), V.ParentPkg().FullName(true).c_str());
-      });
-      return true;
-   }
-
-   // Generate the base argument list for dpkg
-   std::vector<const char *> Args;
-   string Tmp = _config->Find("Dir::Bin::dpkg","dpkg");
-   {
-      string const dpkgChrootDir = _config->FindDir("DPkg::Chroot-Directory", "/");
-      size_t dpkgChrootLen = dpkgChrootDir.length();
-      if (dpkgChrootDir != "/" && Tmp.find(dpkgChrootDir) == 0)
-      {
-	 if (dpkgChrootDir[dpkgChrootLen - 1] == '/')
-	    --dpkgChrootLen;
-	 Tmp = Tmp.substr(dpkgChrootLen);
-      }
-   }
-   Args.push_back(Tmp.c_str());
-
-   // Stick in any custom dpkg options
-   Configuration::Item const *Opts = _config->Tree("DPkg::Options");
-   if (Opts != 0)
-   {
-      Opts = Opts->Child;
-      for (; Opts != 0; Opts = Opts->Next)
-      {
-	 if (Opts->Value.empty() == true)
-	    continue;
-	 Args.push_back(Opts->Value.c_str());
-      }
-   }
-
-   APT::VersionVector keepoffset;
-   std::copy_if(changeBegin, changeEnd, std::back_inserter(keepoffset),
-	 [](pkgCache::VerIterator const &V) { return V.ParentPkg()->CurrentVer == 0; });
-
-   if (keepoffset.empty() == false)
-   {
-      size_t const BaseArgs = Args.size();
-      Args.push_back("--merge-avail");
-      // FIXME: supported only since 1.17.7 in dpkg
-      Args.push_back("-");
-      Args.push_back(NULL);
-
-      int external[2] = {-1, -1};
-      if (pipe(external) != 0)
-	 return _error->WarningE("DoHold", "Can't create IPC pipe for dpkg --merge-avail");
-
-      pid_t dpkgMergeAvail = ExecFork();
-      if (dpkgMergeAvail == 0)
-      {
-	 close(external[1]);
-	 std::string const chrootDir = _config->FindDir("DPkg::Chroot-Directory");
-	 if (chrootDir != "/" && chroot(chrootDir.c_str()) != 0 && chdir("/") != 0)
-	    _error->WarningE("getArchitecture", "Couldn't chroot into %s for dpkg --merge-avail", chrootDir.c_str());
-	 dup2(external[0], STDIN_FILENO);
-	 int const nullfd = open("/dev/null", O_RDONLY);
-	 dup2(nullfd, STDOUT_FILENO);
-	 execvp(Args[0], (char**) &Args[0]);
-	 _error->WarningE("dpkgGo", "Can't get dpkg --merge-avail running!");
-	 _exit(2);
-      }
-
-      FILE* dpkg = fdopen(external[1], "w");
-      for (auto const &V: keepoffset)
-	 fprintf(dpkg, "Package: %s\nVersion: 0~\nArchitecture: %s\nMaintainer: Dummy Example <dummy@example.org>\n"
-	       "Description: dummy package record\n A record is needed to put a package on hold, so here it is.\n\n", V.ParentPkg().Name(), V.Arch());
-      fclose(dpkg);
-      keepoffset.clear();
-
-      if (dpkgMergeAvail > 0)
-      {
-	 int Status = 0;
-	 while (waitpid(dpkgMergeAvail, &Status, 0) != dpkgMergeAvail)
-	 {
-	    if (errno == EINTR)
-	       continue;
-	    _error->WarningE("dpkgGo", _("Waited for %s but it wasn't there"), "dpkg --merge-avail");
-	    break;
-	 }
-	 if (WIFEXITED(Status) == false || WEXITSTATUS(Status) != 0)
-	    return _error->Error(_("Executing dpkg failed. Are you root?"));
-      }
-      Args.erase(Args.begin() + BaseArgs, Args.end());
-   }
-
-   Args.push_back("--set-selections");
-   Args.push_back(NULL);
+   auto const changeBegin = MarkHold ? part : pkgset.begin();
+   auto const changeEnd = MarkHold ? pkgset.end() : part;
 
-   int external[2] = {-1, -1};
-   if (pipe(external) != 0)
-      return _error->WarningE("DoHold", "Can't create IPC pipe for dpkg --set-selections");
+   APT::StateChanges marks;
+   std::move(changeBegin, changeEnd, std::back_inserter(MarkHold ? marks.Hold() : marks.Unhold()));
+   pkgset.clear();
 
-   pid_t dpkgSelection = ExecFork();
-   if (dpkgSelection == 0)
+   bool success = true;
+   if (_config->FindB("APT::Mark::Simulate", false) == false)
    {
-      close(external[1]);
-      std::string const chrootDir = _config->FindDir("DPkg::Chroot-Directory");
-      if (chrootDir != "/" && chroot(chrootDir.c_str()) != 0 && chdir("/") != 0)
-	 _error->WarningE("getArchitecture", "Couldn't chroot into %s for dpkg --set-selections", chrootDir.c_str());
-      dup2(external[0], STDIN_FILENO);
-      execvp(Args[0], (char**) &Args[0]);
-      _error->WarningE("dpkgGo", "Can't get dpkg --set-selections running!");
-      _exit(2);
+      success = marks.Save();
+      if (success == false)
+	 _error->Error(_("Executing dpkg failed. Are you root?"));
    }
 
-   bool const dpkgMultiArch = _system->MultiArchSupported();
-   FILE* dpkg = fdopen(external[1], "w");
-   for (auto Ver = changeBegin; Ver != changeEnd; ++Ver)
-   {
-      pkgCache::PkgIterator P = Ver.ParentPkg();
-      if (dpkgMultiArch == false)
-	 fprintf(dpkg, "%s", P.FullName(true).c_str());
-      else
-	 fprintf(dpkg, "%s:%s", P.Name(), Ver.Arch());
+   for (auto Ver : marks.Hold())
+      ioprintf(c1out,_("%s set on hold.\n"), Ver.ParentPkg().FullName(true).c_str());
+   for (auto Ver : marks.Unhold())
+      ioprintf(c1out,_("Canceled hold on %s.\n"), Ver.ParentPkg().FullName(true).c_str());
 
-      if (MarkHold == true)
-      {
-	 fprintf(dpkg, " hold\n");
-	 ioprintf(c1out,_("%s set on hold.\n"), P.FullName(true).c_str());
-      }
-      else
-      {
-	 fprintf(dpkg, " install\n");
-	 ioprintf(c1out,_("Canceled hold on %s.\n"), P.FullName(true).c_str());
-      }
-   }
-   fclose(dpkg);
-
-   if (dpkgSelection > 0)
-   {
-      int Status = 0;
-      while (waitpid(dpkgSelection, &Status, 0) != dpkgSelection)
-      {
-	 if (errno == EINTR)
-	    continue;
-	 _error->WarningE("dpkgGo", _("Waited for %s but it wasn't there"), "dpkg --set-selection");
-	 break;
-      }
-      if (WIFEXITED(Status) == true && WEXITSTATUS(Status) == 0)
-	 return true;
-   }
-   return _error->Error(_("Executing dpkg failed. Are you root?"));
+   return success;
 }
 									/*}}}*/
 /* ShowHold - show packages set on hold in dpkg status			{{{*/
diff --git a/test/integration/test-apt-mark b/test/integration/test-apt-mark
index 9b68945f9..ec4ed8316 100755
--- a/test/integration/test-apt-mark
+++ b/test/integration/test-apt-mark
@@ -99,3 +99,16 @@ testmarkonepkgashold 'uninstalled-native'
 
 testsuccessequal 'uninstalled set on hold.' aptmark hold uninstalled
 testsuccessequal 'uninstalled-native set on hold.' aptmark hold uninstalled-native
+#FIXME: holds on uninstalled packages are not persistent in dpkg
+testsuccessequal 'Reading package lists...
+Building dependency tree...
+Reading state information...
+The following NEW packages will be installed:
+  uninstalled uninstalled-native
+The following held packages will be changed:
+  uninstalled-native
+0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded.
+Inst uninstalled (1 unstable [all])
+Inst uninstalled-native (1 unstable [amd64])
+Conf uninstalled (1 unstable [all])
+Conf uninstalled-native (1 unstable [amd64])' aptget install uninstalled uninstalled-native -s