Merge branch 'fix/collect-ignored-errors' into 'main'

Collect unprinted Ign errors for display in Err output

See merge request apt-team/apt!405

4 files changed, 86 insertions, 19 deletions

diff --git a/apt-private/acqprogress.cc b/apt-private/acqprogress.cc
index b4b16e6a9..b8b540e75 100644
--- a/apt-private/acqprogress.cc
+++ b/apt-private/acqprogress.cc
@@ -123,39 +123,63 @@ void AcqTextStatus::Fail(pkgAcquire::ItemDesc &Itm)
    AssignItemID(Itm);
    clearLastLine();
 
-   bool ShowErrorText = true;
+   bool ShowErrorText = false;
    if (Itm.Owner->Status == pkgAcquire::Item::StatDone || Itm.Owner->Status == pkgAcquire::Item::StatIdle)
    {
       // TRANSLATOR: Very short word to be displayed for files in 'apt-get update'
       // which failed to download, but the error is ignored (compare "Err:")
       ioprintf(out, _("Ign:%lu %s"), Itm.Owner->ID, Itm.Description.c_str());
-      if (Itm.Owner->ErrorText.empty() ||
-	    _config->FindB("Acquire::Progress::Ignore::ShowErrorText", false) == false)
-	 ShowErrorText = false;
+      if (not Itm.Owner->ErrorText.empty())
+      {
+	 if (_config->FindB("Acquire::Progress::Ignore::ShowErrorText", false))
+	    ShowErrorText = true;
+	 else
+	 {
+	    if (auto const ignored = IgnoredErrorTexts.find(Itm.Owner->ID); ignored != IgnoredErrorTexts.end())
+	    {
+	       if (std::find(ignored->second.begin(), ignored->second.end(), Itm.Owner->ErrorText) == ignored->second.end())
+		  ignored->second.emplace_back(Itm.Owner->ErrorText);
+	    }
+	    else
+	       IgnoredErrorTexts.emplace(Itm.Owner->ID, std::vector<std::string>{Itm.Owner->ErrorText});
+	 }
+      }
    }
    else
    {
       // TRANSLATOR: Very short word to be displayed for files in 'apt-get update'
       // which failed to download and the error is critical (compare "Ign:")
       ioprintf(out, _("Err:%lu %s"), Itm.Owner->ID, Itm.Description.c_str());
+      ShowErrorText = true;
    }
 
-   if (ShowErrorText)
+   if (ShowErrorText && not Itm.Owner->ErrorText.empty())
    {
-      std::string::size_type line_start = 0;
-      std::string::size_type line_end;
-      while ((line_end = Itm.Owner->ErrorText.find_first_of("\n\r", line_start)) != std::string::npos) {
-	 out << std::endl << "  " << Itm.Owner->ErrorText.substr(line_start, line_end - line_start);
-	 line_start = Itm.Owner->ErrorText.find_first_not_of("\n\r", line_end + 1);
-	 if (line_start == std::string::npos)
-	    break;
+      auto const printErrorText = [](std::ostream &out, std::string_view errortext) {
+	 while (not errortext.empty())
+	 {
+	    auto const line_end = errortext.find_first_of("\n\r");
+	    out << "\n  " << errortext.substr(0, line_end);
+	    if (line_end == std::string_view::npos)
+	       break;
+	    errortext.remove_prefix(line_end);
+	    auto const line_start = errortext.find_first_not_of("\n\r");
+	    if (line_start == std::string_view::npos)
+	       break;
+	    errortext.remove_prefix(line_start);
+	 }
+      };
+      if (auto const ignored = IgnoredErrorTexts.find(Itm.Owner->ID); ignored != IgnoredErrorTexts.end())
+      {
+	 for (auto const &text : ignored->second)
+	    printErrorText(out, text);
+	 if (std::find(ignored->second.begin(), ignored->second.end(), Itm.Owner->ErrorText) == ignored->second.end())
+	    printErrorText(out, Itm.Owner->ErrorText);
       }
-      if (line_start == 0)
-	 out << std::endl << "  " << Itm.Owner->ErrorText;
-      else if (line_start != std::string::npos)
-	 out << std::endl << "  " << Itm.Owner->ErrorText.substr(line_start);
+      else
+	 printErrorText(out, Itm.Owner->ErrorText);
    }
-   out << std::endl;
+   out << '\n' << std::flush;
 
    Update = true;
 }
diff --git a/apt-private/acqprogress.h b/apt-private/acqprogress.h
index 87b957e4b..b4015861c 100644
--- a/apt-private/acqprogress.h
+++ b/apt-private/acqprogress.h
@@ -14,6 +14,8 @@
 
 #include <iostream>
 #include <string>
