Do not store .diff_Index files in update

Nowadays we only download the index file if we have a non-current file
on disk which we want to patch. If that is the case, any index file for
patches we could have stored is by definition outdated, so storing those
files just takes up disk space.

At least, that is the case if we have a Release file – if we don't this
commit introduces a needless redownload for such repositories but such
repositories are an error by default and if they can't be bothered to
provide a Release file its very unlikely they actually ship diffs, so
adding detection code for this seems pointless at best.

3 files changed, 35 insertions, 36 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 7df6483ba..400838dcb 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -276,6 +276,15 @@ static HashStringList GetExpectedHashesFromFor(metaIndex * const Parser, std::st
    return R->Hashes;
 }
 									/*}}}*/
+static void RemoveOldLeftoverDiffIndex(IndexTarget const &Target)	/*{{{*/
+{
+   std::string const FinalFile = GetFinalFileNameFromURI(GetDiffIndexURI(Target));
+   RemoveFile("TransactionCommit", FinalFile);
+   for (auto const &ext: APT::Configuration::getCompressorExtensions())
+      if (not ext.empty() && ext != ".")
+	 RemoveFile("TransactionCommit", FinalFile + ext);
+}
+									/*}}}*/
 
 class pkgAcquire::Item::Private /*{{{*/
 {
@@ -414,12 +423,16 @@ bool pkgAcqTransactionItem::QueueURI(pkgAcquire::ItemDesc &Item)
 	 std::clog << "Skip " << Target.URI << " as transaction was already dealt with!" << std::endl;
       return false;
    }
-   std::string const FinalFile = GetFinalFilename();
-   if (TransactionManager->IMSHit == true && FileExists(FinalFile) == true)
+   if (TransactionManager->IMSHit)
    {
-      PartialFile = DestFile = FinalFile;
-      Status = StatDone;
-      return false;
+      std::string const FinalFile = GetFinalFilename();
+      if (FinalFile.empty() || FileExists(FinalFile))
+      {
+	 if (not FinalFile.empty())
+	    PartialFile = DestFile = FinalFile;
+	 Status = StatDone;
+	 return false;
+      }
    }
    // this ensures we rewrite only once and only the first step
    auto const OldBaseURI = Target.Option(IndexTarget::BASE_URI);
@@ -508,11 +521,7 @@ std::string pkgAcquire::Item::GetFinalFilename() const
 }
 std::string pkgAcqDiffIndex::GetFinalFilename() const
 {
-   std::string const FinalFile = GetFinalFileNameFromURI(GetDiffIndexURI(Target));
-   // we don't want recompress, so lets keep whatever we got
-   if (CurrentCompressionExtension == "uncompressed")
-      return FinalFile;
-   return FinalFile + "." + CurrentCompressionExtension;
+   return {};
 }
 std::string pkgAcqIndex::GetFinalFilename() const
 {
@@ -675,12 +684,11 @@ bool pkgAcqDiffIndex::TransactionState(TransactionStates const state)
 
    switch (state)
    {
-      case TransactionStarted: _error->Fatal("Item %s changed to invalid transaction start state!", Target.URI.c_str()); break;
+      case TransactionStarted: _error->Fatal("Item %s changed to invalid transaction start state!", GetDiffIndexURI(Target).c_str()); break;
       case TransactionCommit:
+	 RemoveOldLeftoverDiffIndex(Target);
 	 break;
       case TransactionAbort:
-	 std::string const Partial = GetPartialFileNameFromURI(Target.URI);
-	 RemoveFile("TransactionAbort", Partial);
 	 break;
    }
 
@@ -761,6 +769,7 @@ class APT_HIDDEN CleanupItem : public pkgAcqTransactionItem		/*{{{*/
 	       std::clog << "rm " << DestFile << " # " << DescURI() << std::endl;
 	    if (RemoveFile("TransItem::TransactionCommit", DestFile) == false)
 	       return false;
+	    RemoveOldLeftoverDiffIndex(Target);
 	    break;
       }
       return true;
@@ -1704,9 +1713,6 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)			/*{{{*/
 	 if (filename.empty() == false)
 	 {
 	    new NoActionItem(Owner, Target, filename);
-	    std::string const idxfilename = GetFinalFileNameFromURI(GetDiffIndexURI(Target));
-	    if (FileExists(idxfilename))
-	       new NoActionItem(Owner, Target, idxfilename);
 	    targetsSeen.emplace(Target.Option(IndexTarget::CREATED_BY));
 	    continue;
 	 }
@@ -2703,12 +2709,8 @@ void pkgAcqDiffIndex::Failed(string const &Message,pkgAcquire::MethodConfig cons
    new pkgAcqIndex(Owner, TransactionManager, Target);
 }
 									/*}}}*/
