edsp: try 2 to read responses even if writing failed

Commit b60c8a89c281f2bb945d426d2215cbf8f5760738 improved the situation,
but due to inconsistency mostly for planners, not for solvers. As the
idea of hiding errors if we show another error is a bit scary (as the
extern error might be a followup of our intern error, rather than the
reason for our intern error as it is at the moment) we don't discard the
errors, but if we got an extern error we show them directly removing
them from the error list at the end of the run – that list will contain
the extern error which hopefully gives us the best of both worlds.

The problem itself is the same as before: The externals exiting before
apt is done talking to them.

Reported-By: Johannes 'josch' Schauer on IRC

diff --git a/apt-pkg/edsp.cc b/apt-pkg/edsp.cc
index f56625feb5e9044e04ac5a07fd41bf38d5798731..b80b9a45679963899b26e12458a07e6e213bfc4f 100644 (file)
--- a/apt-pkg/edsp.cc
+++ b/apt-pkg/edsp.cc
@@ -648,13 +648,19 @@ bool EDSP::ReadResponse(int const input, pkgDepCache &Cache, OpProgress *Progres
                        }
                        continue;
                } else if (section.Exists("Error") == true) {
+                       if (_error->PendingError()) {
+                               if (Progress != nullptr)
+                                       Progress->Done();
+                               Progress = nullptr;
+                               _error->DumpErrors(std::cerr, GlobalError::DEBUG, false);
+                       }
                        std::string msg = SubstVar(SubstVar(section.FindS("Message"), "\n .\n", "\n\n"), "\n ", "\n");
                        if (msg.empty() == true) {
                                msg = _("External solver failed without a proper error message");
                                _error->Error("%s", msg.c_str());
                        } else
                                _error->Error("External solver failed with: %s", msg.substr(0,msg.find('\n')).c_str());
-                       if (Progress != NULL)
+                       if (Progress != nullptr)
                                Progress->Done();
                        std::cerr << "The solver encountered an error of type: " << section.FindS("Error") << std::endl;
                        std::cerr << "The following information might help you to understand what is wrong:" << std::endl;
@@ -1051,8 +1057,9 @@ bool EDSP::ResolveExternal(const char* const solver, pkgDepCache &Cache,
                Okay &= EDSP::WriteRequest(Cache, output, flags, nullptr);
                return Okay && EDSP::WriteScenario(Cache, output, nullptr);
        }
+       _error->PushToStack();
        int solver_in, solver_out;
-       pid_t const solver_pid = EDSP::ExecuteSolver(solver, &solver_in, &solver_out, true);
+       pid_t const solver_pid = ExecuteSolver(solver, &solver_in, &solver_out, true);
        if (solver_pid == 0)
                return false;
 
@@ -1071,11 +1078,11 @@ bool EDSP::ResolveExternal(const char* const solver, pkgDepCache &Cache,
 
        if (Okay && Progress != NULL)
                Progress->OverallProgress(25, 100, 75, _("Execute external solver"));
-       if (Okay && EDSP::ReadResponse(solver_out, Cache, Progress) == false)
-               return false;
-
-       bool const waited = ExecWait(solver_pid, solver);
-       return Okay && waited;
+       bool const ret = EDSP::ReadResponse(solver_out, Cache, Progress);
+       _error->MergeWithStack();
+       if (ExecWait(solver_pid, solver))
+               return ret;
+       return false;
 }
 bool EDSP::ResolveExternal(const char* const solver, pkgDepCache &Cache,
                         bool const upgrade, bool const distUpgrade,
@@ -1108,7 +1115,7 @@ bool EIPP::OrderInstall(char const * const solver, pkgPackageManager * const PM,
       Okay &= EIPP::WriteRequest(PM->Cache, output, flags, nullptr);
       return Okay && EIPP::WriteScenario(PM->Cache, output, nullptr);
    }
-
+   _error->PushToStack();
    int solver_in, solver_out;
    pid_t const solver_pid = ExecuteExternal("planner", solver, "Dir::Bin::Planners", &solver_in, &solver_out);
    if (solver_pid == 0)
@@ -1139,12 +1146,11 @@ bool EIPP::OrderInstall(char const * const solver, pkgPackageManager * const PM,
            PM->Remove(Pkg, true);
       }
    }
-
-   if (EIPP::ReadResponse(solver_out, PM, Progress) == false)
-      return false;
-
-   bool const waited = ExecWait(solver_pid, solver);
-   return Okay && waited;
+   bool const ret = EIPP::ReadResponse(solver_out, PM, Progress);
+   _error->MergeWithStack();
+   if (ExecWait(solver_pid, solver))
+      return ret;
+   return false;
 }
                                                                        /*}}}*/
 bool EIPP::WriteRequest(pkgDepCache &Cache, FileFd &output,            /*{{{*/
@@ -1341,13 +1347,19 @@ bool EIPP::ReadResponse(int const input, pkgPackageManager * const PM, OpProgres
         }
         continue;
       } else if (section.Exists("Error") == true) {
+        if (_error->PendingError()) {
+           if (Progress != nullptr)
+              Progress->Done();
+           Progress = nullptr;
+           _error->DumpErrors(std::cerr, GlobalError::DEBUG, false);
+        }
         std::string msg = SubstVar(SubstVar(section.FindS("Message"), "\n .\n", "\n\n"), "\n ", "\n");
         if (msg.empty() == true) {
            msg = _("External planner failed without a proper error message");
            _error->Error("%s", msg.c_str());
         } else
            _error->Error("External planner failed with: %s", msg.substr(0,msg.find('\n')).c_str());
-        if (Progress != NULL)
+        if (Progress != nullptr)
            Progress->Done();
         std::cerr << "The planner encountered an error of type: " << section.FindS("Error") << std::endl;
         std::cerr << "The following information might help you to understand what is wrong:" << std::endl;

diff --git a/apt-private/private-install.cc b/apt-private/private-install.cc
index aa28780da7b802333bf14622536e3208f23f51d8..fab742dc7244c167765c9564ad8dc5c0f4a67bdf 100644 (file)
--- a/apt-private/private-install.cc
+++ b/apt-private/private-install.cc
@@ -48,6 +48,10 @@ bool CheckNothingBroken(CacheFile &Cache)                            /*{{{*/
    if (Cache->BrokenCount() == 0)
       return true;
 
+   // FIXME: if an external solver showed an error, we shouldn't show one here
+   if (_error->PendingError() && _config->Find("APT::Solver") == "dump")
+      return false;
+
    c1out <<
       _("Some packages could not be installed. This may mean that you have\n"
            "requested an impossible situation or if you are using the unstable\n"

diff --git a/test/integration/test-external-dependency-solver-protocol b/test/integration/test-external-dependency-solver-protocol
index 17a30cf2f76a3d6825941992568af8d404de938a..d126cd84b4c87cea1571c15be77df97daf419216 100755 (executable)
--- a/test/integration/test-external-dependency-solver-protocol
+++ b/test/integration/test-external-dependency-solver-protocol
@@ -262,3 +262,24 @@ EOF
 chmod +x rootdir/usr/lib/apt/solvers/installall
 testfailure apt full-upgrade -s --solver installall
 testsuccess grep "is already installed!" rootdir/tmp/testfailure.output
+
+testsolverfailuremsg() {
+       local SOLVER="rootdir/usr/lib/apt/solvers/$1"
+       echo "$2" > "$SOLVER"
+       chmod +x "$SOLVER"
+       testfailuremsg "$3" apt full-upgrade -s --solver $1
+}
+
+testsolverfailuremsg 'exit0withmsg' "#!/bin/sh
+echo 'Error: instant-exit
+Message: This solver exits instantly'
+exit 0" 'E: External solver failed with: This solver exits instantly'
+
+testsolverfailuremsg 'exit1withoutmsg' "#!/bin/sh
+exit 1" 'E: Sub-process exit1withoutmsg returned an error code (1)'
+
+testsolverfailuremsg 'exit1withmsg' "#!/bin/sh
+echo 'Error: instant-exit
+Message: This solver exits instantly'
+exit 1" 'E: External solver failed with: This solver exits instantly
+E: Sub-process exit1withmsg returned an error code (1)'

diff --git a/test/integration/test-external-installation-planner-protocol b/test/integration/test-external-installation-planner-protocol
index 8d80930bc7863088dda4b254ea0bef52298be105..03b2c4831ceca7c6ee784071e576813c124fcff1 100755 (executable)
--- a/test/integration/test-external-installation-planner-protocol
+++ b/test/integration/test-external-installation-planner-protocol
@@ -70,3 +70,24 @@ Remove: foo:amd64
 Planner: internal' head -n 5 "$EIPPLOG"
 aptinternalplanner < "$EIPPLOG" > planner.log || true
 testsuccessequal 'Remove: 4' grep -e '^Unpack:' -e '^Install:' -e '^Configure:' -e '^Remove:' planner.log
+
+testplannerfailuremsg() {
+       local PLANNER="rootdir/usr/lib/apt/planners/$1"
+       echo "$2" > "$PLANNER"
+       chmod +x "$PLANNER"
+       testfailuremsg "$3" apt install foo -s --planner $1
+}
+
+testplannerfailuremsg 'exit0withmsg' "#!/bin/sh
+echo 'Error: instant-exit
+Message: This planner exits instantly'
+exit 0" 'E: External planner failed with: This planner exits instantly'
+
+testplannerfailuremsg 'exit1withoutmsg' "#!/bin/sh
+exit 1" 'E: Sub-process exit1withoutmsg returned an error code (1)'
+
+testplannerfailuremsg 'exit1withmsg' "#!/bin/sh
+echo 'Error: instant-exit
+Message: This planner exits instantly'
+exit 1" 'E: External planner failed with: This planner exits instantly
+E: Sub-process exit1withmsg returned an error code (1)'