use one 'store' method to rule all (de)compressors

Adding a new compressor method meant adding a new method as well – even
if that boilt down to just linking to our generalized decompressor with
a new name. That is unneeded busywork if we can instead just call the
generalized decompressor and let it figure out which compressor to use
based on the filenames rather than by program name.

For compatibility we ship still 'gzip', 'bzip2' and co, but they are
just links to our "new" 'store' method.

apt-pkg/acquire-item.cc

@@ -422,7 +422,7 @@ bool pkgAcqIndex::TransactionState(TransactionStates const state)
 	 {
 	    // keep the compressed file, but drop the decompressed
 	    EraseFileName.clear();
-	    if (PartialFile.empty() == false && flExtension(PartialFile) == "decomp")
+	    if (PartialFile.empty() == false && flExtension(PartialFile) != CurrentCompressionExtension)
 	       RemoveFile("TransactionAbort", PartialFile);
 	 }
 	 break;
@@ -2681,7 +2681,8 @@ void pkgAcqIndex::StageDownloadDone(string const &Message, HashStringList const
    {
       Stage = STAGE_DECOMPRESS_AND_VERIFY;
       Local = true;
-      DestFile += ".decomp";
+      if (CurrentCompressionExtension != "uncompressed")
+	 DestFile.erase(DestFile.length() - (CurrentCompressionExtension.length() + 1));
       Desc.URI = "copy:" + FileName;
       QueueURI(Desc);
       SetActiveSubprocess("copy");
@@ -2703,6 +2704,18 @@ void pkgAcqIndex::StageDownloadDone(string const &Message, HashStringList const
 	 SetActiveSubprocess("copy");
 	 return;
       }
+      else
+      {
+	 // symlinking ensures that the filename can be used for compression detection
+	 // that is e.g. needed for by-hash over file
+	 if (symlink(FileName.c_str(),DestFile.c_str()) != 0)
+	    _error->WarningE("pkgAcqIndex::StageDownloadDone", "Symlinking file %s to %s failed", FileName.c_str(), DestFile.c_str());
+	 else
+	 {
+	    EraseFileName = DestFile;
+	    FileName = DestFile;
+	 }
+      }
    }
    else
       EraseFileName = FileName;
@@ -2717,25 +2730,19 @@ void pkgAcqIndex::StageDownloadDone(string const &Message, HashStringList const
       return;
    }
 
-   // get the binary name for your used compression type
-   string decompProg;
-   if(CurrentCompressionExtension == "uncompressed")
-      decompProg = "copy";
-   else
-      decompProg = _config->Find(string("Acquire::CompressionTypes::").append(CurrentCompressionExtension),"");
-   if(decompProg.empty() == true)
-   {
-      _error->Error("Unsupported extension: %s", CurrentCompressionExtension.c_str());
-      return;
-   }
-
+   string decompProg = "store";
    if (Target.KeepCompressed == true)
    {
       DestFile = "/dev/null";
       EraseFileName.clear();
    }
    else
-      DestFile += ".decomp";
+   {
+      if (CurrentCompressionExtension == "uncompressed")
+	 decompProg = "copy";
+      else
+	 DestFile.erase(DestFile.length() - (CurrentCompressionExtension.length() + 1));
+   }
 
    // queue uri for the next stage
    Stage = STAGE_DECOMPRESS_AND_VERIFY;

methods/makefile

@@ -23,11 +23,11 @@ LIB_MAKES = apt-pkg/makefile
 SOURCE = copy.cc
 include $(PROGRAM_H)
 
-# The gzip method
-PROGRAM=gzip
+# The store method
+PROGRAM=store
 SLIBS = -lapt-pkg $(INTLLIBS)
 LIB_MAKES = apt-pkg/makefile
-SOURCE = gzip.cc
+SOURCE = store.cc
 include $(PROGRAM_H)
 
 # The gpgv method
@@ -96,15 +96,15 @@ $(BIN)/ssh:
 clean-$(BIN)/ssh:
 	-rm $(BIN)/ssh
 
-# create links for all other compressors
-COMPRESSORS=bzip2 lzma xz
+# create compat links for all compressors
+COMPRESSORS=gzip bzip2 lzma xz
 
 binary: $(addprefix $(BIN)/,$(COMPRESSORS))
 veryclean: $(addprefix clean-$(BIN)/,$(COMPRESSORS))
 
-$(addprefix $(BIN)/,$(COMPRESSORS)): $(BIN)/gzip
+$(addprefix $(BIN)/,$(COMPRESSORS)): $(BIN)/store
 	echo "Installing $(notdir $@) method link"
-	ln -fs gzip $@
+	ln -fs store $@
 
 $(addprefix clean-$(BIN)/,$(COMPRESSORS)):
 	-rm $(BIN)/$(notdir $@)

methods/gzip.cc

@@ -1,11 +1,13 @@
 // -*- mode: cpp; mode: fold -*-
 // Description								/*{{{*/
-// $Id: gzip.cc,v 1.17.2.1 2004/01/16 18:58:50 mdz Exp $
 /* ######################################################################
 
-   GZip method - Take a file URI in and decompress it into the target 
-   file.
-   
+   Store method - Takes a file URI and stores its content (for which it will
+   calculate the hashes) in the given destination. The input file will be
+   extracted based on its file extension (or with the given compressor if
+   called with one of the compatible symlinks) and potentially recompressed
+   based on the file extension of the destination filename.
+
    ##################################################################### */
 									/*}}}*/
 // Include Files							/*{{{*/
@@ -29,72 +31,78 @@
 #include <apti18n.h>
 									/*}}}*/
 
-class GzipMethod : public aptMethod
+class StoreMethod : public aptMethod
 {
    std::string const Prog;
    virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
 
    public:
 
-   explicit GzipMethod(std::string const &pProg) : aptMethod(pProg.c_str(),"1.1",SingleInstance | SendConfig), Prog(pProg) {};
+   explicit StoreMethod(std::string const &pProg) : aptMethod(pProg.c_str(),"1.2",SingleInstance | SendConfig), Prog(pProg) {};
 };
 
-// GzipMethod::Fetch - Decompress the passed URI			/*{{{*/
-// ---------------------------------------------------------------------
-/* */
-bool GzipMethod::Fetch(FetchItem *Itm)
+static bool OpenFileWithCompressorByName(FileFd &fileFd, std::string const &Filename, unsigned int const Mode, std::string const &Name)
 {
-   URI Get = Itm->Uri;
-   std::string Path = Get.Host + Get.Path; // To account for relative paths
-   
-   FetchResult Res;
-   Res.Filename = Itm->DestFile;
-   URIStart(Res);
+   if (Name == "store")
+      return fileFd.Open(Filename, Mode, FileFd::Extension);
 
    std::vector<APT::Configuration::Compressor> const compressors = APT::Configuration::getCompressors();
    std::vector<APT::Configuration::Compressor>::const_iterator compressor = compressors.begin();
    for (; compressor != compressors.end(); ++compressor)
-      if (compressor->Name == Prog)
+      if (compressor->Name == Name)
 	 break;
    if (compressor == compressors.end())
-      return _error->Error("Extraction of file %s requires unknown compressor %s", Path.c_str(), Prog.c_str());
+      return _error->Error("Extraction of file %s requires unknown compressor %s", Filename.c_str(), Name.c_str());
+   return fileFd.Open(Filename, Mode, *compressor);
+}
+
+
+									/*}}}*/
+bool StoreMethod::Fetch(FetchItem *Itm)					/*{{{*/
+{
+   URI Get = Itm->Uri;
+   std::string Path = Get.Host + Get.Path; // To account for relative paths
+   
+   FetchResult Res;
+   Res.Filename = Itm->DestFile;
+   URIStart(Res);
 
    // Open the source and destination files
    FileFd From;
    if (_config->FindB("Method::Compress", false) == false)
    {
-      From.Open(Path, FileFd::ReadOnly, *compressor);
+      if (OpenFileWithCompressorByName(From, Path, FileFd::ReadOnly, Prog) == false)
+	 return false;
       if(From.FileSize() == 0)
 	 return _error->Error(_("Empty files can't be valid archives"));
    }
    else
-      From.Open(Path, FileFd::ReadOnly);
+      From.Open(Path, FileFd::ReadOnly, FileFd::Extension);
    if (From.IsOpen() == false || From.Failed() == true)
       return false;
 
    FileFd To;
-   if (Itm->DestFile != "/dev/null")
+   if (Itm->DestFile != "/dev/null" && Itm->DestFile != Path)
    {
       if (_config->FindB("Method::Compress", false) == false)
-	 To.Open(Itm->DestFile, FileFd::WriteAtomic);
-      else
-	 To.Open(Itm->DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Empty, *compressor);
+	 To.Open(Itm->DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Atomic, FileFd::Extension);
+      else if (OpenFileWithCompressorByName(To, Itm->DestFile, FileFd::WriteOnly | FileFd::Create | FileFd::Empty, Prog) == false)
+	    return false;
 
       if (To.IsOpen() == false || To.Failed() == true)
 	 return false;
       To.EraseOnFailure();
    }
 
-
    // Read data from source, generate checksums and write
    Hashes Hash(Itm->ExpectedHashes);
    bool Failed = false;
    Res.Size = 0;
-   while (1) 
+   while (1)
    {
       unsigned char Buffer[4*1024];
       unsigned long long Count = 0;
-      
+
       if (!From.Read(Buffer,sizeof(Buffer),&Count))
       {
 	 if (To.IsOpen())
@@ -110,9 +118,9 @@ bool GzipMethod::Fetch(FetchItem *Itm)
       {
 	 Failed = true;
 	 break;
-      }      
+      }
    }
-   
+
    From.Close();
    To.Close();
 
@@ -146,6 +154,6 @@ int main(int, char *argv[])
 {
    setlocale(LC_ALL, "");
 
-   GzipMethod Mth(flNotDir(argv[0]));
+   StoreMethod Mth(flNotDir(argv[0]));
    return Mth.Run();
 }

test/integration/test-bug-595691-empty-and-broken-archive-files

@@ -55,7 +55,7 @@ Reading package lists..." "empty archive Packages.$COMPRESS over file"
 Err:2 file:$APTARCHIVE  Packages
   Empty files can't be valid archives
 Reading package lists...
-W: Failed to fetch ${COMPRESSOR}:${APTARCHIVE}/Packages.$COMPRESS  Empty files can't be valid archives
+W: Failed to fetch store:$(readlink -f rootdir/var/lib/apt/lists/partial/$(echo "$APTARCHIVE" | sed -e 's#/#_#g')_Packages.${COMPRESS})  Empty files can't be valid archives
 E: Some index files failed to download. They have been ignored, or old ones used instead." "empty file Packages.$COMPRESS over file"
 }
 
@@ -72,7 +72,7 @@ Reading package lists..." "empty archive Packages.$COMPRESS over http"
 Err:2 http://localhost:${APTHTTPPORT}  Packages
   Empty files can't be valid archives
 Reading package lists...
-W: Failed to fetch ${COMPRESSOR}:$(readlink -f rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_Packages.${COMPRESS})  Empty files can't be valid archives
+W: Failed to fetch store:$(readlink -f rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_Packages.${COMPRESS})  Empty files can't be valid archives
 E: Some index files failed to download. They have been ignored, or old ones used instead." "empty file Packages.$COMPRESS over http"
 }