-bool pkgAcqDiffIndex::VerifyDone(std::string const &Message, pkgAcquire::MethodConfig const * const)/*{{{*/
+bool pkgAcqDiffIndex::VerifyDone(std::string const &/*Message*/, pkgAcquire::MethodConfig const * const)/*{{{*/
 {
-   string const FinalFile = GetFinalFilename();
-   if(StringToBool(LookupTag(Message,"IMS-Hit"),false))
-      DestFile = FinalFile;
-
    if (ParseDiffIndex(DestFile))
       return true;
 
@@ -2748,7 +2750,7 @@ void pkgAcqDiffIndex::Done(string const &Message,HashStringList const &Hashes,	/
       }
    }
 
-   TransactionManager->TransactionStageCopy(this, DestFile, GetFinalFilename());
+   TransactionManager->TransactionStageRemoval(this, DestFile);
 
    Complete = true;
    Status = StatDone;
diff --git a/apt-private/private-download.cc b/apt-private/private-download.cc
index d95c8ef43..84a088f3d 100644
--- a/apt-private/private-download.cc
+++ b/apt-private/private-download.cc
@@ -316,7 +316,7 @@ static bool CleanDownloadDirectories(bool const ListsToo)
       std::cout << "Del " << archivedir << "* " << archivedir << "partial/*" << std::endl
 		<< "Del " << listsdir << "partial/*" << std::endl;
       if (ListsToo)
-	 std::cout << "Del " << listsdir << "*_{Packages,Sources,Translation-*}{,.diff_Index}" << std::endl;
+	 std::cout << "Del " << listsdir << "*_{Packages,Sources,Translation-*}" << std::endl;
       std::cout << "Del " << pkgcache << " " << srcpkgcache << std::endl;
       return true;
    }
diff --git a/test/integration/test-pdiff-usage b/test/integration/test-pdiff-usage
index 38e455b00..cf3a2f2d9 100755
--- a/test/integration/test-pdiff-usage
+++ b/test/integration/test-pdiff-usage
@@ -38,14 +38,7 @@ wasmergeused() {
 	#apt update "$@" 2>&1 | tee rootdir/tmp/testsuccess.output
 
 	msgtest 'No intermediate patch files' 'still exist'
-	local EDS="$(find rootdir/var/lib/apt/lists -name '*.ed' -o -name '*.ed.*')"
-	if [ -z "$EDS" ]; then
-		msgpass
-	else
-		echo
-		echo "$EDS"
-		msgfail
-	fi
+	testempty --nomsg find rootdir/var/lib/apt/lists -name '*.ed' -o -name '*.ed.*' -o -name '*.diff_Index' -o -name '*.diff_Index.xz'
 
 	if echo "$*" | grep -q -- '-o test::cannot-use-pdiff=1'; then
 		msgtest 'Check if pdiff was' 'not used'
@@ -389,21 +382,25 @@ testcase() {
 	testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=0 "$@"
 }
 generatepartialleftovers() {
+	local PREFIX="$1"
+	shift
 	for f in "$@"; do
-		cat "${PKGFILE}" "${PKGFILE}" > "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${f}"
-		printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${f}"
+		cat "${PKGFILE}" "${PKGFILE}" > "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${PREFIX}${f}"
+		printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_${PREFIX}${f}"
 	done
+	printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/localhost:${APTHTTPPORT}_${PREFIX}Packages.diff_Index"
+	printf '\n\nInvalid\nStanza: yes\n\n' >> "rootdir/var/lib/apt/lists-bak/localhost:${APTHTTPPORT}_${PREFIX}Packages.diff_Index.xz"
 }
 
-partialleftovers() { generatepartialleftovers 'Packages' 'Packages-patched'; }
+partialleftovers() { generatepartialleftovers '' 'Packages' 'Packages-patched'; }
 aptautotest_apt_update() { aptautotest_aptget_update "$@"; testsuccess test -e "rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_Packages"; }
 testcase -o Acquire::IndexTargets::deb::Packages::KeepCompressed=false
-partialleftovers() { generatepartialleftovers "Packages.${LOWCOSTEXT}" "Packages-patched.${LOWCOSTEXT}"; }
+partialleftovers() { generatepartialleftovers '' "Packages.${LOWCOSTEXT}" "Packages-patched.${LOWCOSTEXT}"; }
 aptautotest_apt_update() { aptautotest_aptget_update "$@"; testsuccess test -e "rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_Packages.$LOWCOSTEXT"; }
 testcase -o Acquire::IndexTargets::deb::Packages::KeepCompressed=true
 
 
-partialleftovers() { generatepartialleftovers "redirectme_Packages.${LOWCOSTEXT}" "redirectme_Packages-patched.${LOWCOSTEXT}"; }
+partialleftovers() { generatepartialleftovers 'redirectme_' "Packages.${LOWCOSTEXT}" "Packages-patched.${LOWCOSTEXT}"; }
 # redirect the InRelease file only – the other files are auto-redirected by apt
 webserverconfig 'aptwebserver::redirect::replace::/redirectme/I' "http://0.0.0.0:${APTHTTPPORT}/I"
 rewritesourceslist "http://localhost:${APTHTTPPORT}/redirectme"