don't use Desc.URI to calculate .diff/Index filenames

The URI descibing an item can change via mirrors/redirectors which
causes the .diff/Index files to get the wrong names in storage.

Git-Dch: Ignore

2 files changed, 39 insertions, 12 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 45f8cb76c..717751a6e 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -121,6 +121,16 @@ static std::string GetExistingFilename(std::string const &File)		/*{{{*/
    return "";
 }
 									/*}}}*/
+static std::string GetDiffIndexFileName(std::string const &Name)	/*{{{*/
+{
+   return Name + ".diff/Index";
+}
+									/*}}}*/
+static std::string GetDiffIndexURI(IndexTarget const &Target)		/*{{{*/
+{
+   return Target.URI + ".diff/Index";
+}
+									/*}}}*/
 
 static bool MessageInsecureRepository(bool const isError, std::string const &msg)/*{{{*/
 {
@@ -301,12 +311,12 @@ bool pkgAcqDiffIndex::QueueURI(pkgAcquire::ItemDesc &Item)
 // Acquire::Item::GetFinalFilename and specialisations for child classes	/*{{{*/
 std::string pkgAcquire::Item::GetFinalFilename() const
 {
+   // Beware: Desc.URI is modified by redirections
    return GetFinalFileNameFromURI(Desc.URI);
 }
 std::string pkgAcqDiffIndex::GetFinalFilename() const
 {
-   // the logic we inherent from pkgAcqBaseIndex isn't what we need here
-   return pkgAcquire::Item::GetFinalFilename();
+   return GetFinalFileNameFromURI(GetDiffIndexURI(Target));
 }
 std::string pkgAcqIndex::GetFinalFilename() const
 {
@@ -343,7 +353,7 @@ std::string pkgAcqIndex::GetMetaKey() const
 }
 std::string pkgAcqDiffIndex::GetMetaKey() const
 {
-   return Target.MetaKey + ".diff/Index";
+   return GetDiffIndexFileName(Target.MetaKey);
 }
 									/*}}}*/
 //pkgAcqTransactionItem::TransactionState and specialisations for child classes	/*{{{*/
@@ -1166,14 +1176,14 @@ void pkgAcqMetaBase::QueueIndexes(bool const verify)			/*{{{*/
 	 if (filename.empty() == false)
 	 {
 	    new NoActionItem(Owner, *Target, filename);
-	    std::string const idxfilename = GetFinalFileNameFromURI(Target->URI + ".diff/Index");
+	    std::string const idxfilename = GetFinalFileNameFromURI(GetDiffIndexURI(*Target));
 	    if (FileExists(idxfilename))
 	       new NoActionItem(Owner, *Target, idxfilename);
 	    continue;
 	 }
 
 	 // check if we have patches available
-	 trypdiff &= TransactionManager->MetaIndexParser->Exists(Target->MetaKey + ".diff/Index");
+	 trypdiff &= TransactionManager->MetaIndexParser->Exists(GetDiffIndexFileName(Target->MetaKey));
       }
       else
       {
@@ -1728,9 +1738,9 @@ pkgAcqDiffIndex::pkgAcqDiffIndex(pkgAcquire * const Owner,
    Debug = _config->FindB("Debug::pkgAcquire::Diffs",false);
 
    Desc.Owner = this;
-   Desc.Description = Target.Description + ".diff/Index";
+   Desc.Description = GetDiffIndexFileName(Target.Description);
    Desc.ShortDesc = Target.ShortDesc;
-   Desc.URI = Target.URI + ".diff/Index";
+   Desc.URI = GetDiffIndexURI(Target);
 
    DestFile = GetPartialFileNameFromURI(Desc.URI);
 
@@ -2444,16 +2454,18 @@ void pkgAcqIndexMergeDiffs::Failed(string const &Message,pkgAcquire::MethodConfi
    for (std::vector<pkgAcqIndexMergeDiffs *>::const_iterator I = allPatches->begin();
 	 I != allPatches->end(); ++I)
       if ((*I)->State == StateErrorDiff)
+      {
+	 State = StateErrorDiff;
 	 return;
+      }
 
    // first failure means we should fallback
    State = StateErrorDiff;
    if (Debug)
       std::clog << "Falling back to normal index file acquire" << std::endl;
    RenameOnError(PDiffError);
-   std::string const patchname = GetPartialFileNameFromURI(Desc.URI);
-   if (RealFileExists(patchname))
-      Rename(patchname, patchname + ".FAILED");
+   if (RealFileExists(DestFile))
+      Rename(DestFile, DestFile + ".FAILED");
    std::string const UnpatchedFile = GetExistingFilename(GetPartialFileNameFromURI(Target.URI));
    if (UnpatchedFile.empty() == false && FileExists(UnpatchedFile))
       Rename(UnpatchedFile, UnpatchedFile + ".FAILED");
@@ -2474,6 +2486,7 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
    {
       if(Debug)
 	 std::clog << "Another patch failed already, no point in processing this one." << std::endl;
+      State = StateErrorDiff;
       return;
    }
 
@@ -2481,7 +2494,8 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
    std::string const UnpatchedFile = GetExistingFilename(UncompressedUnpatchedFile);
    if (UnpatchedFile.empty())
    {
-      _error->Fatal("Unpatched file %s doesn't exist (anymore)!", UnpatchedFile.c_str());
+      _error->Fatal("Unpatched file %s doesn't exist (anymore)!", UncompressedUnpatchedFile.c_str());
+      State = StateErrorDiff;
       return;
    }
    std::string const PatchFile = GetMergeDiffsPatchFileName(UnpatchedFile, patch.file);
diff --git a/test/integration/test-pdiff-usage b/test/integration/test-pdiff-usage
index 2318448f5..11c8c4cc1 100755
--- a/test/integration/test-pdiff-usage
+++ b/test/integration/test-pdiff-usage
@@ -65,7 +65,7 @@ testrun() {
 	configcompression '.' 'gz'
 
 	# see if the code deals properly with leftover partial files
-	touch rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_Packages
+	partialleftovers
 
 	msgmsg "Testcase: apply with one patch: $*"
 	find aptarchive -name 'Packages*' -type f -delete
@@ -279,7 +279,20 @@ testcase() {
 	testrun -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=0 "$@"
 	testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=0 "$@"
 }
+partialleftovers() { touch "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_Packages"; }
 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() { touch "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_Packages.$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() { touch "rootdir/var/lib/apt/lists-bak/partial/localhost:${APTHTTPPORT}_redirectme_Packages.$LOWCOSTEXT"; }
+webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "http://0.0.0.0:${APTHTTPPORT}/"
+rewritesourceslist "http://localhost:${APTHTTPPORT}/redirectme"
+aptautotest_apt_update() {
+	aptautotest_aptget_update "$@"
+	testsuccess test -e "rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_redirectme_Packages.$LOWCOSTEXT"
+	testempty find rootdir/var/lib/apt/lists -type f \! \( -name lock -o -name '*_redirectme_*' \)
+}
+testcase -o Acquire::IndexTargets::deb::Packages::KeepCompressed=true