implement PDiff patching for compressed files

Some additional files like 'Contents' are very big and should therefore
kept compressed on the disk, which apt-file did in the past. It also
implemented pdiff patching of these files by un- and recompressing these
files on-the-fly, with this commit we can do the same – but we can do
this in both pdiff patching styles (client and server merging) and
secured by hashes.

Hashes are in so far slightly complicated as we can't compare the hashes
of the compressed files as we might compress them differently than the
server would (different compressor versions, options, …), so we must
compare the hashes of the uncompressed content.

While this commit has changes in public headers, the classes it changes
are marked as hidden, so nobody can use them directly, which means the
ABI break is internal only.

apt-pkg/acquire-item.cc

@@ -32,6 +32,7 @@
 #include <apt-pkg/pkgrecords.h>
 #include <apt-pkg/gpgv.h>
 
+#include <algorithm>
 #include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
@@ -79,6 +80,21 @@ static std::string GetFinalFileNameFromURI(std::string const &uri)	/*{{{*/
    return _config->FindDir("Dir::State::lists") + URItoFileName(uri);
 }
 									/*}}}*/
+static std::string GetKeepCompressedFileName(std::string file, IndexTarget const &Target)/*{{{*/
+{
+   if (Target.KeepCompressed == false)
+      return file;
+
+   std::string const CompressionTypes = Target.Option(IndexTarget::COMPRESSIONTYPES);
+   if (CompressionTypes.empty() == false)
+   {
+      std::string const ext = CompressionTypes.substr(0, CompressionTypes.find(' '));
+      if (ext != "uncompressed")
+	 file.append(".").append(ext);
+   }
+   return file;
+}
+									/*}}}*/
 static std::string GetCompressedFileName(IndexTarget const &Target, std::string const &Name, std::string const &Ext) /*{{{*/
 {
    if (Ext.empty() || Ext == "uncompressed")
@@ -107,6 +123,30 @@ static std::string GetDiffsPatchFileName(std::string const &Final)	/*{{{*/
    return Final + ".ed";
 }
 									/*}}}*/
+static bool BootstrapPDiffWith(std::string const &PartialFile, std::string const &FinalFile, IndexTarget const &Target)/*{{{*/
+{
+   // patching needs to be bootstrapped with the 'old' version
+   std::vector<std::string> types = VectorizeString(Target.Option(IndexTarget::COMPRESSIONTYPES), ' ');
+   auto typeItr = types.cbegin();
+   for (; typeItr != types.cend(); ++typeItr)
+   {
+      std::string Final = FinalFile;
+      if (*typeItr != "uncompressed")
+	 Final.append(".").append(*typeItr);
+      if (RealFileExists(Final) == false)
+	 continue;
+      std::string Partial = PartialFile;
+      if (*typeItr != "uncompressed")
+	 Partial.append(".").append(*typeItr);
+      if (FileExists(Partial.c_str()) == true)
+	 return true;
+      if (symlink(Final.c_str(), Partial.c_str()) != 0)
+	 return false;
+      break;
+   }
+   return typeItr != types.cend();
+}
+									/*}}}*/
 
 static bool AllowInsecureRepositories(metaIndex const * const MetaIndexParser, pkgAcqMetaClearSig * const TransactionManager, pkgAcquire::Item * const I) /*{{{*/
 {
@@ -399,6 +439,12 @@ class APT_HIDDEN NoActionItem : public pkgAcquire::Item			/*{{{*/
       Status = StatDone;
       DestFile = GetFinalFileNameFromURI(Target.URI);
    }
+   NoActionItem(pkgAcquire * const Owner, IndexTarget const &Target, std::string const &FinalFile) :
+      pkgAcquire::Item(Owner), Target(Target)
+   {
+      Status = StatDone;
+      DestFile = FinalFile;
+   }
 };
 									/*}}}*/
 
@@ -953,7 +999,7 @@ void pkgAcqMetaBase::QueueIndexes(bool const verify)			/*{{{*/
    // at this point the real Items are loaded in the fetcher
    ExpectedAdditionalItems = 0;
 