+#include <unordered_map>
+#include <vector>
 
 class APT_PUBLIC AcqTextStatus : public pkgAcquireStatus
 {
@@ -22,6 +24,7 @@ class APT_PUBLIC AcqTextStatus : public pkgAcquireStatus
    size_t LastLineLength;
    unsigned long ID;
    unsigned long Quiet;
+   std::unordered_map<unsigned long, std::vector<std::string>> IgnoredErrorTexts;
 
    APT_HIDDEN void clearLastLine();
    APT_HIDDEN void AssignItemID(pkgAcquire::ItemDesc &Itm);
diff --git a/test/integration/test-bug-869859-retry-downloads b/test/integration/test-bug-869859-retry-downloads
index 333ce816e..821d7bc3e 100755
--- a/test/integration/test-bug-869859-retry-downloads
+++ b/test/integration/test-bug-869859-retry-downloads
@@ -48,7 +48,17 @@ Ign:1 http://localhost:${APTHTTPPORT} stable/main all testpkg all 1
   429  Unknown HTTP code
 Err:1 http://localhost:${APTHTTPPORT} stable/main all testpkg all 1
   429  Unknown HTTP code
-E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/testpkg_1_all.deb  429  Unknown HTTP code" apt download testpkg -o acquire::retries=3 -o debug::acquire::retries=true -q
+E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/testpkg_1_all.deb  429  Unknown HTTP code" apt download testpkg -o acquire::retries=3 -o debug::acquire::retries=true -q -o Acquire::Progress::Ignore::ShowErrorText=true
+testfailure test -f testpkg_1_all.deb
+testsecondsgreaterequal 5 testfailureequal "Delaying http://localhost:${APTHTTPPORT} stable/main all testpkg all 1 by 1 seconds
+Ign:1 http://localhost:${APTHTTPPORT} stable/main all testpkg all 1
+Delaying http://localhost:${APTHTTPPORT} stable/main all testpkg all 1 by 2 seconds
+Ign:1 http://localhost:${APTHTTPPORT} stable/main all testpkg all 1
+Delaying http://localhost:${APTHTTPPORT} stable/main all testpkg all 1 by 4 seconds
+Ign:1 http://localhost:${APTHTTPPORT} stable/main all testpkg all 1
+Err:1 http://localhost:${APTHTTPPORT} stable/main all testpkg all 1
+  429  Unknown HTTP code
+E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/testpkg_1_all.deb  429  Unknown HTTP code" apt download testpkg -o acquire::retries=3 -o debug::acquire::retries=true -q -o Acquire::Progress::Ignore::ShowErrorText=false
 testfailure test -f testpkg_1_all.deb
 
 msgmsg 'Retry 429 TooManyRequests after Retry-After time'
@@ -66,6 +76,25 @@ testsuccess apt download testpkg -o acquire::retries=3 -o acquire::retries::dela
 testsuccess test -f testpkg_1_all.deb
 rm -f testpkg_1_all.deb
 
+msgmsg 'Hard failure after retry request'
+webserverconfig 'aptwebserver::failrequest::pool/unavailable_1_all.deb' '1'
+testfailureequal "Ign:1 http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb
+  429  Unknown HTTP code
+Err:1 http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb
+  404  Not Found
+E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb  404  Not Found
+E: Download Failed" apthelper download-file "http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb" 'unavailable_1_all.deb' -o acquire::retries=2 -o acquire::retries::delay=false -o Acquire::Progress::Ignore::ShowErrorText=true
+testfailure test -f unavailable_1_all.deb
+webserverconfig 'aptwebserver::failrequest::pool/unavailable_1_all.deb' '1'
+testfailureequal "Ign:1 http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb
+Err:1 http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb
+  429  Unknown HTTP code
+  404  Not Found
+E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb  404  Not Found
+E: Download Failed" apthelper download-file "http://localhost:${APTHTTPPORT}/pool/unavailable_1_all.deb" 'unavailable_1_all.deb' -o acquire::retries=2 -o acquire::retries::delay=false -o Acquire::Progress::Ignore::ShowErrorText=false
+testfailure test -f unavailable_1_all.deb
+
+
 msgmsg 'Do not try everything again, hard failures keep hard failures'
 webserverconfig 'aptwebserver::failrequest' '404'
 webserverconfig 'aptwebserver::failrequest::pool/testpkg_1_all.deb' '2'
diff --git a/test/integration/test-handle-redirect-as-used-mirror-change b/test/integration/test-handle-redirect-as-used-mirror-change
index a6f8b788f..1ce1232e2 100755
--- a/test/integration/test-handle-redirect-as-used-mirror-change
+++ b/test/integration/test-handle-redirect-as-used-mirror-change
@@ -67,7 +67,18 @@ Get:3 http://0.0.0.0:${APTHTTPPORT} unstable/main all Packages [$(stat -c %s apt
 Get:4 http://0.0.0.0:${APTHTTPPORT} unstable/main Translation-en [$(stat -c %s aptarchive/dists/unstable/main/i18n/Translation-en.gz) B]
 Reading package lists...
 Building dependency tree...
-All packages are up to date." apt update
+All packages are up to date." apt update -o Acquire::Progress::Ignore::ShowErrorText=true
+rm -rf rootdir/var/lib/apt/lists
+testsuccessequal "Get:1 http://0.0.0.0:${APTHTTPPORT}/storage unstable InRelease [$(stat -c %s aptarchive/storage/dists/unstable/InRelease) B]
+Ign:2 http://0.0.0.0:${APTHTTPPORT}/storage unstable/main Sources
+Ign:3 http://0.0.0.0:${APTHTTPPORT}/storage unstable/main all Packages
+Ign:4 http://0.0.0.0:${APTHTTPPORT}/storage unstable/main Translation-en
+Get:2 http://0.0.0.0:${APTHTTPPORT} unstable/main Sources [$(stat -c %s aptarchive/dists/unstable/main/source/Sources.gz) B]
+Get:3 http://0.0.0.0:${APTHTTPPORT} unstable/main all Packages [$(stat -c %s aptarchive/dists/unstable/main/binary-all/Packages.gz) B]
+Get:4 http://0.0.0.0:${APTHTTPPORT} unstable/main Translation-en [$(stat -c %s aptarchive/dists/unstable/main/i18n/Translation-en.gz) B]
+Reading package lists...
+Building dependency tree...
+All packages are up to date." apt update -o Acquire::Progress::Ignore::ShowErrorText=false
 
 find aptarchive -name 'InRelease' -delete
 rm -rf rootdir/var/lib/apt/lists