-   for (std::vector <IndexTarget>::const_iterator Target = IndexTargets.begin();
+   for (std::vector <IndexTarget>::iterator Target = IndexTargets.begin();
         Target != IndexTargets.end();
         ++Target)
    {
@@ -971,34 +1017,84 @@ void pkgAcqMetaBase::QueueIndexes(bool const verify)			/*{{{*/
 	    return;
 	 }
 
-	 if (RealFileExists(GetFinalFileNameFromURI(Target->URI)))
+	 // autoselect the compression method
+	 std::vector<std::string> types = VectorizeString(Target->Option(IndexTarget::COMPRESSIONTYPES), ' ');
+	 types.erase(std::remove_if(types.begin(), types.end(), [&](std::string const &t) {
+	    if (t == "uncompressed")
+	       return TransactionManager->MetaIndexParser->Exists(Target->MetaKey) == false;
+	    std::string const MetaKey = Target->MetaKey + "." + t;
+	    return TransactionManager->MetaIndexParser->Exists(MetaKey) == false;
+	 }), types.end());
+	 if (types.empty() == false)
 	 {
-	    if (TransactionManager->LastMetaIndexParser != NULL)
+	    std::ostringstream os;
+	    std::copy(types.begin(), types.end()-1, std::ostream_iterator<std::string>(os, " "));
+	    os << *types.rbegin();
+	    Target->Options["COMPRESSIONTYPES"] = os.str();
+	 }
+	 else
+	    Target->Options["COMPRESSIONTYPES"].clear();
+
+	 std::string filename = GetFinalFileNameFromURI(Target->URI);
+	 if (RealFileExists(filename) == false)
+	 {
+	    if (Target->KeepCompressed)
 	    {
-	       HashStringList const newFile = GetExpectedHashesFromFor(TransactionManager->MetaIndexParser, Target->MetaKey);
-	       HashStringList const oldFile = GetExpectedHashesFromFor(TransactionManager->LastMetaIndexParser, Target->MetaKey);
-	       if (newFile == oldFile)
-	       {
-		  // we have the file already, no point in trying to acquire it again
-		  new NoActionItem(Owner, *Target);
-		  continue;
-	       }
+	       filename = GetKeepCompressedFileName(filename, *Target);
+	       if (RealFileExists(filename) == false)
+		  filename.clear();
 	    }
-	    else if (TransactionManager->IMSHit == true)
+	    else
+	       filename.clear();
+	 }
+
+	 if (filename.empty() == false)
+	 {
+	    // if the Release file is a hit and we have an index it must be the current one
+	    if (TransactionManager->IMSHit == true)
+	       ;
+	    else if (TransactionManager->LastMetaIndexParser != NULL)
 	    {
-	       // we have the file already, no point in trying to acquire it again
-	       new NoActionItem(Owner, *Target);
-	       continue;
+	       // see if the file changed since the last Release file
+	       // we use the uncompressed files as we might compress differently compared to the server,
+	       // so the hashes might not match, even if they contain the same data.
+	       HashStringList const newFile = GetExpectedHashesFromFor(TransactionManager->MetaIndexParser, Target->MetaKey);
+	       HashStringList const oldFile = GetExpectedHashesFromFor(TransactionManager->LastMetaIndexParser, Target->MetaKey);
+	       if (newFile != oldFile)
+		  filename.clear();
 	    }
+	    else
+	       filename.clear();
 	 }
 	 else
 	    trypdiff = false; // no file to patch
 
+	 if (filename.empty() == false)
+	 {
+	    new NoActionItem(Owner, *Target, filename);
+	    continue;
+	 }
+
 	 // check if we have patches available
 	 trypdiff &= TransactionManager->MetaIndexParser->Exists(Target->MetaKey + ".diff/Index");
       }
-      // if we have no file to patch, no point in trying
-      trypdiff &= RealFileExists(GetFinalFileNameFromURI(Target->URI));
+      else
+      {
+	 // if we have no file to patch, no point in trying
+	 std::string filename = GetFinalFileNameFromURI(Target->URI);
+	 if (RealFileExists(filename) == false)
+	 {
+	    if (Target->KeepCompressed)
+	    {
+	       filename = GetKeepCompressedFileName(filename, *Target);
+	       if (RealFileExists(filename) == false)
+		  filename.clear();
+	    }
+	    else
+	       filename.clear();
+	 }
+	 trypdiff &= (filename.empty() == false);
+      }
 
       // no point in patching from local sources
       if (trypdiff)
@@ -1658,7 +1754,7 @@ bool pkgAcqDiffIndex::ParseDiffIndex(string const &IndexDiffFile)	/*{{{*/
       LocalHashes = GetExpectedHashesFromFor(TransactionManager->LastMetaIndexParser, Target.MetaKey);
    if (LocalHashes.usable() == false)
    {
-      FileFd fd(CurrentPackagesFile, FileFd::ReadOnly);
+      FileFd fd(CurrentPackagesFile, FileFd::ReadOnly, FileFd::Auto);
       Hashes LocalHashesCalc(ServerHashes);
       LocalHashesCalc.AddFD(fd);
       LocalHashes = LocalHashesCalc.GetHashStringList();
@@ -1961,7 +2057,7 @@ pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire * const Owner,
    : pkgAcqBaseIndex(Owner, TransactionManager, Target), d(NULL),
      available_patches(diffs)
 {
-   DestFile = GetPartialFileNameFromURI(Target.URI);
+   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
 
    Debug = _config->FindB("Debug::pkgAcquire::Diffs",false);
 
@@ -1972,20 +2068,15 @@ pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire * const Owner,
    if(available_patches.empty() == true)
    {
       // we are done (yeah!), check hashes against the final file
-      DestFile = GetFinalFileNameFromURI(Target.URI);
+      DestFile = GetKeepCompressedFileName(GetFinalFileNameFromURI(Target.URI), Target);
       Finish(true);
    }
    else
    {
-      // patching needs to be bootstrapped with the 'old' version
-      std::string const PartialFile = GetPartialFileNameFromURI(Target.URI);
-      if (RealFileExists(PartialFile) == false)
+      if (BootstrapPDiffWith(GetPartialFileNameFromURI(Target.URI), GetFinalFilename(), Target) == false)
       {
-	 if (symlink(GetFinalFilename().c_str(), PartialFile.c_str()) != 0)
-	 {
-	    Failed("Link creation of " + PartialFile + " to " + GetFinalFilename() + " failed", NULL);
-	    return;
-	 }
+	 Failed("Bootstrapping of " + DestFile + " failed", NULL);
+	 return;
       }
 
       // get the next diff
@@ -1999,10 +2090,10 @@ void pkgAcqIndexDiffs::Failed(string const &Message,pkgAcquire::MethodConfig con
    Item::Failed(Message,Cnf);
    Status = StatDone;
 
+   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
    if(Debug)
       std::clog << "pkgAcqIndexDiffs failed: " << Desc.URI << " with " << Message << std::endl
-		<< "Falling back to normal index file acquire" << std::endl;
-   DestFile = GetPartialFileNameFromURI(Target.URI);
+		<< "Falling back to normal index file acquire " << std::endl;
    RenameOnError(PDiffError);
    std::string const patchname = GetDiffsPatchFileName(DestFile);
    if (RealFileExists(patchname))
@@ -2023,7 +2114,14 @@ void pkgAcqIndexDiffs::Finish(bool allDone)
    // the file will be cleaned
    if(allDone)
    {
-      TransactionManager->TransactionStageCopy(this, DestFile, GetFinalFilename());
+      std::string Final = GetFinalFilename();
+      if (Target.KeepCompressed)
+      {
+	 std::string const ext = flExtension(DestFile);
+	 if (ext.empty() == false)
+	    Final.append(".").append(ext);
+      }
+      TransactionManager->TransactionStageCopy(this, DestFile, Final);
 
       // this is for the "real" finish
       Complete = true;
@@ -2033,6 +2131,8 @@ void pkgAcqIndexDiffs::Finish(bool allDone)
 	 std::clog << "\n\nallDone: " << DestFile << "\n" << std::endl;
       return;
    }
+   else
+      DestFile.clear();
 
    if(Debug)
       std::clog << "Finishing: " << Desc.URI << std::endl;
@@ -2045,15 +2145,14 @@ void pkgAcqIndexDiffs::Finish(bool allDone)
 bool pkgAcqIndexDiffs::QueueNextDiff()					/*{{{*/
 {
    // calc sha1 of the just patched file
-   std::string const FinalFile = GetPartialFileNameFromURI(Target.URI);
-
+   std::string const FinalFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
    if(!FileExists(FinalFile))
    {
       Failed("Message: No FinalFile " + FinalFile + " available", NULL);
       return false;
    }
 
-   FileFd fd(FinalFile, FileFd::ReadOnly);
+   FileFd fd(FinalFile, FileFd::ReadOnly, FileFd::Extension);
    Hashes LocalHashesCalc;
    LocalHashesCalc.AddFD(fd);
    HashStringList const LocalHashes = LocalHashesCalc.GetHashStringList();
@@ -2097,7 +2196,7 @@ bool pkgAcqIndexDiffs::QueueNextDiff()					/*{{{*/
    // queue the right diff
    Desc.URI = Target.URI + ".diff/" + available_patches[0].file + ".gz";
    Desc.Description = Description + " " + available_patches[0].file + string(".pdiff");
-   DestFile = GetPartialFileNameFromURI(Target.URI + ".diff/" + available_patches[0].file);
+   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI + ".diff/" + available_patches[0].file), Target);
 
    if(Debug)
       std::clog << "pkgAcqIndexDiffs::QueueNextDiff(): " << Desc.URI << std::endl;
@@ -2115,7 +2214,7 @@ void pkgAcqIndexDiffs::Done(string const &Message, HashStringList const &Hashes,
 
    Item::Done(Message, Hashes, Cnf);
 
-   std::string const FinalFile = GetPartialFileNameFromURI(Target.URI);
+   std::string const FinalFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
    std::string const PatchFile = GetDiffsPatchFileName(FinalFile);
 
    // success in downloading a diff, enter ApplyDiff state
@@ -2194,7 +2293,7 @@ pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcquire * const Owner,
    Desc.URI = Target.URI + ".diff/" + patch.file + ".gz";
    Desc.Description = Description + " " + patch.file + string(".pdiff");
 
-   DestFile = GetPartialFileNameFromURI(Target.URI + ".diff/" + patch.file);
+   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI + ".diff/" + patch.file), Target);
 
    if(Debug)
       std::clog << "pkgAcqIndexMergeDiffs: " << Desc.URI << std::endl;
@@ -2221,12 +2320,13 @@ void pkgAcqIndexMergeDiffs::Failed(string const &Message,pkgAcquire::MethodConfi
    State = StateErrorDiff;
    if (Debug)
       std::clog << "Falling back to normal index file acquire" << std::endl;
-   DestFile = GetPartialFileNameFromURI(Target.URI);
+   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
    RenameOnError(PDiffError);
    std::string const patchname = GetMergeDiffsPatchFileName(DestFile, patch.file);
    if (RealFileExists(patchname))
       rename(patchname.c_str(), std::string(patchname + ".FAILED").c_str());
    new pkgAcqIndex(Owner, TransactionManager, Target);
+   DestFile.clear();
 }
 									/*}}}*/
 void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Hashes,	/*{{{*/
@@ -2237,7 +2337,8 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
 
    Item::Done(Message, Hashes, Cnf);
 
-   string const FinalFile = GetPartialFileNameFromURI(Target.URI);
+   std::string const UncompressedFinalFile = GetPartialFileNameFromURI(Target.URI);
+   std::string const FinalFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
    if (State == StateFetchDiff)
    {
       Rename(DestFile, GetMergeDiffsPatchFileName(FinalFile, patch.file));
@@ -2256,10 +2357,9 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
       // this is the last completed diff, so we are ready to apply now
       State = StateApplyDiff;
 
-      // patching needs to be bootstrapped with the 'old' version
-      if (symlink(GetFinalFilename().c_str(), FinalFile.c_str()) != 0)
+      if (BootstrapPDiffWith(UncompressedFinalFile, GetFinalFilename(), Target) == false)
       {
-	 Failed("Link creation of " + FinalFile + " to " + GetFinalFilename() + " failed", NULL);
+	 Failed("Bootstrapping of " + DestFile + " failed", NULL);
 	 return;
       }
 
@@ -2276,7 +2376,7 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
    else if (State == StateApplyDiff)
    {
       // move the result into place
-      std::string const Final = GetFinalFilename();
+      std::string const Final = GetKeepCompressedFileName(GetFinalFilename(), Target);
       if(Debug)
 	 std::clog << "Queue patched file in place: " << std::endl
 		   << DestFile << " -> " << Final << std::endl;
@@ -2288,7 +2388,7 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
       for (std::vector<pkgAcqIndexMergeDiffs *>::const_iterator I = allPatches->begin();
 	    I != allPatches->end(); ++I)
       {
-	 std::string const PartialFile = GetPartialFileNameFromURI(Target.URI);
+	 std::string const PartialFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
 	 std::string const patch = GetMergeDiffsPatchFileName(PartialFile, (*I)->patch.file);
 	 unlink(patch.c_str());
       }
@@ -2325,10 +2425,9 @@ pkgAcqIndexMergeDiffs::~pkgAcqIndexMergeDiffs() {}
 pkgAcqIndex::pkgAcqIndex(pkgAcquire * const Owner,
                          pkgAcqMetaClearSig * const TransactionManager,
                          IndexTarget const &Target)
-   : pkgAcqBaseIndex(Owner, TransactionManager, Target), d(NULL), Stage(STAGE_DOWNLOAD)
+   : pkgAcqBaseIndex(Owner, TransactionManager, Target), d(NULL), Stage(STAGE_DOWNLOAD),
+   CompressionExtensions(Target.Option(IndexTarget::COMPRESSIONTYPES))
 {
-   // autoselect the compression method
-   AutoSelectCompression();
    Init(Target.URI, Target.Description, Target.ShortDesc);
 
    if(_config->FindB("Debug::Acquire::Transaction", false) == true)
@@ -2336,31 +2435,6 @@ pkgAcqIndex::pkgAcqIndex(pkgAcquire * const Owner,
                 << TransactionManager << std::endl;
 }
 									/*}}}*/
-// AcqIndex::AutoSelectCompression - Select compression			/*{{{*/
-void pkgAcqIndex::AutoSelectCompression()
-{
-   std::vector<std::string> types = APT::Configuration::getCompressionTypes();
-   CompressionExtensions = "";
-   if (TransactionManager->MetaIndexParser != NULL && TransactionManager->MetaIndexParser->Exists(Target.MetaKey))
-   {
-      for (std::vector<std::string>::const_iterator t = types.begin();
-           t != types.end(); ++t)
-      {
-         std::string CompressedMetaKey = string(Target.MetaKey).append(".").append(*t);
-         if (*t == "uncompressed" ||
-             TransactionManager->MetaIndexParser->Exists(CompressedMetaKey) == true)
-            CompressionExtensions.append(*t).append(" ");
-      }
-   }
-   else
-   {
-      for (std::vector<std::string>::const_iterator t = types.begin(); t != types.end(); ++t)
-	 CompressionExtensions.append(*t).append(" ");
-   }
-   if (CompressionExtensions.empty() == false)
-      CompressionExtensions.erase(CompressionExtensions.end()-1);
-}
-									/*}}}*/
 // AcqIndex::Init - defered Constructor					/*{{{*/
 void pkgAcqIndex::Init(string const &URI, string const &URIDesc,
                        string const &ShortDesc)

apt-pkg/acquire-item.h

@@ -409,7 +409,7 @@ class APT_HIDDEN pkgAcqMetaBase : public pkgAcqTransactionItem		/*{{{*/
    /** \brief The index files which should be looked up in the meta-index
     *  and then downloaded.
     */
-   std::vector<IndexTarget> const IndexTargets;
+   std::vector<IndexTarget> IndexTargets;
 
    /** \brief If \b true, the index's signature is currently being verified.
     */
@@ -944,9 +944,6 @@ class APT_HIDDEN pkgAcqIndex : public pkgAcqBaseIndex
    /** \brief Do the changes needed to fetch via AptByHash (if needed) */
    void InitByHashIfNeeded();
 
-   /** \brief Auto select the right compression to use */
-   void AutoSelectCompression();
-
    /** \brief Schedule file for verification after a IMS hit */
    void ReverifyAfterIMS();
 

apt-pkg/deb/debmetaindex.cc

@@ -21,6 +21,7 @@
 #include <utility>
 #include <vector>
 #include <algorithm>
+#include <sstream>
 
 #include <sys/stat.h>
 #include <string.h>
@@ -121,19 +122,33 @@ static void GetIndexTargetsFor(char const * const Type, std::string const &URI,
    std::string const Release = (Dist == "/") ? "" : Dist;
    std::string const Site = ::URI::ArchiveOnly(URI);
 
+   std::string DefCompressionTypes;
+   {
+      std::vector<std::string> types = APT::Configuration::getCompressionTypes();
+      if (types.empty() == false)
+      {
+	 std::ostringstream os;
+	 std::copy(types.begin(), types.end()-1, std::ostream_iterator<std::string>(os, " "));
+	 os << *types.rbegin();
+	 DefCompressionTypes = os.str();
+      }
+   }
    bool const GzipIndex = _config->FindB("Acquire::GzipIndexes", false);
    for (std::vector<debReleaseIndexPrivate::debSectionEntry>::const_iterator E = entries.begin(); E != entries.end(); ++E)
    {
       for (std::vector<std::string>::const_iterator T = E->Targets.begin(); T != E->Targets.end(); ++T)
       {
-#define APT_T_CONFIG(X) _config->Find(std::string("Acquire::IndexTargets::") + Type  + "::" + *T + "::" + (X))
-	 std::string const tplMetaKey = APT_T_CONFIG(flatArchive ? "flatMetaKey" : "MetaKey");
-	 std::string const tplShortDesc = APT_T_CONFIG("ShortDescription");
-	 std::string const tplLongDesc = "$(SITE) " + APT_T_CONFIG(flatArchive ? "flatDescription" : "Description");
-	 bool const IsOptional = _config->FindB(std::string("Acquire::IndexTargets::") + Type + "::" + *T + "::Optional", true);
-	 bool const KeepCompressed = _config->FindB(std::string("Acquire::IndexTargets::") + Type + "::" + *T + "::KeepCompressed", GzipIndex);
-	 bool const UsePDiffs = _config->FindB(std::string("Acquire::IndexTargets::") + Type + "::" + *T + "::PDiffs", E->UsePDiffs);
-#undef APT_T_CONFIG
+#define APT_T_CONFIG_STR(X, Y) _config->Find(std::string("Acquire::IndexTargets::") + Type  + "::" + *T + "::" + (X), (Y))
+#define APT_T_CONFIG_BOOL(X, Y) _config->FindB(std::string("Acquire::IndexTargets::") + Type  + "::" + *T + "::" + (X), (Y))
+	 std::string const tplMetaKey = APT_T_CONFIG_STR(flatArchive ? "flatMetaKey" : "MetaKey", "");
+	 std::string const tplShortDesc = APT_T_CONFIG_STR("ShortDescription", "");
+	 std::string const tplLongDesc = "$(SITE) " + APT_T_CONFIG_STR(flatArchive ? "flatDescription" : "Description", "");
+	 bool const IsOptional = APT_T_CONFIG_BOOL("Optional", true);
+	 bool const KeepCompressed = APT_T_CONFIG_BOOL("KeepCompressed", GzipIndex);
+	 bool const UsePDiffs = APT_T_CONFIG_BOOL("PDiffs", E->UsePDiffs);
+	 std::string const CompressionTypes = APT_T_CONFIG_STR("CompressionTypes", DefCompressionTypes);
+#undef APT_T_CONFIG_BOOL
+#undef APT_T_CONFIG_STR
 	 if (tplMetaKey.empty())
 	    continue;
 
@@ -144,7 +159,7 @@ static void GetIndexTargetsFor(char const * const Type, std::string const &URI,
 
 	    for (std::vector<std::string>::const_iterator A = E->Architectures.begin(); A != E->Architectures.end(); ++A)
 	    {
-
+	       // available in templates
 	       std::map<std::string, std::string> Options;
 	       Options.insert(std::make_pair("SITE", Site));
 	       Options.insert(std::make_pair("RELEASE", Release));
@@ -154,14 +169,6 @@ static void GetIndexTargetsFor(char const * const Type, std::string const &URI,
 		  Options.insert(std::make_pair("LANGUAGE", *L));
 	       if (tplMetaKey.find("$(ARCHITECTURE)") != std::string::npos)
 		  Options.insert(std::make_pair("ARCHITECTURE", *A));
-	       Options.insert(std::make_pair("BASE_URI", baseURI));
-	       Options.insert(std::make_pair("REPO_URI", URI));
-	       Options.insert(std::make_pair("TARGET_OF", Type));
-	       Options.insert(std::make_pair("CREATED_BY", *T));
-	       if (UsePDiffs)
-		  Options.insert(std::make_pair("PDIFFS", "yes"));
-	       else
-		  Options.insert(std::make_pair("PDIFFS", "no"));
 
 	       std::string MetaKey = tplMetaKey;
 	       std::string ShortDesc = tplShortDesc;
@@ -172,6 +179,18 @@ static void GetIndexTargetsFor(char const * const Type, std::string const &URI,
 		  ShortDesc = SubstVar(ShortDesc, std::string("$(") + O->first + ")", O->second);
 		  LongDesc = SubstVar(LongDesc, std::string("$(") + O->first + ")", O->second);
 	       }
+
+	       // not available in templates, but in the indextarget
+	       Options.insert(std::make_pair("BASE_URI", baseURI));
+	       Options.insert(std::make_pair("REPO_URI", URI));
+	       Options.insert(std::make_pair("TARGET_OF", Type));
+	       Options.insert(std::make_pair("CREATED_BY", *T));
+	       if (UsePDiffs)
+		  Options.insert(std::make_pair("PDIFFS", "yes"));
+	       else
+		  Options.insert(std::make_pair("PDIFFS", "no"));
+	       Options.insert(std::make_pair("COMPRESSIONTYPES", CompressionTypes));
+
 	       IndexTarget Target(
 		     MetaKey,
 		     ShortDesc,

apt-pkg/indexfile.cc

@@ -19,6 +19,7 @@
 #include <apt-pkg/pkgcachegen.h>
 #include <apt-pkg/cacheiterators.h>
 #include <apt-pkg/srcrecords.h>
+#include <apt-pkg/strutl.h>
 #include <apt-pkg/progress.h>
 #include <apt-pkg/macros.h>
 
@@ -144,11 +145,12 @@ std::string IndexTarget::Option(OptionKeys const EnumKey) const		/*{{{*/
       APT_CASE(TARGET_OF);
       APT_CASE(CREATED_BY);
       APT_CASE(PDIFFS);
+      APT_CASE(COMPRESSIONTYPES);
 #undef APT_CASE
       case FILENAME: return _config->FindDir("Dir::State::lists") + URItoFileName(URI);
       case EXISTING_FILENAME:
 	 std::string const filename = Option(FILENAME);
-	 std::vector<std::string> const types = APT::Configuration::getCompressionTypes();
+	 std::vector<std::string> const types = VectorizeString(Option(COMPRESSIONTYPES), ' ');
 	 for (std::vector<std::string>::const_iterator t = types.begin(); t != types.end(); ++t)
 	 {
 	    if (t->empty())
@@ -208,7 +210,7 @@ std::string pkgDebianIndexTargetFile::IndexFileName() const			/*{{{*/
    if (FileExists(s))
       return s;
 
-   std::vector<std::string> types = APT::Configuration::getCompressionTypes();
+   std::vector<std::string> const types = VectorizeString(Target.Option(IndexTarget::COMPRESSIONTYPES), ' ');
    for (std::vector<std::string>::const_iterator t = types.begin(); t != types.end(); ++t)
    {
       std::string p = s + '.' + *t;

apt-pkg/indexfile.h

@@ -86,6 +86,7 @@ class IndexTarget							/*{{{*/
       FILENAME,
       EXISTING_FILENAME,
       PDIFFS,
+      COMPRESSIONTYPES,
    };
    std::string Option(OptionKeys const Key) const;
    bool OptionBool(OptionKeys const Key) const;

cmdline/apt-get.cc

@@ -1495,13 +1495,16 @@ static bool DoIndexTargets(CommandLine &CmdL)
 	       << "Description: " << T->Description << "\n"
 	       << "URI: " << T->URI << "\n"
 	       << "Filename: " << filename << "\n"
-	       << "Optional: " << (T->IsOptional ? "yes" : "no") << "\n";
+	       << "Optional: " << (T->IsOptional ? "yes" : "no") << "\n"
+	       << "KeepCompressed: " << (T->KeepCompressed ? "yes" : "no") << "\n";
 	    for (std::map<std::string,std::string>::const_iterator O = AddOptions.begin(); O != AddOptions.end(); ++O)
 	       stanza << format_key(O->first) << ": " << O->second << "\n";
 	    for (std::map<std::string,std::string>::const_iterator O = T->Options.begin(); O != T->Options.end(); ++O)
 	    {
 	       if (O->first == "PDIFFS")
 		  stanza << "PDiffs: " << O->second << "\n";
+	       else if (O->first == "COMPRESSIONTYPES")
+		  stanza << "CompressionTypes: " << O->second << "\n";
 	       else
 		  stanza << format_key(O->first) << ": " << O->second << "\n";
 	    }

doc/acquire-additional-files.txt

@@ -82,22 +82,37 @@ Additional optional properties:
   a hard error and the update process fails. Note that failures while
   downloading (e.g. 404 or hash verification errors) are failures,
   regardless of this setting.
+* KeepCompressed: The default is the value of Acquire::GzipIndexes,
+  which defaults to false. If true, the acquire system will keep the
+  file compressed on disk rather than extract it. If your frontend can't
+  deal with compressed files transparently you have to explicitly set
+  this option to false to avoid problems with users setting the option
+  globally. On the other hand, if you set it to true or don't set it you
+  have to ensure your frontend can deal with all compressed fileformats
+  supported by apt (libapt users can e.g. use FileFd).
 
 
 The acquire system will automatically choose to download a compressed
 file if it is available and uncompress it for you, just as it will also
-use pdiff patching if provided by the repository and enabled by the
+use PDiff patching if provided by the repository and enabled by the
 user. You only have to ensure that the Release file contains the
-information about the compressed files/pdiffs to make this happen.
+information about the compressed files/PDiffs to make this happen.
 *NO* properties have to be set to enable this!
 
 
-Additional properties exist, but these should *NOT* be set by frontends
+More properties exist, but these should *NOT* be set by frontends
 requesting files. They exist for internal and end-user usage only:
-* PDiffs: controls if apt will try to use pdiffs for this target.
+* PDiffs: controls if apt will try to use PDiffs for this target.
   Defaults to the value of Acquire::PDiffs which is true by default.
   Can be overridden per-source by the sources.list option of the same
   name. See the documentation for both of these for details.
+* CompressionTypes: The default value is a space separated list of
+  compression types supported by apt (see Acquire::CompressionTypes).
+  You can set this option to prevent apt from downloading a compression
+  type a frontend can't open transparently. This should always be
+  a temporary workaround through and a bug should be reported against
+  the frontend in question.
+
 
 # More examples
 
@@ -177,7 +192,7 @@ tools like 'grep-dctrl'.
 
 Accessing this information via libapt is done by reading the
 sources.lists (pkgSourceList), iterating over the metaIndex objects this
-creates and calling GetIndexTargets() on them. See the sourcecode of
+creates and calling GetIndexTargets() on them. See the source code of
 "apt-get indextargets" for a complete example.
 
 Note that by default targets are not listed if they weren't downloaded.
@@ -195,7 +210,7 @@ it will always refer to an uncompressed file, even if the index would be
 
 Remarks on fields only available in (default) --release-info mode:
 * Trusted: Denotes with a 'yes' or 'no' if the data in this file is
-  authenticated by a trustchain rooted in a trusted gpg key. You should
+  authenticated by a trust chain rooted in a trusted gpg key. You should
   be careful with untrusted data and warn the user if you use it.
 * Codename, Suite, Version, Origin and Label are fields from the Release
   file, are only present if they are present in the Release file and

methods/rred.cc

@@ -334,35 +334,30 @@ class Patch {
    FileChanges filechanges;
    MemBlock add_text;
 
-   static bool retry_fwrite(char *b, size_t l, FILE *f, Hashes *hash)
+   static bool retry_fwrite(char *b, size_t l, FileFd &f, Hashes *hash)
    {
-      size_t r = 1;
-      while (r > 0 && l > 0)
-      {
-         r = fwrite(b, 1, l, f);
-	 if (hash)
-	    hash->Add((unsigned char*)b, r);
-	 l -= r;
-	 b += r;
-      }
-      return l == 0;
+      if (f.Write(b, l) == false)
+	 return false;
+      if (hash)
+	 hash->Add((unsigned char*)b, l);
+      return true;
    }
 
-   static void dump_rest(FILE *o, FILE *i, Hashes *hash)
+   static void dump_rest(FileFd &o, FileFd &i, Hashes *hash)
    {
       char buffer[BLOCK_SIZE];
-      size_t l;
-      while (0 < (l = fread(buffer, 1, sizeof(buffer), i))) {
-	 if (!retry_fwrite(buffer, l, o, hash))
+      unsigned long long l = 0;
+      while (i.Read(buffer, sizeof(buffer), &l)) {
+	 if (l ==0  || !retry_fwrite(buffer, l, o, hash))
 	    break;
       }
    }
 
-   static void dump_lines(FILE *o, FILE *i, size_t n, Hashes *hash)
+   static void dump_lines(FileFd &o, FileFd &i, size_t n, Hashes *hash)
    {
       char buffer[BLOCK_SIZE];
       while (n > 0) {
-	 if (fgets(buffer, sizeof(buffer), i) == 0)
+	 if (i.ReadLine(buffer, sizeof(buffer)) == NULL)
 	    buffer[0] = '\0';
 	 size_t const l = strlen(buffer);
 	 if (l == 0 || buffer[l-1] == '\n')
@@ -371,11 +366,11 @@ class Patch {
       }
    }
 
-   static void skip_lines(FILE *i, int n)
+   static void skip_lines(FileFd &i, int n)
    {
       char buffer[BLOCK_SIZE];
       while (n > 0) {
-	 if (fgets(buffer, sizeof(buffer), i) == 0)
+	 if (i.ReadLine(buffer, sizeof(buffer)) == NULL)
 	    buffer[0] = '\0';
 	 size_t const l = strlen(buffer);
 	 if (l == 0 || buffer[l-1] == '\n')
@@ -383,7 +378,7 @@ class Patch {
       }
    }
 
-   static void dump_mem(FILE *o, char *p, size_t s, Hashes *hash) {
+   static void dump_mem(FileFd &o, char *p, size_t s, Hashes *hash) {
       retry_fwrite(p, s, o, hash);
    }
 
@@ -483,7 +478,7 @@ class Patch {
       return true;
    }
 
-   void write_diff(FILE *f)
+   void write_diff(FileFd &f)
    {
       unsigned long long line = 0;
       std::list<struct Change>::reverse_iterator ch;
@@ -496,31 +491,36 @@ class Patch {
 	 while (ch->del_cnt == 0 && ch->offset == 0)
 	    ++ch;
 	 line -= ch->del_cnt;
+	 std::string buf;
 	 if (ch->add_cnt > 0) {
 	    if (ch->del_cnt == 0) {
-	       fprintf(f, "%llua\n", line);
+	       strprintf(buf, "%llua\n", line);
 	    } else if (ch->del_cnt == 1) {
-	       fprintf(f, "%lluc\n", line+1);
+	       strprintf(buf, "%lluc\n", line+1);
 	    } else {
-	       fprintf(f, "%llu,%lluc\n", line+1, line+ch->del_cnt);
+	       strprintf(buf, "%llu,%lluc\n", line+1, line+ch->del_cnt);
 	    }
+	    f.Write(buf.c_str(), buf.length());
 
 	    mg_i = ch;
 	    do {
 	       dump_mem(f, mg_i->add, mg_i->add_len, NULL);
 	    } while (mg_i-- != mg_e);
 
-	    fprintf(f, ".\n");
+	    buf = ".\n";
+	    f.Write(buf.c_str(), buf.length());
 	 } else if (ch->del_cnt == 1) {
-	    fprintf(f, "%llud\n", line+1);
+	    strprintf(buf, "%llud\n", line+1);
+	    f.Write(buf.c_str(), buf.length());
 	 } else if (ch->del_cnt > 1) {
-	    fprintf(f, "%llu,%llud\n", line+1, line+ch->del_cnt);
+	    strprintf(buf, "%llu,%llud\n", line+1, line+ch->del_cnt);
+	    f.Write(buf.c_str(), buf.length());
 	 }
 	 line -= ch->offset;
       }
    }
 
-   void apply_against_file(FILE *out, FILE *in, Hashes *hash = NULL)
+   void apply_against_file(FileFd &out, FileFd &in, Hashes *hash = NULL)
    {
       std::list<struct Change>::iterator ch;
       for (ch = filechanges.begin(); ch != filechanges.end(); ++ch) {
@@ -635,14 +635,23 @@ class RredMethod : public pkgAcqMethod {
 	       << " and writing results to " << Itm->DestFile
 	       << std::endl;
 
-	 FILE *inp = fopen(Path.c_str(), "r");
-	 FILE *out = fopen(Itm->DestFile.c_str(), "w");
+	 FileFd inp, out;
+	 if (inp.Open(Path, FileFd::ReadOnly, FileFd::Extension) == false)
+	 {
+	    std::cerr << "FAILED to open inp " << Path << std::endl;
+	    return _error->Error("Failed to open inp %s", Path.c_str());
+	 }
+	 if (out.Open(Itm->DestFile, FileFd::WriteOnly | FileFd::Create, FileFd::Extension) == false)
+	 {
+	    std::cerr << "FAILED to open out " << Itm->DestFile << std::endl;
+	    return _error->Error("Failed to open out %s", Itm->DestFile.c_str());
+	 }
 
 	 Hashes hash(Itm->ExpectedHashes);
 	 patch.apply_against_file(out, inp, &hash);
 
-	 fclose(out);
-	 fclose(inp);
+	 out.Close();
+	 inp.Close();
 
 	 if (Debug == true) {
 	    std::clog << "rred: finished file patching of " << Path  << "." << std::endl;
@@ -717,12 +726,13 @@ int main(int argc, char **argv)
    }
 
    if (just_diff) {
-      patch.write_diff(stdout);
+      FileFd out;
+      out.OpenDescriptor(STDOUT_FILENO, FileFd::WriteOnly | FileFd::Create);
+      patch.write_diff(out);
    } else {
-      FILE *out, *inp;
-      out = stdout;
-      inp = stdin;
-
+      FileFd out, inp;
+      out.OpenDescriptor(STDOUT_FILENO, FileFd::WriteOnly | FileFd::Create);
+      inp.OpenDescriptor(STDIN_FILENO, FileFd::ReadOnly);
       patch.apply_against_file(out, inp);
    }
    return 0;

test/integration/test-pdiff-usage

@@ -254,23 +254,28 @@ echo 'Debug::pkgAcquire::Diffs "true";
 Debug::Acquire::Transaction "true";
 Debug::pkgAcquire::rred "true";' > rootdir/etc/apt/apt.conf.d/rreddebug.conf
 
-testrun nohash -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=1
-testrun nohash -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=1
+testcase() {
+	testrun nohash -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=1 "$@"
+	testrun nohash -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=1 "$@"
 
-testrun -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=1
-testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=1
-testrun -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=0
-testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=0
+	testrun -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=1 "$@"
+	testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=1 "$@"
+	testrun -o Acquire::PDiffs::Merge=0 -o APT::Get::List-Cleanup=0 "$@"
+	testrun -o Acquire::PDiffs::Merge=1 -o APT::Get::List-Cleanup=0 "$@"
 
-sha256sum() {
-	echo '01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b -'
-}
-testrun -o Acquire::PDiffs::Merge=0 -o Acquire::ForceHash=SHA1
-testrun -o Acquire::PDiffs::Merge=1 -o Acquire::ForceHash=SHA1
+	sha256sum() {
+		echo '01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b -'
+	}
+	testrun -o Acquire::PDiffs::Merge=0 -o Acquire::ForceHash=SHA1 "$@"
+	testrun -o Acquire::PDiffs::Merge=1 -o Acquire::ForceHash=SHA1 "$@"
+	unset -f sha256sum
 
-unset -f sha256sum
-sha1sum() {
-	echo 'adc83b19e793491b1c6ea0fd8b46cd9f32e592fc -'
+	sha1sum() {
+		echo 'adc83b19e793491b1c6ea0fd8b46cd9f32e592fc -'
+	}
+	testrun -o Acquire::PDiffs::Merge=0 -o Acquire::ForceHash=SHA256 "$@"
+	testrun -o Acquire::PDiffs::Merge=1 -o Acquire::ForceHash=SHA256 "$@"
+	unset -f sha1sum
 }
-testrun -o Acquire::PDiffs::Merge=0 -o Acquire::ForceHash=SHA256
-testrun -o Acquire::PDiffs::Merge=1 -o Acquire::ForceHash=SHA256
+testcase -o Acquire::GzipIndexes=0
+testcase -o Acquire::GzipIndexes=1